fix: accept pre-response data on CONNECT #284
Conversation
szmarczak
changed the title
src/chain.ts
Outdated
| @@ -59,7 +59,8 @@ export const chain = ( | |||
| }: ChainOpts, | |||
| ): void => { | |||
| if (head && head.length > 0) { | |||
| throw new Error(`Unexpected data on CONNECT: ${head.length} bytes`); | |||
| sourceSocket.end(createHttpResponse(400, 'CONNECT payload must be empty')); | |||
There was a problem hiding this comment.
Is this really a non-standard response? I thought it's okay, and that's why Node supports it. Anyway, please at least add a comment why it's okay to drop those silently. Shouldn't we at least log something, so that the server has a chance to track these?
There was a problem hiding this comment.
https://www.rfc-editor.org/rfc/rfc9110.html
The CONNECT method requests that the recipient establish a tunnel to the destination origin server identified by the request target and, if successful, thereafter restrict its behavior to blind forwarding of data, in both directions, until the tunnel is closed.
Note the if successful.
A CONNECT request message does not have content. The interpretation of data sent after the header section of the CONNECT request message is specific to the version of HTTP in use.
while the obsoleted spec says:
A payload within a CONNECT request message has no defined semantics;
sending a payload body on a CONNECT request might cause some existing
implementations to reject the request.
I think Node.js implemented head because it has no defined semantics (according to the obsoleted spec).
MDN docs also say that the method does not have body.
Sending data pre-response invalidates the entire socket e.g. when replying with 401 instead of 200, because the client thinks the tunnel is pending however on the server-side - because the CONNECT was invalid - parsers could still accept HTTP requests. Node.js in this case transfers ownership of the socket to the developer regardless of the response.
HTTP/2 says this:
After the initial HEADERS frame sent by each peer, all subsequent DATA frames correspond to data sent on the TCP connection.
However, HTTP/3 says this:
All DATA frames on the stream correspond to data sent or received on the TCP connection.
So HTTP/3 apparently says that pre-response data is valid.
There was a problem hiding this comment.
FYI when I was rewriting proxy-chain HTTP/3 spec wasn't even out yet.
There was a problem hiding this comment.
I reverted to the old behavior. PTAL.
szmarczak
changed the title
That's from upstream proxies. I don't think in a real case scenario this would ever come from a server, because after the connection is established, the data is sent in another packet, therefore it should be not possible for this to occur. I remember there was a case where the upstream proxy sent a valid HTTP response (non-200 however) to a CONNECT. I guess you're after backwards compatibility, so I have done that change. |
No description provided.