Path should always be a path, not a URL. #344

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

tomlea commented Dec 2, 2012

When proxying to an app that is rather strict about it's HTTP headers, I was finding that the path was being set to the URL, causing 404 responses.

I've checked this does not break any of the vows, but I can't figure out how to test this in an acceptable way.

Any ideas?

Owner

indexzero commented Dec 19, 2012

Why would this ever happen? Seems like a node.js core bug and not related to node-http-proxy

Contributor

mmalecki commented Dec 22, 2012

@indexzero is correct. Do you have a reproducible example of such behavior?

good patch, very helpful to me.

Contributor

mmalecki commented Jan 8, 2013

@luolonghao can you reproduce this behavior? What source are those requests coming from?

tomlea commented Jan 29, 2013

Further investigation shows that I'm wrong, but not for the reasons thought.

The proxy works perfectly when used as a reverse proxy in front of an app. The problem only seems to come up when using it as a forwarding proxy (one that is set up as the HTTP proxy server for my machine).

This very minimal app, when used as a forwarding proxy, demonstrates the problem:

// Instructions:
// * Start server
// * Set browser to proxy to localhost:8001
// * Navigate to http://bits.wikimedia.org/en.wikipedia.org/load.php
// * Observe as it 404s
var httpProxy = require('http-proxy');
httpProxy.createServer(function (req, res, proxy) {
  var target = {host: 'bits.wikimedia.org', port: 80}
  proxy.proxyRequest(req, res, target);
}).listen(8001);

When requesting via a proxy, we get requests from browsers that look like this:

GET http://localhost:8001/ HTTP/1.1
Host: localhost:8001
Proxy-Connection: keep-alive

Compared to this where no proxy is configured, and a direct connection is made:

GET / HTTP/1.1
Host: localhost:8001
Connection: keep-alive

As you can see, the GET line has the full URI. Looking into this (http://www.ietf.org/rfc/rfc2616.txt 5.1.2), we find that the use of an absolute URI here is normal.

node-http-proxy then does a permissible thing by the spec, and passes it on to the destination HTTP server. Unfortunately, not all servers are conformant, and some fail when given an absolute URI rather than an absolute path (one such server serves the page at http://bits.wikimedia.org/en.wikipedia.org/load.php, although this is just one example of many).

From rfc2616:

To allow for transition to absoluteURIs in all requests in future versions of HTTP, all HTTP/1.1 servers MUST accept the absoluteURI form in requests, even though HTTP/1.1 clients will only generate them in requests to proxies.

Basically, all down stream servers should accept it, but in practice, they do not.

Anyway, the patch will allow users to use node-http-proxy as a forwarding proxy in the real world.

Owner

indexzero commented Mar 9, 2013

This is too much overhead to support incorrect backends.

@indexzero indexzero closed this Mar 9, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment