From 15d4f8c9bfd55e293b6d5faad93dfc29fb98e629 Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Tue, 14 Mar 2017 18:22:53 +0100 Subject: [PATCH 1/4] src: ensure that fd 0-2 are valid on windows Check that stdin, stdout and stderr are valid file descriptors on Windows. If not, reopen them with 'nul' file. Refs: https://github.com/nodejs/node/pull/875 Fixes: https://github.com/nodejs/node/issues/11656 --- src/node.cc | 10 ++++++++++ test/fixtures/spawn_closed_stdio.py | 9 +++++++++ test/parallel/test-stdio-closed.js | 9 ++++++++- 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/spawn_closed_stdio.py diff --git a/src/node.cc b/src/node.cc index fda6c3f257730b..698299950c24ce 100644 --- a/src/node.cc +++ b/src/node.cc @@ -4213,6 +4213,16 @@ inline void PlatformInit() { } while (min + 1 < max); } #endif // __POSIX__ +#ifdef _WIN32 + for (int fd = 0; fd <= 2; ++fd) { + auto handle = (HANDLE) _get_osfhandle(fd); + if (handle == INVALID_HANDLE_VALUE || + GetFileType(handle) == FILE_TYPE_UNKNOWN) { + if (_close(fd) || fd != _open("nul", _O_RDWR)) + ABORT(); + } + } +#endif // _WIN32 } diff --git a/test/fixtures/spawn_closed_stdio.py b/test/fixtures/spawn_closed_stdio.py new file mode 100644 index 00000000000000..bcf16d30088c59 --- /dev/null +++ b/test/fixtures/spawn_closed_stdio.py @@ -0,0 +1,9 @@ +import os +import sys +import subprocess +os.close(0) +os.close(1) +os.close(2) +cmd = sys.argv[1] + ' -e "process.stdin; process.stdout; process.stderr;"' +exit_code = subprocess.call(cmd, shell=False) +sys.exit(exit_code) diff --git a/test/parallel/test-stdio-closed.js b/test/parallel/test-stdio-closed.js index 98e4f980d50dd6..7ba9d27d309d30 100644 --- a/test/parallel/test-stdio-closed.js +++ b/test/parallel/test-stdio-closed.js @@ -3,9 +3,16 @@ const common = require('../common'); const assert = require('assert'); const spawn = require('child_process').spawn; const fs = require('fs'); +const path = require('path'); if (common.isWindows) { - common.skip('platform not supported.'); + const python = process.env.PYTHON || 'python'; + const script = path.join(__dirname, '..', 'fixtures', + 'spawn_closed_stdio.py'); + const proc = spawn(python, [ script, process.execPath ]); + proc.on('exit', common.mustCall(function(exitCode) { + assert.strictEqual(exitCode, 0); + })); return; } From a73e803c5d44f8cb813cc5ef5a1c23584e06e9ed Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Thu, 16 Mar 2017 15:45:20 +0100 Subject: [PATCH 2/4] fixup: cast in cpp, works on win2008, python things --- src/node.cc | 5 +++-- test/fixtures/spawn_closed_stdio.py | 3 +-- test/parallel/test-stdio-closed.js | 11 ++++++++--- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/node.cc b/src/node.cc index 698299950c24ce..8e35cb36db5213 100644 --- a/src/node.cc +++ b/src/node.cc @@ -4215,10 +4215,11 @@ inline void PlatformInit() { #endif // __POSIX__ #ifdef _WIN32 for (int fd = 0; fd <= 2; ++fd) { - auto handle = (HANDLE) _get_osfhandle(fd); + auto handle = reinterpret_cast(_get_osfhandle(fd)); if (handle == INVALID_HANDLE_VALUE || GetFileType(handle) == FILE_TYPE_UNKNOWN) { - if (_close(fd) || fd != _open("nul", _O_RDWR)) + _close(fd); + if (fd != _open("nul", _O_RDWR)) ABORT(); } } diff --git a/test/fixtures/spawn_closed_stdio.py b/test/fixtures/spawn_closed_stdio.py index bcf16d30088c59..9fff4ad875e77e 100644 --- a/test/fixtures/spawn_closed_stdio.py +++ b/test/fixtures/spawn_closed_stdio.py @@ -4,6 +4,5 @@ os.close(0) os.close(1) os.close(2) -cmd = sys.argv[1] + ' -e "process.stdin; process.stdout; process.stderr;"' -exit_code = subprocess.call(cmd, shell=False) +exit_code = subprocess.call(sys.argv[1:4], shell=False) sys.exit(exit_code) diff --git a/test/parallel/test-stdio-closed.js b/test/parallel/test-stdio-closed.js index 7ba9d27d309d30..9fde2e96c0be54 100644 --- a/test/parallel/test-stdio-closed.js +++ b/test/parallel/test-stdio-closed.js @@ -6,10 +6,15 @@ const fs = require('fs'); const path = require('path'); if (common.isWindows) { + if (process.argv[2] === 'child') { + process.stdin; + process.stdout; + process.stderr; + process.exit(0); + } const python = process.env.PYTHON || 'python'; - const script = path.join(__dirname, '..', 'fixtures', - 'spawn_closed_stdio.py'); - const proc = spawn(python, [ script, process.execPath ]); + const script = path.join(common.fixturesDir, 'spawn_closed_stdio.py'); + const proc = spawn(python, [script, process.execPath, __filename, 'child']); proc.on('exit', common.mustCall(function(exitCode) { assert.strictEqual(exitCode, 0); })); From db01a43bf92c57f1bc6e46f2c63b20c32cfbce7e Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Thu, 16 Mar 2017 16:54:18 +0100 Subject: [PATCH 3/4] fixup: more changes --- test/fixtures/spawn_closed_stdio.py | 2 +- test/parallel/test-stdio-closed.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/fixtures/spawn_closed_stdio.py b/test/fixtures/spawn_closed_stdio.py index 9fff4ad875e77e..a329c2a2369251 100644 --- a/test/fixtures/spawn_closed_stdio.py +++ b/test/fixtures/spawn_closed_stdio.py @@ -4,5 +4,5 @@ os.close(0) os.close(1) os.close(2) -exit_code = subprocess.call(sys.argv[1:4], shell=False) +exit_code = subprocess.call(sys.argv[1:], shell=False) sys.exit(exit_code) diff --git a/test/parallel/test-stdio-closed.js b/test/parallel/test-stdio-closed.js index 9fde2e96c0be54..2313140a26aea7 100644 --- a/test/parallel/test-stdio-closed.js +++ b/test/parallel/test-stdio-closed.js @@ -10,7 +10,7 @@ if (common.isWindows) { process.stdin; process.stdout; process.stderr; - process.exit(0); + return; } const python = process.env.PYTHON || 'python'; const script = path.join(common.fixturesDir, 'spawn_closed_stdio.py'); From 5d563cfe130e1be6575a84f16f6ce5e943d43ca9 Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Mon, 20 Mar 2017 12:01:44 +0100 Subject: [PATCH 4/4] fixup: add comment about _close --- src/node.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/node.cc b/src/node.cc index 8e35cb36db5213..d79b8ee30edd2b 100644 --- a/src/node.cc +++ b/src/node.cc @@ -4218,6 +4218,8 @@ inline void PlatformInit() { auto handle = reinterpret_cast(_get_osfhandle(fd)); if (handle == INVALID_HANDLE_VALUE || GetFileType(handle) == FILE_TYPE_UNKNOWN) { + // Ignore _close result. If it fails or not depends on used Windows + // version. We will just check _open result. _close(fd); if (fd != _open("nul", _O_RDWR)) ABORT();