HTTP Upgrade IncomingMessage does not parse body

[mscdex]

Version

22.15.0

Platform

Linux

Subsystem

http

What steps will reproduce the bug?

'use strict';

const { createServer, request } = require('http');

createServer((req, res) => {
  res.writeHead(200).end();
}).on('upgrade', (req, reqSocket, head) => {
  console.log('Server Upgrade', req.headers);
  let buf = '';
  req.on('data', (str) => buf += str);
  req.on('close', () => {
    console.log(
      'Request close',
      'data', JSON.stringify(buf.trim()),
      'head', Buffer.from(head)
    );
  });
  req.setEncoding('utf8');
}).listen(0, '127.0.0.1', function() {
  const req = request({
    host: '127.0.0.1',
    port: this.address().port,
    method: 'POST',
    path: '/foo',
    headers: {
      Connection: 'Upgrade',
      Upgrade: 'db',
    },
  });
  req.on('upgrade', (res, socket, head) => {
    console.log(
      'Client Upgrade',
      'status', res.statusCode,
      'head', Buffer.from(head)
    );
  });
  setTimeout(() => {
    req.write('2');
    setTimeout(() => req.end(), 1000);
  }, 1000);
});

Output:

Server Upgrade {                                                                                                                                                                                                   
  connection: 'Upgrade',                                                                                                                                                                                           
  upgrade: 'db',                                                                                                                                                                                                   
  host: '127.0.0.1:39511',                                                                                                                                                                                         
  'transfer-encoding': 'chunked'                                                                                                                                                                                   
}                                                                                                                                                                                                                  
Request close data "" head <Buffer 31 0d 0a 32 0d 0a>

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

Reproduces every time.

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

The expected behavior is to have node parse the body like it normally would.

What do you see instead?

Node ignores the content or potentially puts some of it (unparsed -- especially evident in the case of a chunked encoded body) in the head argument passed to the 'upgrade' event.

Additional information

No response

[lpinca]

Isn't this expected? It is upgrading to a different protocol. The HTTP parser is freed after the 'upgrade' event.

[mscdex]

Isn't this expected?

Not to me. Node can tell if there is a body just like any normal HTTP request and so it should be able to parse that. This is very useful if you want to pass information to the server before switching protocols and without resorting to spamming the querystring or headers.

[lpinca]

FWIW, it worked like this since forever, and I think it makes sense. You can't tell beforehand if the body need to be used as data for the upgraded protocol. For example, even though the WebSocket protocol spec specifies that no data should be exchanged until the opening handshake is complete, a lot of implementations use the body of the handshake request as WebSocket data. I think this is also why the callback of the 'upgrade' events takes the head parameter.

[mscdex]

You can't tell beforehand if the body need to be used as data for the upgraded protocol.

Why would a client bother sending a body (in the HTTP sense indicated via Content-Length or Transfer-Encoding: chunked) if they could just send the same data as part of the upgraded protocol (whether it's before or after the server sends the 101 response)?

For example, even though the WebSocket protocol spec specifies that no data should be exchanged until the opening handshake is complete, a lot of implementations use the body of the handshake request as WebSocket data.

The difference is that WebSocket requests must be GET requests, which means they can never have a body, so there is no problem there.

It's only a problem for other/custom upgraded protocols that allow non-GET upgrade requests.

[lpinca]

Why would a client bother sending a body (in the HTTP sense indicated via Content-Length or Transfer-Encoding: chunked) if they could just send the same data as part of the upgraded protocol (whether it's before or after the server sends the 101 response)?

How and when do you decide to which protocol the incoming data belongs to?

The difference is that WebSocket requests must be GET requests, which means they can never have a body, so there is no problem there.

There are misbehaving clients that actually send data in the body of the initial GET request.

[mscdex]

How and when do you decide to which protocol the incoming data belongs to?

I see two scenarios:

1. A client has indicated their HTTP request includes a body. Any upgraded protocol data would come immediately after the end of the body.

2. A client has not indicated their HTTP request includes a body. Any upgraded protocol data would come immediately after the end of the client HTTP headers.

There are misbehaving clients that actually send data in the body of the initial GET request.

Perhaps there are, but how common is that? I would wager that browsers make up the bulk majority of WebSocket requests, which do not misbehave (especially in 2025).

Additionally, there should be a distinction between misbehaving WebSocket clients sending handshake data immediately after the HTTP headers and those who are indicating an HTTP body in their request (by setting the appropriate HTTP headers). The former does not matter because they never indicated there was a body to be parsed and the latter is a bit of a moot case because the client is just extra broken at that point (and if node really wanted to care about that kind of case, you could just check for that specific scenario -- GET + "Upgrade: Websocket" + HTTP body).

[lpinca]

I never investigated, but is the current behavior Node.js specific? Is there anything about this in HTTP spec? How are other runtimes / languages handling this?

[mscdex]

Is there anything about this in HTTP spec?

The current RFC says this about upgrade requests: (emphasis mine)

A client cannot begin using an upgraded protocol on the connection until it has completely sent the request message (i.e., the client can't change the protocol it is sending in the middle of a message).

Earlier in the same RFC it describes the definition of a "message", which includes everything from the first line containing (in the case of a request) the method and path to the optional HTTP trailers.

This seems to align with my expectation that the server should be processing any HTTP request body as normal and anything after that is processed as the upgraded protocol (if the server sends a 101).