Fix proxy path

[Domiii]

This fix considers the actual target path again (which has been ignored).

[jcrugzz]

@Domiii I can see this being valid but you need to handle the case where path is not defined. See the failing tests under 0.10.x.

[Domiii]

Yeah. I suppose, we should also use url.resolve instead of +. I'll take a shot at it later today.

[jcrugzz]

@Domiii well it needs to be ignored in the case where the path is undefined

[Rush]

I would instead like to see a hook so that is could be fixed manually.

[greaterweb]

I ran into a similar issue when using this in my project. Having the full path honored seems to be the expected functionality so hopefully this will be patch in a future release.

For the time being, here is an example work around that doesn't require any updates to http-proxy, it expands on the ProxyTable API example.

var httpProxy = require('http-proxy');

// Additional dependencies needed
var path = require('path');
var url = require('url');

var proxy = httpProxy.createProxy();

var routes = {  
    'foo.com': 'http://website.com:8001',
    'bar.com': 'http://website2.com:8002/foo',
    'baz.com': 'http://website.com:8001/foo/bar',
    'qux.com': 'http://website2.com:8002/foo/bar/baz'
};

require('http').createServer(function(req, res) {  
    // Before sending to the proxy, I actually test to see
    // there is a key present in the proxy table (aka, routes)
    if (routes[req.headers.host]) {
        // parse out the hostname
        var route = url.parse(routes[req.headers.host]);
        // use path to neatly join the route path to the
        // requested url. this keeps the output valid when
        // the route.path == '/'
        req.url = path.join(route.path, req.url);
        // send off to the proxy as you normally would
        return proxy.web(req, res, {
            target: routes[req.headers.host]
        });
    }
    // If we have no proxy target just send a 404
    res.writeHead(404);
    res.end();
}).listen(8000);

[jcrugzz]

@greaterweb thanks for the example! I just modified your comment to have the correct api :).

[greaterweb]

Is this better fit as written like below? This matches the convention used throughout the setupOutgoing method.

outgoing.path =  options[forward || 'target'].path;

This could also be expanded to be written as below. This would likely satisfy the integration testing.

outgoing.path =  options[forward || 'target'].path || '';

If we don't like the idea of setting outgoing.path as an empty string this could easily be rewritten to be wrapped in some conditionals.

[greaterweb]

Thanks for the fix, @jcrugzz

Copy and paste fail on my end :-) I coupled things from the ProxyTable example and my project (which uses express and does a few other things).

[EndangeredMassa]

What's the next step for this? Just fix the tests?

[jcrugzz]

@EndangeredMassa yes it needs to be able to pass the current tests and there should be tests for the use case that this is a fix for.

[EndangeredMassa]

@Domiii do you plan on fixing up the tests for this? If not, I could take a look.

[indexzero]

@EndangeredMassa that would be huge.

[EndangeredMassa]

Submitted a new PR (that includes this original commit) to address the tests. Required some code changes.

#693

[jcrugzz]

Fixed in #693

[Domiii]

Thank you, EndangeredMassa! And sorry for the late response!

[EndangeredMassa]

No worries!