hoodiehq / hoodie-server

:dog: Hapi plugin for Hoodie’s server core module
Apache License 2.0
244 stars 48 forks source link

Static files on windows #262

Closed Acconut closed 8 years ago

Acconut commented 10 years ago

In the past there have been problems with serving static files using hapi which seem to be not fixed properly. This is neither a problem in hoodie nor hapi but in node.js core (they are aware of that but don't fix it; maybe because of compatibility).

I've created a dirty hotfix which overrides path.join in order to support driver letters which are used on windows:

if(process.platform === 'win32') {
    /*****************************
    PATH FIX FOR WINDOWS
    *****************************/

    console.log("Hotpatching path.join for windows compatibility. Avoid this in production!");

    var path = require('path');
    path.__orig_join = path.join;
    path.join = function() {

        var args = Array.prototype.slice.call(arguments);

        for(var i = args.length - 1; i >= 0; i--) {

            if(/^[a-zA-Z]{1}:\\/i.test(args[i])) {
               return path.__orig_join.apply(path, args.slice(i));
            }

        }

        return path.__orig_join.apply(path, args);

    };
}

It should be placed inside lib/index.js so that every module uses the patched version. I don't think this should be merged into hoodie-server as I assume security issues but a note somewhere (e.g. hood.ie) could be worth it.

svnlto commented 10 years ago

Can you point to a reference on the node.js issue tracker where this is being discussed?

Acconut commented 10 years ago

You can't call it a discussion: https://github.com/joyent/node/issues/5989

janl commented 10 years ago

Heya @Acconut — I commented on the issue. I think the easiest thing is to ensure that all but the first argument to path.join() is not an absolute path, outside of path.join(). That’s basic input sanitation that we could have in hoodie-server.

janl commented 10 years ago

@Acconut also: thanks for sniffing these out, it makes Hoodie a really solid project. Keep it up! :)

Acconut commented 10 years ago

I tried to get into Hapi's core to find a solution without this hack but I couldn't find one. This doesn't mean there is none but I just didn't find one. So this approach is the simplest one. I just want to use Hoodie and if that requires such "rough" methods I'm going to use them. :) And I should thank you for building this awesome project.

janl commented 10 years ago

we can just add some code to hoodie to sanitise the input to make sure we don’t call path.resolve() with an absolute windows path in all but the first arguments. Note that I disagree with you on what the default case should be :)

Acconut commented 10 years ago

Ok, I'll try to find a solution without this hack. :)