From 299da1f5037d5cbb0146df1007191c51ec9f5d4f Mon Sep 17 00:00:00 2001
From: Anatoli Papirovski <apapirovski@mac.com>
Date: Tue, 17 Apr 2018 00:45:43 +0200
Subject: [PATCH] http: fix _dump regression

A recent set of changes removed _consuming tracking from server
incoming messages which ensures that _dump only runs if the
user has never attempted to read the incoming data. Fix by
reintroducing _consuming which tracks whether _read was ever
successfully called.

PR-URL: https://github.com/nodejs/node/pull/20088
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
---
 lib/_http_incoming.js                    |  8 ++++++
 lib/_http_server.js                      |  2 +-
 test/parallel/test-http-pause-no-dump.js | 33 ++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-http-pause-no-dump.js

diff --git a/lib/_http_incoming.js b/lib/_http_incoming.js
index a84cb64bdb8144..55c196399c5e6b 100644
--- a/lib/_http_incoming.js
+++ b/lib/_http_incoming.js
@@ -38,6 +38,8 @@ function readStop(socket) {
 function IncomingMessage(socket) {
   Stream.Readable.call(this);
 
+  this._readableState.readingMore = true;
+
   this.socket = socket;
   this.connection = socket;
 
@@ -63,6 +65,7 @@ function IncomingMessage(socket) {
   this.statusMessage = null;
   this.client = socket;
 
+  this._consuming = false;
   // flag for when we decide that this message cannot possibly be
   // read by the user, so there's no point continuing to handle it.
   this._dumped = false;
@@ -79,6 +82,11 @@ IncomingMessage.prototype.setTimeout = function setTimeout(msecs, callback) {
 
 
 IncomingMessage.prototype._read = function _read(n) {
+  if (!this._consuming) {
+    this._readableState.readingMore = false;
+    this._consuming = true;
+  }
+
   // We actually do almost nothing here, because the parserOnBody
   // function fills up our internal buffer directly.  However, we
   // do need to unpause the underlying socket so that it flows.
diff --git a/lib/_http_server.js b/lib/_http_server.js
index a8389dbda62f0d..c3abfd37bb5887 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -560,7 +560,7 @@ function resOnFinish(req, res, socket, state, server) {
   // if the user never called req.read(), and didn't pipe() or
   // .resume() or .on('data'), then we call req._dump() so that the
   // bytes will be pulled off the wire.
-  if (!req._readableState.resumeScheduled)
+  if (!req._consuming && !req._readableState.resumeScheduled)
     req._dump();
 
   res.detachSocket(socket);
diff --git a/test/parallel/test-http-pause-no-dump.js b/test/parallel/test-http-pause-no-dump.js
new file mode 100644
index 00000000000000..b794634c489326
--- /dev/null
+++ b/test/parallel/test-http-pause-no-dump.js
@@ -0,0 +1,33 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+
+const server = http.createServer(common.mustCall(function(req, res) {
+  req.once('data', common.mustCall(() => {
+    req.pause();
+    res.writeHead(200);
+    res.end();
+    res.on('finish', common.mustCall(() => {
+      assert(!req._dumped);
+    }));
+  }));
+}));
+server.listen(0);
+
+server.on('listening', common.mustCall(function() {
+  const req = http.request({
+    port: this.address().port,
+    method: 'POST',
+    path: '/'
+  }, common.mustCall(function(res) {
+    assert.strictEqual(res.statusCode, 200);
+    res.resume();
+    res.on('end', common.mustCall(() => {
+      server.close();
+    }));
+  }));
+
+  req.end(Buffer.allocUnsafe(1024));
+}));