From c74652fe2c99b24995f53de4d99709766b2fb27e Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 6 Oct 2019 11:24:00 +0200 Subject: [PATCH] http: IncomingMessage destroyed state IncomingMessage should implement _destroy() instead of overriding destroy() in order to behave properly like a stream. --- lib/_http_client.js | 8 +++-- lib/_http_incoming.js | 6 ++-- test/parallel/test-http-request-destroy.js | 39 ++++++++++++++++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-http-request-destroy.js diff --git a/lib/_http_client.js b/lib/_http_client.js index c7c27f9ad598a5..78f3cf07e2c1fd 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -318,7 +318,9 @@ ClientRequest.prototype.abort = function abort() { // If we're aborting, we don't care about any more response data. if (this.res) { - this.res._dump(); + // TODO(ronag): No more data events should occur after destroy. + this.res.removeAllListeners('data'); + this.res.destroy(); } // In the event that we don't have a socket, we will pop out of @@ -365,11 +367,11 @@ function socketCloseListener() { req.emit('close'); if (res.readable) { res.on('end', function() { - this.emit('close'); + res.destroy(); }); res.push(null); } else { - res.emit('close'); + res.destroy(); } } else { if (!req.socket._hadError) { diff --git a/lib/_http_incoming.js b/lib/_http_incoming.js index ad3699cc44dc33..ee52dba7ecdfb6 100644 --- a/lib/_http_incoming.js +++ b/lib/_http_incoming.js @@ -109,9 +109,11 @@ IncomingMessage.prototype._read = function _read(n) { // It's possible that the socket will be destroyed, and removed from // any messages, before ever calling this. In that case, just skip // it, since something else is destroying this connection anyway. -IncomingMessage.prototype.destroy = function destroy(error) { +IncomingMessage.prototype._destroy = function destroy(err, cb) { if (this.socket) - this.socket.destroy(error); + this.socket.destroy(err, cb); + else + cb(err); }; diff --git a/test/parallel/test-http-request-destroy.js b/test/parallel/test-http-request-destroy.js new file mode 100644 index 00000000000000..1095039b3f0cd1 --- /dev/null +++ b/test/parallel/test-http-request-destroy.js @@ -0,0 +1,39 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +{ + const server = http.Server(function(req, res) { + req.destroy(); + assert.strictEqual(req.destroyed, true); + }); + + server.listen(0, function() { + http.get({ + port: this.address().port, + }).on('error', common.mustCall(() => { + server.close(); + })); + }); +} + +{ + const server = http.Server(function(req, res) { + req.destroy(new Error('kaboom')); + req.destroy(new Error('kaboom2')); + assert.strictEqual(req.destroyed, true); + req.on('error', common.mustCall((err) => { + assert.strictEqual(err.message, 'kaboom'); + })); + }); + + server.listen(0, function() { + http.get({ + port: this.address().port, + }).on('error', common.mustCall(() => { + server.close(); + })); + }); +}