Node: When http header size limit is reached, instead of a generic 400 response, return 431 Request Header Fields Too Large

[albertstill]

We overlooked a Node security release in November 2018 that downsized the max http header size limit to 8192 bytes.

After a Node bump we occasionally saw 400s in production due to large cookies but the response contained no body or interesting headers, all we got is a generic 400 http error code. Is there a good reason for this?

We solved the problem eventually but it would of been nice if the HTTP response body gave text explanation of the error. For example Max HTTP header size of ${maxHttpHeaderSize} reached.

Thanks.

[bnoordhuis]

For example Max HTTP header size of ${maxHttpHeaderSize} reached.

Including the actual size limit might leak mildly sensitive information. I say "mildly" because you can binary-search it with multiple requests but why make life easier for an attacker?

[albertstill]

Since I posted this issue, I discovered there is a HTTP error code for this exact case, 431 Request Header Fields Too Large. This would be the perfect solution and mean we can leave out the response body suggestion.

This would also be really valuable in CDN/logging reporting tools as we can pinpoint exactly how many clients are getting bloated cookies and triggering 431's.

[albertstill]

Furthermore 431 exists in Node's http.STATUS_CODES already, i.e node -p "require('http').STATUS_CODES[431]" prints Request Header Fields Too Large

[albertstill]

Also seems the default 400 behaviour is documented in the Event: 'clientError' documentation.

Default behavior is to close the socket with an HTTP '400 Bad Request' response if possible, otherwise the socket is immediately destroyed.

The following code would cause a server to behave how I wanted, using the HPE_HEADER_OVERFLOW error code.

server.on('clientError', (err, socket) => {
  if (err.code === 'HPE_HEADER_OVERFLOW') {
    socket.end('HTTP/1.1 431 Request Header Fields Too Large\r\n\r\n');
    return;
  }

  socket.end('HTTP/1.1 400 Bad Request\r\n\r\n');
});

Is it intentional the consumer has to handle this mapping themselves? Seems a sensible default but I understand if this is a design principle of Node.

[sam-github]

431 sounds pretty reasonable to me, @nodejs/http @mcollina, would you agree?

[mcollina]

I’m OK in using a 431, it seems a good idea! I think we might not want to change the default, and change this in a semver-major PR. Would you like to open one?

[albertstill]

I'd love to give the PR a crack, I'm reading the contributing guide now and should have a PR within 24 hours, I've located the code responsible for the 400.

[albertstill]

Here is the PR @sam-github @mcollina https://github.com/nodejs/node/pull/25605