Skip to content

Commit bcf2886

Browse files
albertstillmcollina
authored andcommitted
http: return HTTP 431 on HPE_HEADER_OVERFLOW error
Instead of returning a generic 400 response when the max header size is reached, return a 431 Request Header Fields Too Large. This is a semver-major because it changes the HTTP status code for requests that trigger the header overflow error. PR-URL: #25605 Fixes: #25528 Refs: https://tools.ietf.org/html/rfc6585#section-5 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent a861add commit bcf2886

File tree

3 files changed

+62
-3
lines changed

3 files changed

+62
-3
lines changed

doc/api/http.md

+9-2
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,10 @@ changes:
829829
description: The `rawPacket` is the current buffer that just parsed. Adding
830830
this buffer to the error object of `'clientError'` event is to
831831
make it possible that developers can log the broken packet.
832+
- version: REPLACEME
833+
pr-url: https://github.com/nodejs/node/pull/25605
834+
description: The default behavior will return a 431 Request Header
835+
Fields Too Large if a HPE_HEADER_OVERFLOW error occurs.
832836
-->
833837

834838
* `exception` {Error}
@@ -839,8 +843,10 @@ Listener of this event is responsible for closing/destroying the underlying
839843
socket. For example, one may wish to more gracefully close the socket with a
840844
custom HTTP response instead of abruptly severing the connection.
841845

842-
Default behavior is to close the socket with an HTTP '400 Bad Request' response
843-
if possible, otherwise the socket is immediately destroyed.
846+
Default behavior is to try close the socket with a HTTP '400 Bad Request',
847+
or a HTTP '431 Request Header Fields Too Large' in the case of a
848+
[`HPE_HEADER_OVERFLOW`][] error. If the socket is not writable it is
849+
immediately destroyed.
844850

845851
`socket` is the [`net.Socket`][] object that the error originated from.
846852

@@ -2171,3 +2177,4 @@ not abort the request or do anything besides add a `'timeout'` event.
21712177
[`url.parse()`]: url.html#url_url_parse_urlstring_parsequerystring_slashesdenotehost
21722178
[Readable Stream]: stream.html#stream_class_stream_readable
21732179
[Stream]: stream.html#stream_stream
2180+
[`HPE_HEADER_OVERFLOW`]: errors.html#errors_hpe_header_overflow

lib/_http_server.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -507,14 +507,19 @@ const noop = () => {};
507507
const badRequestResponse = Buffer.from(
508508
`HTTP/1.1 400 ${STATUS_CODES[400]}${CRLF}${CRLF}`, 'ascii'
509509
);
510+
const requestHeaderFieldsTooLargeResponse = Buffer.from(
511+
`HTTP/1.1 431 ${STATUS_CODES[431]}${CRLF}${CRLF}`, 'ascii'
512+
);
510513
function socketOnError(e) {
511514
// Ignore further errors
512515
this.removeListener('error', socketOnError);
513516
this.on('error', noop);
514517

515518
if (!this.server.emit('clientError', e, this)) {
516519
if (this.writable) {
517-
this.write(badRequestResponse);
520+
const response = e.code === 'HPE_HEADER_OVERFLOW' ?
521+
requestHeaderFieldsTooLargeResponse : badRequestResponse;
522+
this.write(response);
518523
}
519524
this.destroy(e);
520525
}
+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
'use strict';
2+
const assert = require('assert');
3+
const { createServer, maxHeaderSize } = require('http');
4+
const { createConnection } = require('net');
5+
const { expectsError, mustCall } = require('../common');
6+
7+
const CRLF = '\r\n';
8+
const DUMMY_HEADER_NAME = 'Cookie: ';
9+
const DUMMY_HEADER_VALUE = 'a'.repeat(
10+
// plus one is to make it 1 byte too big
11+
maxHeaderSize - DUMMY_HEADER_NAME.length - (2 * CRLF.length) + 1
12+
);
13+
const PAYLOAD_GET = 'GET /blah HTTP/1.1';
14+
const PAYLOAD = PAYLOAD_GET + CRLF +
15+
DUMMY_HEADER_NAME + DUMMY_HEADER_VALUE + CRLF.repeat(2);
16+
17+
const server = createServer();
18+
19+
server.on('connection', mustCall((socket) => {
20+
socket.on('error', expectsError({
21+
type: Error,
22+
message: 'Parse Error',
23+
code: 'HPE_HEADER_OVERFLOW',
24+
bytesParsed: maxHeaderSize + PAYLOAD_GET.length,
25+
rawPacket: Buffer.from(PAYLOAD)
26+
}));
27+
}));
28+
29+
server.listen(0, mustCall(() => {
30+
const c = createConnection(server.address().port);
31+
let received = '';
32+
33+
c.on('connect', mustCall(() => {
34+
c.write(PAYLOAD);
35+
}));
36+
c.on('data', mustCall((data) => {
37+
received += data.toString();
38+
}));
39+
c.on('end', mustCall(() => {
40+
assert.strictEqual(
41+
received,
42+
'HTTP/1.1 431 Request Header Fields Too Large\r\n\r\n'
43+
);
44+
c.end();
45+
}));
46+
c.on('close', mustCall(() => server.close()));
47+
}));

0 commit comments

Comments
 (0)