FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http: don't destroy completed request · nodejs/node@cc5c8e0 · GitHub

/ node Public

Commit cc5c8e0

Browse files
ronag
authored and
codebytere
committed
http: don't destroy completed request
Calling destroy() on a completed ClientRequest, i.e. once 'close' will be emitted should be a noop. Also before emitting 'close' destroyed === true. Fixes: #32851 PR-URL: #33120 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent e1e57a4 commit cc5c8e0

10 files changed

Lines changed: 60 additions & 5 deletions

‎lib/_http_client.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,8 @@ function socketCloseListener() {
413413
// the `socketOnData`.
414414
const parser = socket.parser;
415415
const res = req.res;
416+
417+
req.destroyed = true;
416418
if (res) {
417419
// Socket closed before we emitted 'end' below.
418420
if (!res.complete) {
@@ -667,7 +669,9 @@ function responseKeepAlive(req) {
667669
// handlers have a chance to run.
668670
defaultTriggerAsyncIdScope(asyncId, process.nextTick, emitFreeNT, req);
669671

672+
req.destroyed = true;
670673
if (req.res) {
674+
req.res.destroyed = true;
671675
// Detach socket from IncomingMessage to avoid destroying the freed
672676
// socket in IncomingMessage.destroy().
673677
req.res.socket = null;
@@ -713,7 +717,6 @@ function requestOnPrefinish() {
713717
function emitFreeNT(req) {
714718
req.emit('close');
715719
if (req.res) {
716-
req.res.destroyed = true;
717720
req.res.emit('close');
718721
}
719722

‎test/parallel/test-http-client-agent-abort-close-event.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict';
22
const common = require('../common');
3+
const assert = require('assert');
34
const http = require('http');
45

56
const server = http.createServer(common.mustNotCall());
@@ -16,6 +17,7 @@ server.listen(0, common.mustCall(() => {
1617
.on('socket', common.mustNotCall())
1718
.on('response', common.mustNotCall())
1819
.on('close', common.mustCall(() => {
20+
assert.strictEqual(req.destroyed, true);
1921
server.close();
2022
keepAliveAgent.destroy();
2123
}))

‎test/parallel/test-http-client-agent-end-close-event.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict';
22
const common = require('../common');
3+
const assert = require('assert');
34
const http = require('http');
45

56
const server = http.createServer(common.mustCall((req, res) => {
@@ -18,6 +19,7 @@ server.listen(0, common.mustCall(() => {
1819
.on('response', common.mustCall((res) => {
1920
res
2021
.on('close', common.mustCall(() => {
22+
assert.strictEqual(req.destroyed, true);
2123
server.close();
2224
keepAliveAgent.destroy();
2325
}))

‎test/parallel/test-http-client-close-event.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ server.listen(0, common.mustCall(() => {
2222
}));
2323

2424
req.on('close', common.mustCall(() => {
25+
assert.strictEqual(req.destroyed, true);
2526
assert.strictEqual(errorEmitted, true);
2627
server.close();
2728
}));

‎test/parallel/test-http-client-override-global-agent.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ function makeRequest() {
2121
const req = http.get({
2222
port: server.address().port
2323
});
24-
req.on('close', () =>
25-
server.close());
24+
req.on('close', () => {
25+
assert.strictEqual(req.destroyed, true);
26+
server.close();
27+
});
2628
}

‎test/parallel/test-http-client-set-timeout.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ server.listen(0, mustCall(() => {
3838
}));
3939

4040
req.on('close', mustCall(() => {
41+
strictEqual(req.destroyed, true);
4142
server.close();
4243
}));
4344

‎test/parallel/test-http-client-timeout-event.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
'use strict';
2323
const common = require('../common');
24+
const assert = require('assert');
2425
const http = require('http');
2526

2627
const options = {
@@ -38,7 +39,10 @@ server.listen(0, options.host, function() {
3839
req.on('error', function() {
3940
// This space is intentionally left blank
4041
});
41-
req.on('close', common.mustCall(() => server.close()));
42+
req.on('close', common.mustCall(() => {
43+
assert.strictEqual(req.destroyed, true);
44+
server.close();
45+
}));
4246

4347
req.setTimeout(1);
4448
req.on('timeout', common.mustCall(() => {

‎test/parallel/test-http-client-timeout-option.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@ server.listen(0, options.host, function() {
2323
req.on('error', function() {
2424
// This space is intentionally left blank
2525
});
26-
req.on('close', common.mustCall(() => server.close()));
26+
req.on('close', common.mustCall(() => {
27+
assert.strictEqual(req.destroyed, true);
28+
server.close();
29+
}));
2730

2831
let timeout_events = 0;
2932
req.on('timeout', common.mustCall(() => timeout_events += 1));

‎test/parallel/test-http-client-timeout.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ server.listen(0, options.host, function() {
4141
// This space intentionally left blank
4242
});
4343
req.on('close', function() {
44+
assert.strictEqual(req.destroyed, true);
4445
server.close();
4546
});
4647
function destroy() {

‎test/parallel/test-http-keepalive-free.js‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
6+
const http = require('http');
7+
8+
for (const method of ['abort', 'destroy']) {
9+
const server = http.createServer(common.mustCall((req, res) => {
10+
res.end(req.url);
11+
}));
12+
server.listen(0, common.mustCall(() => {
13+
const agent = http.Agent({ keepAlive: true });
14+
15+
const req = http
16+
.request({
17+
port: server.address().port,
18+
agent
19+
})
20+
.on('socket', common.mustCall((socket) => {
21+
socket.on('free', common.mustCall());
22+
}))
23+
.on('response', common.mustCall((res) => {
24+
assert.strictEqual(req.destroyed, false);
25+
res.on('end', () => {
26+
assert.strictEqual(req.destroyed, true);
27+
req[method]();
28+
assert.strictEqual(req.socket.destroyed, false);
29+
agent.destroy();
30+
server.close();
31+
}).resume();
32+
}))
33+
.end();
34+
assert.strictEqual(req.destroyed, false);
35+
}));
36+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL