Skip to content

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

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.

Sign up for GitHub

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

Closed
albertstill wants to merge 2 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50b7711
lib,test: return HTTP 431 on HPE_HEADER_OVERFLOW error
albertstill Jan 21, 2019
cd1e61c
doc: http.Server clientError default behavior slightly changed
albertstill Jan 21, 2019
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
Next Next commit
lib,test: 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
  • Loading branch information
@albertstill
albertstill committed Feb 1, 2019
commit 50b7711adada3c662a9cb33a085008e18ea67c2f
7 changes: 6 additions & 1 deletion lib/_http_server.js
View file
Open in desktop
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
Copy Markdown
Contributor Author

@albertstill albertstill Jan 21, 2019

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
View file
Open in desktop
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()));
}));