Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Host out of www directory #2

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

BigBlueHat
Copy link
Collaborator

I've tweaked the defaults to host out of a www directory, so that the write space doesn't end up being identical to the code space.

It makes things a bit easier to understand and manage when committing and updating. 😺

@csarven
Copy link
Owner

csarven commented May 9, 2017

I'm not sure I understand the intention entirely. The write space is relative to the root (the *Path stuff configs). Were you referring to something else?

Using www would be hardcoding no? The intention with . was that, you can place it in any directory and it will figure out the actual path it needs to at run time.

@BigBlueHat
Copy link
Collaborator Author

This just changed the defaults in the config and if absent. Feel free to try out and try these newfangled GitHub code review things. There's no hard coding. Just defaults.

Let me know (for instance) of your current config and this code don't get along, but it should.

@csarven
Copy link
Owner

csarven commented May 10, 2017

Ok, I missed the obvious "Host out of www directory". That seems reasonable indeed, and okay with moving some of the stuff from the root directory to www/ eg., *Path/, index.html.

However, I think this PR is unfinished. Code below starts server, and the request $ curl -i http://localhost:3000/ breaks it.

$ node index.js 
Applied: /var/www/mayktso/config.json
Applying defaults:
{ port: '3000',
  inboxPath: 'inbox/',
  queuePath: 'queue/',
  maxPayloadSize: 1000,
  maxResourceCount: 10,
  proxyURL: 'https://dokie.li/proxy?uri=',
  hostname: 'localhost',
  scheme: 'http',
  authority: 'http://localhost:3000',
  rootPath: 'www/',
  basePath: '',
  annotationPath: 'www/annotation/',
  reportsPath: 'www/reports/' }
process.cwd(): /var/www/mayktso
rootPath: www/
curl -i http://localhost:3000

[2017-05-10 10:36:34.766] [LOG]   GET http://localhost:3000/  {"host":"localhost:3000","user-agent":"curl/7.52.1","accept":"*/*"}
[2017-05-10 10:36:34.769] [LOG]   GET http://localhost:3000/  { Error: ENOENT: no such file or directory, open 'www//index.html'
  errno: -2,
  code: 'ENOENT',
  syscall: 'open',
  path: 'www//index.html' }
buffer.js:362
    throw new TypeError('"string" must be a string, Buffer, or ArrayBuffer');
    ^

TypeError: "string" must be a string, Buffer, or ArrayBuffer
    at Function.byteLength (buffer.js:362:11)
    at sendHeaders (/var/www/mayktso/index.js:699:42)
    at ReadFileContext.callback (/var/www/mayktso/index.js:707:9)
    at FSReqWrap.readFileAfterOpen [as oncomplete] (fs.js:359:13)

Request to $ curl -i http://localhost:3000/www/ returns a 404 because index.html is not loaded from root directory (i.e., we need to move index.html to www/index.html) but that's a minor issue. Probably need to check all paths as well e.g., Location in response header, and base paths/urls etc.

The idea to just load from . was to have minimal involvment to have it up and running. node install, node index.js pretty much. Ideally we don't want www publicly visible in the URL like at http://localhost:3000/www/ even if served from that directory. Similarly, if reverse proxy is used, www shouldn't be visible on the Web. Makes sense?

@BigBlueHat
Copy link
Collaborator Author

Made some more tweaks to deal with the index.html situation. It should also fix/improve some of the rootPath assumptions I'd made.

@BigBlueHat
Copy link
Collaborator Author

@csarven does this need any more changes?

Enforce local building of code--for now.
Prevent accidently over sharing credentials.
This provides for bootstrapping a hosting directory
with a minimal discovery HTML document.

It is copied on the first run of the server, but not
on subsequent runs to avoid overwriting changes.

Re-bootstrapping simply requires moving the `index.html`
from the `rootPath` directory.
rootPath is now...root. Other paths are all expected to
be served from inside of rootPath (relatively).
Avoids double // issues
@BigBlueHat
Copy link
Collaborator Author

@csarven curious if you're still interested in this nearly-year-old PR. 😺

@csarven
Copy link
Owner

csarven commented May 3, 2018

Sorry, let me come back to this. Still interested in this change.

I recall that we wanted to keep maytkso working more or less the same way without breaking other stuff like https://github.com/csarven/ldn-tests which happens to extend it - sort of like a specialised site. So, I'll have to test the paths, configs etc on ldn-tests as well, and update both at the same time.

createDir(config['rootPath'] + path);
});

// rootPath folder does not contain an index.html file...so we'll copy the
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we ship with www/index.html and avoid this altogether? It is possible that the server may not want to have an index.html and so this shouldn't override that I think.

@BigBlueHat
Copy link
Collaborator Author

@csarven for what very little it's worth, I've (attempted to) resolve the conflicts here--which were minimal.

I'm still happy to contribute to this project if you need/want. 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants