Skip to content

Commit 9645d0e

Browse files
committed
http: fix parser memory leak on double response
When a server sends two complete HTTP responses in a single TCP chunk, the HTTP client code detects the double response and destroys the socket at lib/_http_client.js:695. However, the parser cleanup that normally happens in socketOnData() fails because it checks parser.incoming.complete, and parser.incoming now points to the incomplete second response that will never finish. This leaves the parser object allocated forever. The fix immediately frees the parser when a double response is detected, before destroying the socket. This ensures the parser is properly released even though it's in an invalid state. Fixes: #60025
1 parent bfc81ca commit 9645d0e

File tree

2 files changed

+100
-0
lines changed

2 files changed

+100
-0
lines changed

lib/_http_client.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,17 @@ function parserOnIncomingClient(res, shouldKeepAlive) {
691691
if (req.res) {
692692
// We already have a response object, this means the server
693693
// sent a double response.
694+
const parser = socket.parser;
694695
socket.destroy();
696+
697+
// Free the parser immediately to prevent memory leak (issue #60025).
698+
// The parser is in an invalid state with a partial second response
699+
// that will never complete, so we must clean it up here.
700+
if (parser) {
701+
parser.finish();
702+
freeParser(parser, req, socket);
703+
}
704+
695705
return 0; // No special treatment.
696706
}
697707
req.res = res;
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
'use strict';
2+
3+
// Test for memory leak when server sends double HTTP response
4+
// Refs: https://github.com/nodejs/node/issues/60025
5+
6+
const common = require('../common');
7+
const assert = require('assert');
8+
const http = require('http');
9+
const net = require('net');
10+
11+
// This test creates a scenario where the server sends two complete HTTP
12+
// responses in a single TCP chunk, which puts the HTTP parser in an invalid
13+
// state. Without the fix, the parser is never freed because the cleanup
14+
// logic checks parser.incoming.complete, but parser.incoming points to the
15+
// incomplete second response that will never finish.
16+
17+
async function testDoubleResponseLeak() {
18+
const iterations = 1000;
19+
const memBefore = process.memoryUsage().heapUsed;
20+
21+
for (let i = 0; i < iterations; i++) {
22+
await new Promise((resolve) => {
23+
// Create a raw TCP server that will send double HTTP response
24+
const server = net.createServer((socket) => {
25+
// Send two complete HTTP responses in one chunk
26+
socket.write(
27+
'HTTP/1.1 200 OK\r\n' +
28+
'Content-Length: 5\r\n' +
29+
'\r\n' +
30+
'first' +
31+
'HTTP/1.1 200 OK\r\n' +
32+
'Content-Length: 6\r\n' +
33+
'\r\n' +
34+
'second'
35+
);
36+
socket.end();
37+
});
38+
39+
server.listen(0, common.mustCall(() => {
40+
const req = http.get(`http://127.0.0.1:${server.address().port}`);
41+
42+
req.on('response', common.mustCall((res) => {
43+
res.resume();
44+
res.on('end', () => {
45+
// Response ended normally
46+
});
47+
}));
48+
49+
req.on('error', () => {
50+
// Expected error due to socket destruction on double response
51+
});
52+
53+
req.on('close', () => {
54+
server.close(() => {
55+
resolve();
56+
});
57+
});
58+
}));
59+
});
60+
61+
// Force GC every 100 iterations to verify parsers are being freed
62+
if (i % 100 === 0 && global.gc) {
63+
global.gc();
64+
await new Promise(setImmediate);
65+
}
66+
}
67+
68+
if (global.gc) {
69+
global.gc();
70+
await new Promise(setImmediate);
71+
}
72+
73+
const memAfter = process.memoryUsage().heapUsed;
74+
const growth = memAfter - memBefore;
75+
const growthMB = growth / 1024 / 1024;
76+
77+
console.log(`Memory growth: ${growthMB.toFixed(2)} MB`);
78+
79+
// With the fix, memory growth should be minimal (< 10 MB for 1000 iterations)
80+
// Without the fix, each iteration leaks a parser (~500 bytes + buffers),
81+
// leading to growth of 50+ MB
82+
assert.ok(growthMB < 10,
83+
`Excessive memory growth: ${growthMB.toFixed(2)} MB (expected < 10 MB)`);
84+
}
85+
86+
(async () => {
87+
console.log('Testing HTTP client double response memory leak...');
88+
await testDoubleResponseLeak();
89+
console.log('Test passed!');
90+
})().catch(common.mustNotCall());

0 commit comments

Comments
 (0)