From 18052364ce83fb4760b759553e75bcea1969ecb2 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Wed, 9 May 2018 20:42:31 +0200
Subject: [PATCH] process: provide dummy stdio for non-console Windows apps
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The only known condition where we could not provide appropriate
stdio streams so far were non-console Windows applications.
Since this issue has come up a few times in our issue tracker now,
switch to providing dummy streams for these cases instead.

If there are other valid cases in which `uv_guess_handle` fails,
and where there is a more sensible way to provide stdio,
we’ll probably still find out because the streams don’t work
properly either way.

Refs: https://github.com/nodejs/help/issues/1251

PR-URL: https://github.com/nodejs/node/pull/20640
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
---
 doc/api/errors.md                 | 36 +++++++++++++++++++------------
 lib/internal/errors.js            |  3 ---
 lib/internal/process/stdio.js     | 28 ++++++++++++++++--------
 test/parallel/test-dummy-stdio.js | 27 +++++++++++++++++++++++
 4 files changed, 68 insertions(+), 26 deletions(-)
 create mode 100644 test/parallel/test-dummy-stdio.js

diff --git a/doc/api/errors.md b/doc/api/errors.md
index f504acd8c02a8f..68e6955925c117 100644
--- a/doc/api/errors.md
+++ b/doc/api/errors.md
@@ -1810,20 +1810,6 @@ An attempt was made to load a module with an unknown or unsupported format.
 An invalid or unknown process signal was passed to an API expecting a valid
 signal (such as [`subprocess.kill()`][]).
 
-<a id="ERR_UNKNOWN_STDIN_TYPE"></a>
-### ERR_UNKNOWN_STDIN_TYPE
-
-An attempt was made to launch a Node.js process with an unknown `stdin` file
-type. This error is usually an indication of a bug within Node.js itself,
-although it is possible for user code to trigger it.
-
-<a id="ERR_UNKNOWN_STREAM_TYPE"></a>
-### ERR_UNKNOWN_STREAM_TYPE
-
-An attempt was made to launch a Node.js process with an unknown `stdout` or
-`stderr` file type. This error is usually an indication of a bug within Node.js
-itself, although it is possible for user code to trigger it.
-
 <a id="ERR_V8BREAKITERATOR"></a>
 ### ERR_V8BREAKITERATOR
 
@@ -2080,6 +2066,28 @@ kind of internal Node.js error that should not typically be triggered by user
 code. Instances of this error point to an internal bug within the Node.js
 binary itself.
 
+<a id="ERR_UNKNOWN_STDIN_TYPE"></a>
+### ERR_UNKNOWN_STDIN_TYPE
+<!-- YAML
+added: v8.0.0
+removed: REPLACEME
+-->
+
+An attempt was made to launch a Node.js process with an unknown `stdin` file
+type. This error is usually an indication of a bug within Node.js itself,
+although it is possible for user code to trigger it.
+
+<a id="ERR_UNKNOWN_STREAM_TYPE"></a>
+### ERR_UNKNOWN_STREAM_TYPE
+<!-- YAML
+added: v8.0.0
+removed: REPLACEME
+-->
+
+An attempt was made to launch a Node.js process with an unknown `stdout` or
+`stderr` file type. This error is usually an indication of a bug within Node.js
+itself, although it is possible for user code to trigger it.
+
 <a id="ERR_VALUE_OUT_OF_RANGE"></a>
 ### ERR_VALUE_OUT_OF_RANGE
 <!-- YAML
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index e8e62b5d2ca3de..700277630054a6 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -947,10 +947,7 @@ E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s', TypeError);
 E('ERR_UNKNOWN_FILE_EXTENSION', 'Unknown file extension: %s', Error);
 E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s', RangeError);
 E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError);
-E('ERR_UNKNOWN_STDIN_TYPE', 'Unknown stdin file type', Error);
 
-// This should probably be a `TypeError`.
-E('ERR_UNKNOWN_STREAM_TYPE', 'Unknown stream file type', Error);
 E('ERR_V8BREAKITERATOR',
   'Full ICU data not installed. See https://github.com/nodejs/node/wiki/Intl',
   Error);
diff --git a/lib/internal/process/stdio.js b/lib/internal/process/stdio.js
index a52bbb7b1d204c..5e9ff6b26097a6 100644
--- a/lib/internal/process/stdio.js
+++ b/lib/internal/process/stdio.js
@@ -1,10 +1,5 @@
 'use strict';
 
-const {
-  ERR_UNKNOWN_STDIN_TYPE,
-  ERR_UNKNOWN_STREAM_TYPE
-} = require('internal/errors').codes;
-
 exports.setupProcessStdio = setupProcessStdio;
 exports.getMainThreadStdio = getMainThreadStdio;
 
@@ -87,8 +82,11 @@ function getMainThreadStdio() {
         break;
 
       default:
-        // Probably an error on in uv_guess_handle()
-        throw new ERR_UNKNOWN_STDIN_TYPE();
+        // Provide a dummy contentless input for e.g. non-console
+        // Windows applications.
+        const { Readable } = require('stream');
+        stdin = new Readable({ read() {} });
+        stdin.push(null);
     }
 
     // For supporting legacy API we put the FD here.
@@ -123,6 +121,12 @@ function getMainThreadStdio() {
     return stdin;
   }
 
+  exports.resetStdioForTesting = function() {
+    stdin = undefined;
+    stdout = undefined;
+    stderr = undefined;
+  };
+
   return {
     getStdout,
     getStderr,
@@ -199,8 +203,14 @@ function createWritableStdioStream(fd) {
       break;
 
     default:
-      // Probably an error on in uv_guess_handle()
-      throw new ERR_UNKNOWN_STREAM_TYPE();
+      // Provide a dummy black-hole output for e.g. non-console
+      // Windows applications.
+      const { Writable } = require('stream');
+      stream = new Writable({
+        write(buf, enc, cb) {
+          cb();
+        }
+      });
   }
 
   // For supporting legacy API we put the FD here.
diff --git a/test/parallel/test-dummy-stdio.js b/test/parallel/test-dummy-stdio.js
new file mode 100644
index 00000000000000..165c2c2c575ff5
--- /dev/null
+++ b/test/parallel/test-dummy-stdio.js
@@ -0,0 +1,27 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const child_process = require('child_process');
+
+if (common.isWindows)
+  common.skip('fs.closeSync(n) does not close stdio on Windows');
+
+function runTest(fd, streamName, testOutputStream, expectedName) {
+  const result = child_process.spawnSync(process.execPath, [
+    '--expose-internals',
+    '-e', `
+    require('internal/process/stdio').resetStdioForTesting();
+    fs.closeSync(${fd});
+    const ctorName = process.${streamName}.constructor.name;
+    process.${testOutputStream}.write(ctorName);
+    `]);
+  assert.strictEqual(result[testOutputStream].toString(), expectedName,
+                     `stdout:\n${result.stdout}\nstderr:\n${result.stderr}\n` +
+                     `while running test with fd = ${fd}`);
+  if (testOutputStream !== 'stderr')
+    assert.strictEqual(result.stderr.toString(), '');
+}
+
+runTest(0, 'stdin', 'stdout', 'Readable');
+runTest(1, 'stdout', 'stderr', 'Writable');
+runTest(2, 'stderr', 'stdout', 'Writable');