Incorrect handling of Proxy-Connection: close

[devicenull]

Version

v20.13.1
OR
v22.14.0
OR
v23.11.0

Platform

Darwin xxx.local 24.3.0 Darwin Kernel Version 24.3.0: Thu Jan  2 20:24:23 PST 2025; root:xnu-11215.81.4~3/RELEASE_ARM64_T6020 arm64

(also reproduces on Linux)

Subsystem

http

What steps will reproduce the bug?

Given the following trivial HTTP server:

var http = require('http');

//create a server object:
const server = http.createServer(function (req, res) {
  res.write('Hello World!');
  res.end();
});

server.keepAliveTimeout = 61000;
server.headersTimeout = 62000;

server.listen(4444); 

You will also need two HTTP requests, it's probably possible to rig up a client to do this but I just used nc, so:

req1

GET / HTTP/1.1
Host: www.example.com
Connection: keep-alive
Proxy-Connection: close

req2

GET / HTTP/1.1
Host: www.example.com
Connection: keep-alive

Then, startup the HTTP server and use the following:

cat req1 | nc 127.0.0.1 4444

and compare to

cat req2 | nc 127.0.0.1 4444

How often does it reproduce? Is there a required condition?

Every time

What is the expected behavior? Why is that the expected behavior?

The expected behavior is for nodejs to keep the connection alive I expect to see:

HTTP/1.1 200 OK
Date: Tue, 01 Apr 2025 19:29:49 GMT
Connection: keep-alive
Keep-Alive: timeout=5
Transfer-Encoding: chunked

c
Hello World!
0

What do you see instead?

I get a response with a Connection: close header

HTTP/1.1 200 OK
Date: Tue, 01 Apr 2025 19:29:37 GMT
Connection: close
Transfer-Encoding: chunked

c
Hello World!
0

Additional information

The HTTP RFCs aren't completely clear on this - https://www.rfc-editor.org/rfc/rfc9110.html#name-connection suggests that proxy-connection should be removed (which is what we're now doing as a workaround). https://datatracker.ietf.org/doc/html/rfc9112#appendix-C.2.2 suggests that clients should not be sending it in the first place.

This was causing some issues for us with a nginx frontend - nginx was keeping the connection alive, so whenever a client sent a header with a "proxy-connection" header, the next request on that socket would fail and nginx would respond with a 400.

I do not expect node's HTTP server to be honoring proxy- headers absent some explicit configuration

[pimterry]

I don't think this is a Node bug (although it is a bit debatable).

Sending "proxy-connection" to a server is effectively the client saying "I think you are proxy who will forward this request. This is how I want you to manage the client-to-proxy part of this connection (close vs keep-alive etc)". Sending "Connection" is the modern alternative, intended to work for proxies and servers, and means "Here is how I want you to manage the direct connection between us". I.e. it's slightly more general, but is effectively defining the same thing for the same connection.

In all cases, sending both headers with different values is a bad idea - it's similar to just sending two connection headers with different values on the same request. That's your main problem here. In your specific scenario, this sounds like nginx bug. Nginx (and all other HTTP intermediaries) should never forward this header, as noted in the RFC you reference. Have you discussed this with them? Your workaround to drop this header before forwarding is the correct fix 👍 and that's what nginx should be doing automatically out of the box I think.

Clients are also not really supposed to send this header in the first place either, since Connection completely replaces it nowadays, but the curl developer says they tried to drop it and it is actually in use in quite a few legacy environments, and they received a big stream of bug reports, so the spec may be a bit optimistic about reality there.

Overall I'm not sure what the best behaviour for Node would be here. It's worth noting that Node doesn't ever actually know whether it's an end-server or an intermediary (there's plenty of Node-based HTTP proxies, and in a microservice world you can easily be both), and it does sound like there's a real world of legacy systems that do use this header, so dropping it might break things. Both behaviours are spec compliant anyway: we can ignore proxy-connection and be strict about only listening to connection, as in the spec, but we're also under no obligation to keep-alive regardless - it's completely valid to send connection: close at any time, so there's nothing invalid AFAICT about using proxy-connection as a signal that the client wants the connection closed/kept-alive and honouring that. The only strict rule is that we shouldn't forward it onwards elsewhere.