Skip to content

Commit

Permalink
http: return HTTP 431 on HPE_HEADER_OVERFLOW error
Browse files Browse the repository at this point in the history
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 <matteo.collina@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
albertstill authored and mcollina committed Feb 1, 2019
1 parent a861add commit bcf2886
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 3 deletions.
11 changes: 9 additions & 2 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,10 @@ changes:
description: The `rawPacket` is the current buffer that just parsed. Adding
this buffer to the error object of `'clientError'` event is to
make it possible that developers can log the broken packet.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/25605
description: The default behavior will return a 431 Request Header
Fields Too Large if a HPE_HEADER_OVERFLOW error occurs.
-->

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

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

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

Expand Down Expand Up @@ -2171,3 +2177,4 @@ not abort the request or do anything besides add a `'timeout'` event.
[`url.parse()`]: url.html#url_url_parse_urlstring_parsequerystring_slashesdenotehost
[Readable Stream]: stream.html#stream_class_stream_readable
[Stream]: stream.html#stream_stream
[`HPE_HEADER_OVERFLOW`]: errors.html#errors_hpe_header_overflow
7 changes: 6 additions & 1 deletion lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -507,14 +507,19 @@ const noop = () => {};
const badRequestResponse = Buffer.from(
`HTTP/1.1 400 ${STATUS_CODES[400]}${CRLF}${CRLF}`, 'ascii'
);
const requestHeaderFieldsTooLargeResponse = Buffer.from(
`HTTP/1.1 431 ${STATUS_CODES[431]}${CRLF}${CRLF}`, 'ascii'
);
function socketOnError(e) {
// Ignore further errors
this.removeListener('error', socketOnError);
this.on('error', noop);

if (!this.server.emit('clientError', e, this)) {
if (this.writable) {
this.write(badRequestResponse);
const response = e.code === 'HPE_HEADER_OVERFLOW' ?
requestHeaderFieldsTooLargeResponse : badRequestResponse;
this.write(response);
}
this.destroy(e);
}
Expand Down
47 changes: 47 additions & 0 deletions test/parallel/test-http-header-overflow.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
'use strict';
const assert = require('assert');
const { createServer, maxHeaderSize } = require('http');
const { createConnection } = require('net');
const { expectsError, mustCall } = require('../common');

const CRLF = '\r\n';
const DUMMY_HEADER_NAME = 'Cookie: ';
const DUMMY_HEADER_VALUE = 'a'.repeat(
// plus one is to make it 1 byte too big
maxHeaderSize - DUMMY_HEADER_NAME.length - (2 * CRLF.length) + 1
);
const PAYLOAD_GET = 'GET /blah HTTP/1.1';
const PAYLOAD = PAYLOAD_GET + CRLF +
DUMMY_HEADER_NAME + DUMMY_HEADER_VALUE + CRLF.repeat(2);

const server = createServer();

server.on('connection', mustCall((socket) => {
socket.on('error', expectsError({
type: Error,
message: 'Parse Error',
code: 'HPE_HEADER_OVERFLOW',
bytesParsed: maxHeaderSize + PAYLOAD_GET.length,
rawPacket: Buffer.from(PAYLOAD)
}));
}));

server.listen(0, mustCall(() => {
const c = createConnection(server.address().port);
let received = '';

c.on('connect', mustCall(() => {
c.write(PAYLOAD);
}));
c.on('data', mustCall((data) => {
received += data.toString();
}));
c.on('end', mustCall(() => {
assert.strictEqual(
received,
'HTTP/1.1 431 Request Header Fields Too Large\r\n\r\n'
);
c.end();
}));
c.on('close', mustCall(() => server.close()));
}));

0 comments on commit bcf2886

Please sign in to comment.