From e8517c2b9ee70a19ba478f5d279bf0a078fb89d8 Mon Sep 17 00:00:00 2001
From: Momtchil Momtchev <momtchil@momtchev.com>
Date: Thu, 26 Nov 2020 12:59:00 +0100
Subject: [PATCH 1/2] stdio: lazy read ReadStream

All stdio ReadStream's use manual start to avoid
consuming data for example when a process
execs/spawns.

Using stream._construct would cause the Readable
to incorrectly greedily start reading.

Refs: https://github.com/nodejs/node/issues/36251
---
 lib/internal/streams/readable.js            |  8 +++-
 test/parallel/test-stdin-from-file-spawn.js | 42 +++++++++++++++++++++
 test/parallel/test-stream-construct.js      | 10 +++++
 3 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-stdin-from-file-spawn.js

diff --git a/lib/internal/streams/readable.js b/lib/internal/streams/readable.js
index 6fff20c94ec7f3..7f868ef982dce2 100644
--- a/lib/internal/streams/readable.js
+++ b/lib/internal/streams/readable.js
@@ -166,6 +166,8 @@ function ReadableState(options, stream, isDuplex) {
   // If true, a maybeReadMore has been scheduled.
   this.readingMore = false;
 
+  this.didRead = false;
+
   this.decoder = null;
   this.encoding = null;
   if (options && options.encoding) {
@@ -203,7 +205,9 @@ function Readable(options) {
   Stream.call(this, options);
 
   destroyImpl.construct(this, () => {
-    maybeReadMore(this, this._readableState);
+    if (this._readableState.didRead) {
+      maybeReadMore(this, this._readableState);
+    }
   });
 }
 
@@ -401,6 +405,8 @@ Readable.prototype.read = function(n) {
   const state = this._readableState;
   const nOrig = n;
 
+  this.didRead = true;
+
   // If we're asking for more than the current hwm, then raise the hwm.
   if (n > state.highWaterMark)
     state.highWaterMark = computeNewHighWaterMark(n);
diff --git a/test/parallel/test-stdin-from-file-spawn.js b/test/parallel/test-stdin-from-file-spawn.js
new file mode 100644
index 00000000000000..12c235fcfeb081
--- /dev/null
+++ b/test/parallel/test-stdin-from-file-spawn.js
@@ -0,0 +1,42 @@
+'use strict';
+const common = require('../common');
+const process = require('process');
+
+let defaultShell;
+if (process.platform === 'linux' || process.platform === 'darwin') {
+  defaultShell = '/bin/sh';
+} else if (process.platform === 'win32') {
+  defaultShell = 'cmd.exe';
+} else {
+  common.skip('This is test exists only on Linux/Win32/OSX');
+}
+
+const { execSync } = require('child_process');
+const fs = require('fs');
+const path = require('path');
+const tmpdir = require('../common/tmpdir');
+
+const tmpDir = tmpdir.path;
+tmpdir.refresh();
+const tmpCmdFile = path.join(tmpDir, 'test-stdin-from-file-spawn-cmd');
+const tmpJsFile = path.join(tmpDir, 'test-stdin-from-file-spawn.js');
+fs.writeFileSync(tmpCmdFile, 'echo hello');
+fs.writeFileSync(tmpJsFile, `
+'use strict';
+const { spawn } = require('child_process');
+// Reference the object to invoke the getter
+process.stdin;
+setTimeout(() => {
+  let ok = false;
+  const child = spawn(process.env.SHELL || '${defaultShell}',
+    [], { stdio: ['inherit', 'pipe'] });
+  child.stdout.on('data', () => {
+    ok = true;
+  });
+  child.on('close', () => {
+    process.exit(ok ? 0 : -1);
+  });
+}, 100);
+`);
+
+execSync(`${process.argv[0]} ${tmpJsFile} < ${tmpCmdFile}`);
diff --git a/test/parallel/test-stream-construct.js b/test/parallel/test-stream-construct.js
index 9b38f30a01f8c2..907b9aa0e3e296 100644
--- a/test/parallel/test-stream-construct.js
+++ b/test/parallel/test-stream-construct.js
@@ -268,3 +268,13 @@ testDestroy((opts) => new Writable({
     assert.strictEqual(constructed, true);
   }));
 }
+
+{
+  // Construct should not cause stream to read.
+  new Readable({
+    construct: common.mustCall((callback) => {
+      callback();
+    }),
+    read: common.mustNotCall()
+  });
+}

From c47d895d599fd7d30089f25051448ecbbfe19fc4 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Wed, 6 Jan 2021 23:58:39 +0100
Subject: [PATCH 2/2] fixup: use needReadable

---
 lib/internal/streams/readable.js | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/lib/internal/streams/readable.js b/lib/internal/streams/readable.js
index 7f868ef982dce2..d0424edc1fa32b 100644
--- a/lib/internal/streams/readable.js
+++ b/lib/internal/streams/readable.js
@@ -166,8 +166,6 @@ function ReadableState(options, stream, isDuplex) {
   // If true, a maybeReadMore has been scheduled.
   this.readingMore = false;
 
-  this.didRead = false;
-
   this.decoder = null;
   this.encoding = null;
   if (options && options.encoding) {
@@ -205,7 +203,7 @@ function Readable(options) {
   Stream.call(this, options);
 
   destroyImpl.construct(this, () => {
-    if (this._readableState.didRead) {
+    if (this._readableState.needReadable) {
       maybeReadMore(this, this._readableState);
     }
   });
@@ -405,8 +403,6 @@ Readable.prototype.read = function(n) {
   const state = this._readableState;
   const nOrig = n;
 
-  this.didRead = true;
-
   // If we're asking for more than the current hwm, then raise the hwm.
   if (n > state.highWaterMark)
     state.highWaterMark = computeNewHighWaterMark(n);