Don't write chunked responses to HTTP/1.0 clients

[indexzero]

This is a follow up to this bug on node-http-proxy. The behavior can be seen as follows:

  $ cd /path/to/node-http-proxy
  $ forever start examples/standalone-proxy-server.js
  $ curl -0 -v http://localhost:8004/
  * About to connect() to localhost port 8004 (#0)
*   Trying ::1... Connection refused
*   Trying fe80::1... Connection refused
*   Trying 127.0.0.1... connected
* Connected to localhost (127.0.0.1) port 8004 (#0)
> GET / HTTP/1.0
> User-Agent: curl/7.19.7 (universal-apple-darwin10.0) libcurl/7.19.7 OpenSSL/0.9.8l zlib/1.2.3
> Host: localhost:8004
> Accept: */*
> 
< HTTP/1.1 200 OK
< content-type: text/plain
< connection: close
< transfer-encoding: chunked
< 
request successfully proxied to: /
{
  "user-agent": "curl/7.19.7 (universal-apple-darwin10.0) libcurl/7.19.7 OpenSSL/0.9.8l zlib/1.2.3",
  "host": "localhost:8004",
  "accept": "*/*",
  "x-forwarded-for": "127.0.0.1",
  "x-forwarded-port": "53575",
  "x-forwarded-proto": "http",
  "connection": "close"
* Closing connection #0
}

The source of this bug seems to be squarely in the ServerResponse object. I don't know how much things have changed in v0.5.0 but the statusLine written to all outgoing requests is hard-coded to HTTP/1.1. There is probably more to it, but for starters it seems like the ServerResponse object should infer the version of the HTTP protocol to use from the associated ServerRequest.

[bnoordhuis]

Bug fix and test cases. Merge!

One semi-surprising datum is that the two HTTP/1.1 chunks emitted by the server arrive as a single TCP packet on the client. Is this the kernel being clever? Because I don't think it's node doing the buffering. Anyway.

[ry]

should respond with http/1.1 and but without chunked. terminate message with eof.

[bnoordhuis]

I think we can revert 8dc8773 with 1539385 because http.js doesn't seem to be broken any more. Tests pass (as do probes with netcat and curl) on both master and v0.4. Weird, because they sure failed on June 29.

[mnot]

To be clear - Node should always use HTTP/1.1 in its status line, no matter what the client says (as long as Node is HTTP/1.1 conformant, of course).

However, it shouldn't chunk a response to a 1.0 client.

[koichik]

Node's HTTP server is already implemented like that @mnot says.
https://github.com/joyent/node/blob/master/lib/http.js#L770-773

But Node's HTTP client supports only HTTP/1.1.
https://github.com/joyent/node/blob/master/lib/http.js#L918

Therefore node-http-proxy always sends HTTP/1.1 request to the actual server, doesn't it?
I think this problem occurs by this scenario:

• client sends HTTP/1.0 request to node-http-proxy.
• node-http-proxy sends HTTP/1.1 request to actual server.
• actual server sends HTTP/1.1 (chunked) response to node-http-proxy.
• node-http-proxy sends HTTP/1.1 (chunked) response to client.

Node HTTP client should support for HTTP/1.0?

[koichik]

No, it is not necessary.
If client was HTTP/1.0, node-http-proxy should remove 'transfer-encoding' header from actual server's response before send it to client (https://github.com/nodejitsu/node-http-proxy/blob/master/lib/node-http-proxy.js#L523)
like this:

    if ((req.httpVersionMajor < 1 || req.httpVersionMinor < 1) &&
        response.headers['transfer-encoding']) {
      delete response.headers['transfer-encoding'];
    }
    res.writeHead(response.statusCode, response.headers);

[mnot]

While it's at it, node-http-client needs to delete all "hop-by-hop" headers, in both directions; see:
http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-14#section-7.1.3

[ry]

3e2a2a7 was reverted b62ecdc

[ry]

@koichik - it also needs to modify the response that it's sending - remove the chunk frames and terminate the message with EOF (since it does not know the content-length)

[koichik]

@ry - It is not necessary. Because http.ClieantResponse (receives from actual server) emits stripped data, not raw chunked frame. And http.ServerResponse (sends to client) does not Keep-Alive, Content-Length is not needed.

[koichik]

@indexzero Can this be closed?

[indexzero]

@ry @koichik Did this land in 0.4.x or 0.5.x?

[koichik]

By default, Node does not send chunked response to HTTP/1.0 client.
If and only if a user (node-http-proxy) specifies Transfer-Encoding explicitly, Node will do so.

So, I think that node-http-proxy should not set Transfer-Encoding header (and other hop-by-hop headers) for HTTP/1.0 client.

[indexzero]

@koichik Ok. But is this fix available in 0.4.x? Or is it only available in 0.5.x?

[koichik]

Since v0.1.21 (c5d8238) :-)

[koichik]

Closed by http-party/node-http-proxy#125