Skip to content

Commit 9573192

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

3 files changed

Lines changed: 185 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: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
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+
res.write('partial');
19+
20+
try {
21+
for await (const chunk of req) {
22+
throw new Error(`payload too large: ${chunk.length}`);
23+
}
24+
} catch {
25+
res.end('payload too large');
26+
}
27+
return;
28+
}
29+
30+
res.end('ok');
31+
});
32+
33+
server.listen(0, common.mustCall(() => {
34+
const first = http.request({
35+
port: server.address().port,
36+
method: 'POST',
37+
agent,
38+
}, common.mustCall((res) => {
39+
assert.strictEqual(res.headers.connection, 'keep-alive');
40+
41+
res.on('end', common.mustNotCall());
42+
res.on('aborted', common.mustCall());
43+
res.on('error', common.expectsError({
44+
code: 'ECONNRESET',
45+
message: 'aborted',
46+
}));
47+
48+
res.on('close', common.mustCall(() => {
49+
process.nextTick(common.mustCall(() => {
50+
const second = http.request({
51+
port: server.address().port,
52+
method: 'GET',
53+
agent,
54+
}, common.mustCall((res) => {
55+
second.setTimeout(0);
56+
assert.strictEqual(second.reusedSocket, false);
57+
res.setEncoding('utf8');
58+
59+
let body = '';
60+
61+
res.on('data', (chunk) => {
62+
body += chunk;
63+
});
64+
65+
res.on('end', common.mustCall(() => {
66+
assert.strictEqual(body, 'ok');
67+
assert.strictEqual(serverRequests, 2);
68+
69+
agent.destroy();
70+
server.close();
71+
}));
72+
}));
73+
74+
second.setTimeout(common.platformTimeout(1000), () => {
75+
assert.fail('second request timed out');
76+
});
77+
78+
second.end();
79+
}));
80+
}));
81+
82+
res.resume();
83+
}));
84+
85+
first.on('error', (err) => {
86+
switch (err.code) {
87+
case 'ECONNRESET':
88+
case 'ECONNABORTED':
89+
case 'EPIPE':
90+
break;
91+
default:
92+
assert.fail(`Unexpected error code ${err.code}`);
93+
}
94+
});
95+
96+
first.end(Buffer.alloc(1_000_000));
97+
}));
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
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+
75+
first.end(Buffer.alloc(1_000_000));
76+
}));

0 commit comments

Comments
 (0)