From 25d7e9038609e19782fd10506932880608d72e8d Mon Sep 17 00:00:00 2001
From: Daniele Belardi <dwon.dnl@gmail.com>
Date: Tue, 19 May 2020 08:29:00 +0200
Subject: [PATCH] http: use `autoDestroy: true` in incoming message

Enable the default `autoDestroy: true` option in IncomingMessage.

Refactor `_http_client` and `_http_server` to remove any manual
destroying/closing of IncomingMessage.
Refactor IncomingMessage `destroy` method to use the standard
implementation of the stream module and move the early termination
event emitting inside of it.

PR-URL: https://github.com/nodejs/node/pull/33035
Refs: https://github.com/nodejs/node/issues/30625
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 lib/_http_client.js   | 20 ++------------------
 lib/_http_incoming.js | 19 ++++++++++---------
 lib/_http_server.js   | 14 +-------------
 3 files changed, 13 insertions(+), 40 deletions(-)

diff --git a/lib/_http_client.js b/lib/_http_client.js
index 6fb5dd65cb368c..d647325d20fbb4 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -430,25 +430,13 @@ function socketCloseListener() {
   req.destroyed = true;
   if (res) {
     // Socket closed before we emitted 'end' below.
-    // TOOD(ronag): res.destroy(err)
     if (!res.complete) {
-      res.aborted = true;
-      res.emit('aborted');
-      if (res.listenerCount('error') > 0) {
-        res.emit('error', connResetException('aborted'));
-      }
+      res.destroy(connResetException('aborted'));
     }
     req._closed = true;
     req.emit('close');
     if (!res.aborted && res.readable) {
-      res.on('end', function() {
-        this.destroyed = true;
-        this.emit('close');
-      });
       res.push(null);
-    } else {
-      res.destroyed = true;
-      res.emit('close');
     }
   } else {
     if (!req.socket._hadError) {
@@ -697,7 +685,6 @@ function responseKeepAlive(req) {
 
   req.destroyed = true;
   if (req.res) {
-    req.res.destroyed = true;
     // Detach socket from IncomingMessage to avoid destroying the freed
     // socket in IncomingMessage.destroy().
     req.res.socket = null;
@@ -752,13 +739,10 @@ function requestOnPrefinish() {
 function emitFreeNT(req) {
   req._closed = true;
   req.emit('close');
-  if (req.res) {
-    req.res.emit('close');
-  }
-
   if (req.socket) {
     req.socket.emit('free');
   }
+
 }
 
 function tickOnSocket(req, socket) {
diff --git a/lib/_http_incoming.js b/lib/_http_incoming.js
index 7943c69f54d911..ace18a83d4f99a 100644
--- a/lib/_http_incoming.js
+++ b/lib/_http_incoming.js
@@ -54,7 +54,7 @@ function IncomingMessage(socket) {
     };
   }
 
-  Stream.Readable.call(this, { autoDestroy: false, ...streamOptions });
+  Stream.Readable.call(this, streamOptions);
 
   this._readableState.readingMore = true;
 
@@ -160,19 +160,20 @@ IncomingMessage.prototype._read = function _read(n) {
     readStart(this.socket);
 };
 
-
 // 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) {
-  // TODO(ronag): Implement in terms of _destroy
-  this.destroyed = true;
-  if (this.socket)
-    this.socket.destroy(error);
-  return this;
+IncomingMessage.prototype._destroy = function _destroy(err, cb) {
+  if (!this.readableEnded || !this.complete) {
+    this.aborted = true;
+    this.emit('aborted');
+  }
+  if (this.socket && !this.readableEnded) {
+    this.socket.destroy(err);
+  }
+  this.listenerCount('error') > 0 ? cb(err) : cb();
 };
 
-
 IncomingMessage.prototype._addHeaderLines = _addHeaderLines;
 function _addHeaderLines(headers, n) {
   if (headers && headers.length) {
diff --git a/lib/_http_server.js b/lib/_http_server.js
index 419b08a7a0986d..8cd10bb3a00194 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -575,14 +575,7 @@ function socketOnClose(socket, state) {
 function abortIncoming(incoming) {
   while (incoming.length) {
     const req = incoming.shift();
-    // TODO(ronag): req.destroy(err)
-    req.aborted = true;
-    req.destroyed = true;
-    req.emit('aborted');
-    if (req.listenerCount('error') > 0) {
-      req.emit('error', connResetException('aborted'));
-    }
-    req.emit('close');
+    req.destroy(connResetException('aborted'));
   }
   // Abort socket._httpMessage ?
 }
@@ -741,14 +734,9 @@ function clearIncoming(req) {
   if (parser && parser.incoming === req) {
     if (req.readableEnded) {
       parser.incoming = null;
-      req.destroyed = true;
-      req.emit('close');
     } else {
       req.on('end', clearIncoming);
     }
-  } else {
-    req.destroyed = true;
-    req.emit('close');
   }
 }