Skip to content

Commit 04372aa

Browse files
committed
http: prevent reuse after incomplete request destruction
Signed-off-by: Dayun <dlekdbs6530@gmail.com>
1 parent bb5cffc commit 04372aa

3 files changed

Lines changed: 229 additions & 0 deletions

File tree

lib/internal/streams/destroy.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,18 @@ function destroyer(stream, err) {
330330

331331
// TODO: Remove isRequest branches.
332332
if (isServerRequest(stream)) {
333+
const socket = stream.socket;
334+
const response = socket?._httpMessage;
335+
336+
if (response?.req === stream) {
337+
if (response.headersSent) {
338+
response.destroy();
339+
} else {
340+
response.shouldKeepAlive = false;
341+
response._last = true;
342+
}
343+
}
344+
333345
stream.socket = null;
334346
stream.destroy(err);
335347
} else if (isRequest(stream)) {
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const http = require('http');
6+
7+
const agent = new http.Agent({
8+
keepAlive: true,
9+
maxSockets: 1,
10+
});
11+
12+
let serverRequests = 0;
13+
let firstResponseReceived = false;
14+
let firstResponseConnection;
15+
let firstResponseAborted = false;
16+
let firstResponseClosed = false;
17+
let secondStarted = false;
18+
19+
const server = http.createServer(async (req, res) => {
20+
serverRequests++;
21+
22+
if (serverRequests === 1) {
23+
res.write('partial');
24+
25+
try {
26+
for await (const chunk of req) {
27+
throw new Error(`payload too large: ${chunk.length}`);
28+
}
29+
} catch {
30+
res.end('payload too large');
31+
}
32+
return;
33+
}
34+
35+
res.end('ok');
36+
});
37+
38+
server.listen(0, common.mustCall(() => {
39+
const port = server.address().port;
40+
41+
function startSecondRequest() {
42+
if (secondStarted) {
43+
return;
44+
}
45+
46+
secondStarted = true;
47+
48+
process.nextTick(() => {
49+
const second = http.request({
50+
port,
51+
method: 'GET',
52+
agent,
53+
}, common.mustCall((res) => {
54+
second.setTimeout(0);
55+
56+
assert.strictEqual(second.reusedSocket, false);
57+
58+
if (firstResponseReceived) {
59+
assert.strictEqual(firstResponseConnection, 'keep-alive');
60+
assert.strictEqual(firstResponseAborted, true);
61+
assert.strictEqual(firstResponseClosed, true);
62+
}
63+
64+
res.setEncoding('utf8');
65+
66+
let body = '';
67+
68+
res.on('data', (chunk) => {
69+
body += chunk;
70+
});
71+
72+
res.on('end', common.mustCall(() => {
73+
assert.strictEqual(body, 'ok');
74+
assert.strictEqual(serverRequests, 2);
75+
76+
agent.destroy();
77+
server.close();
78+
}));
79+
}));
80+
81+
second.setTimeout(
82+
common.platformTimeout(1000),
83+
common.mustNotCall('second request timed out'),
84+
);
85+
86+
second.end();
87+
});
88+
}
89+
90+
const first = http.request({
91+
port,
92+
method: 'POST',
93+
agent,
94+
}, (res) => {
95+
firstResponseReceived = true;
96+
firstResponseConnection = res.headers.connection;
97+
98+
res.on('end', common.mustNotCall());
99+
res.on('aborted', () => {
100+
firstResponseAborted = true;
101+
});
102+
res.on('error', common.expectsError({
103+
code: 'ECONNRESET',
104+
message: 'aborted',
105+
}));
106+
107+
res.on('close', () => {
108+
firstResponseClosed = true;
109+
startSecondRequest();
110+
});
111+
112+
res.resume();
113+
});
114+
115+
first.on('error', (err) => {
116+
switch (err.code) {
117+
case 'ECONNRESET':
118+
case 'ECONNABORTED':
119+
case 'EPIPE':
120+
break;
121+
default:
122+
throw err;
123+
}
124+
125+
if (!firstResponseReceived) {
126+
startSecondRequest();
127+
}
128+
});
129+
130+
first.end(Buffer.alloc(1_000_000));
131+
}));
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const http = require('http');
6+
7+
const agent = new http.Agent({
8+
keepAlive: true,
9+
maxSockets: 1,
10+
});
11+
12+
let serverRequests = 0;
13+
14+
const server = http.createServer(async (req, res) => {
15+
serverRequests++;
16+
17+
if (serverRequests === 1) {
18+
try {
19+
for await (const chunk of req) {
20+
throw new Error(`payload too large: ${chunk.length}`);
21+
}
22+
} catch {
23+
res.end('payload too large');
24+
}
25+
return;
26+
}
27+
28+
res.end('ok');
29+
});
30+
31+
server.listen(0, common.mustCall(() => {
32+
const first = http.request({
33+
port: server.address().port,
34+
method: 'POST',
35+
agent,
36+
}, common.mustCall((res) => {
37+
assert.strictEqual(res.headers.connection, 'close');
38+
res.resume();
39+
40+
res.on('end', common.mustCall(() => {
41+
process.nextTick(common.mustCall(() => {
42+
const second = http.request({
43+
port: server.address().port,
44+
method: 'GET',
45+
agent,
46+
}, common.mustCall((res) => {
47+
second.setTimeout(0);
48+
assert.strictEqual(second.reusedSocket, false);
49+
res.setEncoding('utf8');
50+
51+
let body = '';
52+
53+
res.on('data', (chunk) => {
54+
body += chunk;
55+
});
56+
57+
res.on('end', common.mustCall(() => {
58+
assert.strictEqual(body, 'ok');
59+
assert.strictEqual(serverRequests, 2);
60+
61+
agent.destroy();
62+
server.close();
63+
}));
64+
}));
65+
66+
second.setTimeout(common.platformTimeout(1000), () => {
67+
assert.fail('second request timed out');
68+
});
69+
70+
second.end();
71+
}));
72+
}));
73+
}));
74+
first.on('error', (err) => {
75+
switch (err.code) {
76+
case 'ECONNRESET':
77+
case 'ECONNABORTED':
78+
case 'EPIPE':
79+
break;
80+
default:
81+
assert.fail(`Unexpected error code ${err.code}`);
82+
}
83+
});
84+
85+
first.end(Buffer.alloc(1_000_000));
86+
}));

0 commit comments

Comments
 (0)