Skip to content

Commit b634d4b

Browse files
ronagcodebytere
ronag
authored and
codebytere
committed
http: set IncomingMessage.destroyed
IncomingMessage is a Readable stream and should properly set the destroyed property. PR-URL: #33131 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
1 parent 736ca65 commit b634d4b

10 files changed

+94
-4
lines changed

‎lib/_http_client.js

+4
Original file line numberDiff line numberDiff line change
@@ -422,10 +422,12 @@ function socketCloseListener() {
422422
req.emit('close');
423423
if (!res.aborted && res.readable) {
424424
res.on('end', function() {
425+
this.destroyed = true;
425426
this.emit('close');
426427
});
427428
res.push(null);
428429
} else {
430+
res.destroyed = true;
429431
res.emit('close');
430432
}
431433
} else {
@@ -539,6 +541,7 @@ function socketOnData(d) {
539541
socket.readableFlowing = null;
540542

541543
req.emit(eventName, res, socket, bodyHead);
544+
req.destroyed = true;
542545
req.emit('close');
543546
} else {
544547
// Requested Upgrade or used CONNECT method, but have no handler.
@@ -710,6 +713,7 @@ function requestOnPrefinish() {
710713
function emitFreeNT(req) {
711714
req.emit('close');
712715
if (req.res) {
716+
req.res.destroyed = true;
713717
req.res.emit('close');
714718
}
715719

‎lib/_http_incoming.js

+2
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,8 @@ IncomingMessage.prototype._read = function _read(n) {
119119
// any messages, before ever calling this. In that case, just skip
120120
// it, since something else is destroying this connection anyway.
121121
IncomingMessage.prototype.destroy = function destroy(error) {
122+
// TODO(ronag): Implement in terms of _destroy
123+
this.destroyed = true;
122124
if (this.socket)
123125
this.socket.destroy(error);
124126
};

‎lib/_http_server.js

+8-1
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,10 @@ function onServerResponseClose() {
205205
// Ergo, we need to deal with stale 'close' events and handle the case
206206
// where the ServerResponse object has already been deconstructed.
207207
// Fortunately, that requires only a single if check. :-)
208-
if (this._httpMessage) this._httpMessage.emit('close');
208+
if (this._httpMessage) {
209+
this._httpMessage.destroyed = true;
210+
this._httpMessage.emit('close');
211+
}
209212
}
210213

211214
ServerResponse.prototype.assignSocket = function assignSocket(socket) {
@@ -534,6 +537,7 @@ function abortIncoming(incoming) {
534537
while (incoming.length) {
535538
const req = incoming.shift();
536539
req.aborted = true;
540+
req.destroyed = true;
537541
req.emit('aborted');
538542
req.emit('close');
539543
}
@@ -660,11 +664,13 @@ function clearIncoming(req) {
660664
if (parser && parser.incoming === req) {
661665
if (req.readableEnded) {
662666
parser.incoming = null;
667+
req.destroyed = true;
663668
req.emit('close');
664669
} else {
665670
req.on('end', clearIncoming);
666671
}
667672
} else {
673+
req.destroyed = true;
668674
req.emit('close');
669675
}
670676
}
@@ -708,6 +714,7 @@ function resOnFinish(req, res, socket, state, server) {
708714
}
709715

710716
function emitCloseNT(self) {
717+
self.destroyed = true;
711718
self.emit('close');
712719
}
713720

‎test/parallel/test-http-client-res-destroyed.js

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const http = require('http');
5+
6+
{
7+
const server = http.createServer(common.mustCall((req, res) => {
8+
res.end('asd');
9+
}));
10+
11+
server.listen(0, common.mustCall(() => {
12+
http.get({
13+
port: server.address().port
14+
}, common.mustCall((res) => {
15+
assert.strictEqual(res.destroyed, false);
16+
res.destroy();
17+
assert.strictEqual(res.destroyed, true);
18+
res.on('close', common.mustCall(() => {
19+
server.close();
20+
}));
21+
}));
22+
}));
23+
}
24+
25+
{
26+
const server = http.createServer(common.mustCall((req, res) => {
27+
res.end('asd');
28+
}));
29+
30+
server.listen(0, common.mustCall(() => {
31+
http.get({
32+
port: server.address().port
33+
}, common.mustCall((res) => {
34+
assert.strictEqual(res.destroyed, false);
35+
res.on('close', common.mustCall(() => {
36+
assert.strictEqual(res.destroyed, true);
37+
server.close();
38+
})).resume();
39+
}));
40+
}));
41+
}

‎test/parallel/test-http-connect-req-res.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,10 @@ server.listen(0, common.mustCall(function() {
3333
path: 'example.com:443'
3434
}, common.mustNotCall());
3535

36-
req.on('close', common.mustCall());
36+
assert.strictEqual(req.destroyed, false);
37+
req.on('close', common.mustCall(() => {
38+
assert.strictEqual(req.destroyed, true);
39+
}));
3740

3841
req.on('connect', common.mustCall(function(res, socket, firstBodyChunk) {
3942
console.error('Client got CONNECT request');

‎test/parallel/test-http-connect.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,10 @@ server.listen(0, common.mustCall(() => {
6262
assert.strictEqual(socket._httpMessage, req);
6363
}));
6464

65-
req.on('close', common.mustCall());
65+
assert.strictEqual(req.destroyed, false);
66+
req.on('close', common.mustCall(() => {
67+
assert.strictEqual(req.destroyed, true);
68+
}));
6669

6770
req.on('connect', common.mustCall((res, socket, firstBodyChunk) => {
6871
// Make sure this request got removed from the pool.

‎test/parallel/test-http-pause-resume-one-end.js

+8-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
'use strict';
2323
const common = require('../common');
2424
const http = require('http');
25+
const assert = require('assert');
2526

2627
const server = http.Server(function(req, res) {
2728
res.writeHead(200, { 'Content-Type': 'text/plain' });
@@ -43,6 +44,12 @@ server.listen(0, common.mustCall(function() {
4344
});
4445
}));
4546

46-
res.on('end', common.mustCall());
47+
res.on('end', common.mustCall(() => {
48+
assert.strictEqual(res.destroyed, false);
49+
}));
50+
assert.strictEqual(res.destroyed, false);
51+
res.on('close', common.mustCall(() => {
52+
assert.strictEqual(res.destroyed, true);
53+
}));
4754
}));
4855
}));

‎test/parallel/test-http-req-res-close.js

+12
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,25 @@ const server = http.Server(common.mustCall((req, res) => {
88
let resClosed = false;
99

1010
res.end();
11+
let resFinished = false;
1112
res.on('finish', common.mustCall(() => {
13+
resFinished = true;
14+
assert.strictEqual(resClosed, false);
15+
assert.strictEqual(res.destroyed, false);
1216
assert.strictEqual(resClosed, false);
1317
}));
18+
assert.strictEqual(req.destroyed, false);
1419
res.on('close', common.mustCall(() => {
1520
resClosed = true;
21+
assert.strictEqual(resFinished, true);
22+
assert.strictEqual(res.destroyed, true);
23+
}));
24+
assert.strictEqual(req.destroyed, false);
25+
req.on('end', common.mustCall(() => {
26+
assert.strictEqual(req.destroyed, false);
1627
}));
1728
req.on('close', common.mustCall(() => {
29+
assert.strictEqual(req.destroyed, true);
1830
assert.strictEqual(req._readableState.ended, true);
1931
}));
2032
res.socket.on('close', () => server.close());

‎test/parallel/test-http-response-close.js

+8
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
'use strict';
2323
const common = require('../common');
2424
const http = require('http');
25+
const assert = require('assert');
2526

2627
{
2728
const server = http.createServer(
@@ -39,7 +40,9 @@ const http = require('http');
3940
res.on('data', common.mustCall(() => {
4041
res.destroy();
4142
}));
43+
assert.strictEqual(res.destroyed, false);
4244
res.on('close', common.mustCall(() => {
45+
assert.strictEqual(res.destroyed, true);
4346
server.close();
4447
}));
4548
})
@@ -61,7 +64,12 @@ const http = require('http');
6164
http.get(
6265
{ port: server.address().port },
6366
common.mustCall((res) => {
67+
assert.strictEqual(res.destroyed, false);
68+
res.on('end', common.mustCall(() => {
69+
assert.strictEqual(res.destroyed, false);
70+
}));
6471
res.on('close', common.mustCall(() => {
72+
assert.strictEqual(res.destroyed, true);
6573
server.close();
6674
}));
6775
res.resume();

‎test/parallel/test-http-server-stale-close.js

+3
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
require('../common');
2424
const http = require('http');
2525
const fork = require('child_process').fork;
26+
const assert = require('assert');
2627

2728
if (process.env.NODE_TEST_FORK_PORT) {
2829
const req = http.request({
@@ -37,7 +38,9 @@ if (process.env.NODE_TEST_FORK_PORT) {
3738
const server = http.createServer((req, res) => {
3839
res.writeHead(200, { 'Content-Length': '42' });
3940
req.pipe(res);
41+
assert.strictEqual(req.destroyed, false);
4042
req.on('close', () => {
43+
assert.strictEqual(req.destroyed, true);
4144
server.close();
4245
res.end();
4346
});

0 commit comments

Comments
 (0)