Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

when a HTTP header overflow error occurs, instead of a generic 400 response, return 431 Request Header Fields Too Large #25605

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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',
mcollina marked this conversation as resolved.
Show resolved Hide resolved
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' ?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a better way to do this check, maybe with out a magic string? I see it's referenced in http_parser.c code here but not in lib.

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()));
}));