Skip to content

Commit bbdea56

Browse files
committed
http: reject responses exceeding header limit
HTTP client parsers continued parsing headers after maxHeadersCount while omitting excess fields from IncomingMessage. Reject the response instead so framing headers cannot be hidden from the application. Assisted-by: Pi Signed-off-by: Matteo Collina <hello@matteocollina.com>
1 parent 29667e0 commit bbdea56

4 files changed

Lines changed: 33 additions & 6 deletions

File tree

‎src/node_http_parser.cc‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,10 +1040,6 @@ class Parser : public AsyncWrap, public StreamListener {
10401040
}
10411041

10421042
int TrackHeaderPair() {
1043-
if (parser_.type != HTTP_REQUEST) {
1044-
return 0;
1045-
}
1046-
10471043
header_pairs_ += 2;
10481044

10491045
if (max_header_pairs_ < 0) {

‎test/parallel/test-http-max-headers-count-overflow.js‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,34 @@ server.listen(0, common.mustCall(() => {
3232
}));
3333
}));
3434
}));
35+
36+
function testResponseOverflow(maxHeadersCount) {
37+
const max = maxHeadersCount ?? 1000;
38+
const responseServer = net.createServer(common.mustCall((socket) => {
39+
socket.once('data', common.mustCall(() => {
40+
let response = 'HTTP/1.1 200 OK\r\n';
41+
for (let i = 0; i < max; i++) {
42+
response += `X-${i}: a\r\n`;
43+
}
44+
socket.end(response +
45+
'Transfer-Encoding: chunked\r\n' +
46+
'\r\n0\r\n\r\n');
47+
}));
48+
}));
49+
50+
responseServer.listen(0, common.mustCall(() => {
51+
const req = http.get({
52+
port: responseServer.address().port,
53+
}, common.mustNotCall());
54+
if (maxHeadersCount !== undefined) {
55+
req.maxHeadersCount = maxHeadersCount;
56+
}
57+
req.on('error', common.mustCall((err) => {
58+
assert.strictEqual(err.code, 'HPE_HEADER_OVERFLOW');
59+
responseServer.close();
60+
}));
61+
}));
62+
}
63+
64+
testResponseOverflow(2);
65+
testResponseOverflow();

‎test/parallel/test-http-max-headers-count.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ server.maxHeadersCount = max;
6767

6868
server.listen(0, common.mustCall(() => {
6969
const clientMaxAndExpected = [ // for client
70-
[20, 20],
70+
[104, 104],
7171
[1200, 104],
7272
[0, N + 4], // Host and Connection
7373
];

‎test/parallel/test-https-max-headers-count.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ server.maxHeadersCount = max;
5656

5757
server.listen(0, common.mustCall(() => {
5858
const clientMaxAndExpected = [ // for client
59-
[20, 20],
59+
[104, 104],
6060
[1200, 104],
6161
[0, N + 4], // Host and Connection
6262
];

0 commit comments

Comments
 (0)