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

node-static win32 forbiden #42

Closed
dacow opened this issue Dec 30, 2011 · 5 comments · Fixed by #124
Closed

node-static win32 forbiden #42

dacow opened this issue Dec 30, 2011 · 5 comments · Fixed by #124

Comments

@dacow
Copy link

dacow commented Dec 30, 2011

using node + node-static on winxp and setting
var fileServer = new nstatic.Server("C:/public");
gave me an error everytime i tried to acces the localhost:8080 expecting to load the index.html

error comes from this line in node-static.js

this.Server.prototype.servePath = function (pathname, status, headers, req, res, finish)
......
// Make sure we're not trying to access a file outside of the root.
if (new(RegExp)('^' + that.root).test(pathname)) {

in that regex, pathname was C:\public and that.root = C:\public too.

so while in development (i dont have to worry bout someone trying to acces another files)
i comented this line and now is working fine

sorry for my english

@daxxog
Copy link

daxxog commented Dec 22, 2012

Maybe we could have a development flag to allow access outside the path.

@phstc
Copy link
Collaborator

phstc commented Dec 22, 2012

Wasn't clear for me what happened on Windows. Which error was raised?

@anseki
Copy link

anseki commented Dec 26, 2013

Hello. Thanks for your great plugin.

I got a same error, when document-root is relative path. new static.Server('./public')
I found out the cause.
path.join() is used in Server.prototype.resolve().
https://github.com/cloudhead/node-static/blob/master/lib/node-static.js#L158
path.join() returns strange result.

TEST:

var path = require('path');
var current = path.resolve('.');
console.log('current: ' + current);
var joinedPath = path.join(current, 'foo-bar');
console.log('joinedPath: ' + joinedPath);

RESULT: Look the Drive-Letter E and e.

current: E:\Documents
joinedPath: e:\Documents\foo-bar

Therefore, indexOf() returns unmatch, and Forbidden Error is given.
https://github.com/cloudhead/node-static/blob/master/lib/node-static.js#L138
I don't know why path.join() returns strange result. I tried Node v0.11.9, I got same error. Problem may be Python version that was used to build.

So, Windows ignores that letter of path is upper case or lower case. Therefore, I rewrote below, and this error was resolved.
https://github.com/cloudhead/node-static/blob/master/lib/node-static.js#L158

    //if (pathname.indexOf(that.root) === 0) {
    if ((require('os').platform() === 'win32' ?
      pathname.toLowerCase().indexOf(that.root.toLowerCase()) : // Windows
      pathname.indexOf(that.root)) === 0) {                     // Others

Or, lower case drive-letter is given to constructor, and this error was resolved too. new static.Server('e:\Documents\foo-bar')
Because path.resolve don't change given drive-letter.

@mrfabbri
Copy link
Contributor

@anseki [I stumbled on the same problem] actually the problem lies in the commit
nodejs/node-v0.x-archive@a05f973 which changed the behaviour of normalize (used by join), see:

https://github.com/joyent/node/blob/b9bec2031e5f44f47cf01c3ec466ab8cddfa94f6/lib/path.js#L243

[ it is being discussed on https://github.com/nodejs/node-v0.x-archive/issues/7031 ]

Another possible workaround would be to call resolve() and normalize() on any path passed as document-root:

new static.Server(path.normalize(path.resolve(aPath)));

which is what koa-send does

https://github.com/koajs/send/blob/1d02d1b7c264daa43c4b18a918271f1dca8cc5fd/index.js#L41

@anseki
Copy link

anseki commented Jun 18, 2014

I didn't know why path.join and path.resolve return different result. Thank you.
I wrote new static.Server('e:\Documents\foo-bar') above, it is same to using path.normalize.
But, I think that changing all of path string may be better, because the target of this checking is not only drive-letter. Windows ignores capital letter or not in all of path string.
For example:
http://windows.microsoft.com/en-us/windows/home
http://windows.microsoft.com/en-us/WiNdOwS/home
Therefore, toLowerCase(allOfPath) may be better for the check that requested path is outside of the root or not.

(But, I don't know that this behaviour of Web Server is good or not. RFC1738 says that scheme should be ignored capital letter or not. but it doesn't say about others (url-path etc.).)

mrfabbri added a commit to mrfabbri/node-static that referenced this issue Jun 18, 2014
- Calling resolve() doesn't fully normalize relative path on Windows (in Node v0.11.x, see
  nodejs/node-v0.x-archive#7031 ), i.e. doesn't lowercase drive letter, while normalize() does, which causes
  paths resulting from calling normalize() and join() (depending on normalize()) to differ and to
  break comparisons.

Fixes cloudhead#42.
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 a pull request may close this issue.

5 participants