From 70eabc1fa7de5bc303bd10de4019e8a6d8a16ab0 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Wed, 11 Mar 2020 13:42:54 +0100
Subject: [PATCH 1/3] stream: avoid destroying http1 objects

http1 objects are coupled with their corresponding
res/req and cannot be treated independently as
normal streams. Add a special exception for this
in the pipeline cleanup.

Fixes: https://github.com/nodejs/node/issues/32184
---
 lib/internal/streams/pipeline.js      | 30 +++++++++++++++++++++------
 test/parallel/test-stream-pipeline.js | 21 +++++++++++++++++++
 2 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/lib/internal/streams/pipeline.js b/lib/internal/streams/pipeline.js
index 9f36d879ea6589..f0a372ea509b24 100644
--- a/lib/internal/streams/pipeline.js
+++ b/lib/internal/streams/pipeline.js
@@ -25,10 +25,32 @@ let EE;
 let PassThrough;
 let createReadableStreamAsyncIterator;
 
+function isIncoming(stream) {
+  return (
+    stream.socket &&
+    typeof stream._dump === 'function' &&
+    typeof stream._consuming === 'boolean' &&
+    ArrayIsArray(stream.rawHeaders)
+  );
+}
+
+function isOutgoing(stream) {
+  return (
+    stream.socket &&
+    typeof stream.setHeader === 'function'
+  );
+}
+
 function destroyer(stream, reading, writing, final, callback) {
   const _destroy = once((err) => {
-    const readable = stream.readable || isRequest(stream);
-    if (err || !final || !readable) {
+    if (!err && ((isIncoming(stream) || isOutgoing(stream)))) {
+      // http/1 request objects have a coupling to their response and should
+      // not be prematurely destroyed. Assume they will handle their own
+      // lifecycle.
+      return callback();
+    }
+
+    if (err || !final || !stream.readable) {
       destroyImpl.destroyer(stream, err);
     }
     callback(err);
@@ -71,10 +93,6 @@ function popCallback(streams) {
   return streams.pop();
 }
 
-function isRequest(stream) {
-  return stream.setHeader && typeof stream.abort === 'function';
-}
-
 function isPromise(obj) {
   return !!(obj && typeof obj.then === 'function');
 }
diff --git a/test/parallel/test-stream-pipeline.js b/test/parallel/test-stream-pipeline.js
index efb363f83b743d..bd7d23fd83cf83 100644
--- a/test/parallel/test-stream-pipeline.js
+++ b/test/parallel/test-stream-pipeline.js
@@ -1011,3 +1011,24 @@ const { promisify } = require('util');
     assert.strictEqual(res, '');
   }));
 }
+
+{
+  const server = http.createServer((req, res) => {
+    req.socket.on('error', common.mustNotCall());
+    pipeline(req, new PassThrough(), (err) => {
+      assert(!err);
+      res.end();
+      server.close();
+    });
+  });
+
+  server.listen(0, () => {
+    const req = http.request({
+      method: 'PUT',
+      port: server.address().port
+    });
+    req.end('asd123');
+    req.on('response', common.mustCall());
+    req.on('error', common.mustNotCall());
+  });
+}

From bed9a326757d9f9328e7f8102e386a6abab8fdf5 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Wed, 11 Mar 2020 14:34:09 +0100
Subject: [PATCH 2/3] fixup: don't depend on private properties

---
 lib/internal/streams/pipeline.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/internal/streams/pipeline.js b/lib/internal/streams/pipeline.js
index f0a372ea509b24..dcae9e577566aa 100644
--- a/lib/internal/streams/pipeline.js
+++ b/lib/internal/streams/pipeline.js
@@ -28,8 +28,8 @@ let createReadableStreamAsyncIterator;
 function isIncoming(stream) {
   return (
     stream.socket &&
-    typeof stream._dump === 'function' &&
-    typeof stream._consuming === 'boolean' &&
+    typeof stream.complete === 'boolean' &&
+    ArrayIsArray(stream.rawTrailers) &&
     ArrayIsArray(stream.rawHeaders)
   );
 }

From 141546473376b4c99d53f02e42b4895df9c16af8 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Wed, 11 Mar 2020 15:29:55 +0100
Subject: [PATCH 3/3] fixup: suggestions

---
 lib/internal/streams/pipeline.js      | 2 +-
 test/parallel/test-stream-pipeline.js | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/internal/streams/pipeline.js b/lib/internal/streams/pipeline.js
index dcae9e577566aa..a446d5792e64bc 100644
--- a/lib/internal/streams/pipeline.js
+++ b/lib/internal/streams/pipeline.js
@@ -43,7 +43,7 @@ function isOutgoing(stream) {
 
 function destroyer(stream, reading, writing, final, callback) {
   const _destroy = once((err) => {
-    if (!err && ((isIncoming(stream) || isOutgoing(stream)))) {
+    if (!err && (isIncoming(stream) || isOutgoing(stream))) {
       // http/1 request objects have a coupling to their response and should
       // not be prematurely destroyed. Assume they will handle their own
       // lifecycle.
diff --git a/test/parallel/test-stream-pipeline.js b/test/parallel/test-stream-pipeline.js
index bd7d23fd83cf83..6629e749238bda 100644
--- a/test/parallel/test-stream-pipeline.js
+++ b/test/parallel/test-stream-pipeline.js
@@ -1016,7 +1016,7 @@ const { promisify } = require('util');
   const server = http.createServer((req, res) => {
     req.socket.on('error', common.mustNotCall());
     pipeline(req, new PassThrough(), (err) => {
-      assert(!err);
+      assert.ifError(err);
       res.end();
       server.close();
     });