From 759e5b813d65c869d1b928003209ef3e965f7c81 Mon Sep 17 00:00:00 2001 From: XadillaX Date: Mon, 31 May 2021 17:00:46 +0800 Subject: [PATCH 01/11] child_process: allow `options.cwd` receive a URL Ref: https://github.com/nodejs/node/issues/38861 --- doc/api/child_process.md | 14 +++++++------- lib/child_process.js | 19 ++++++++++++++++--- test/parallel/test-child-process-cwd.js | 17 +++++++++++++++++ 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/doc/api/child_process.md b/doc/api/child_process.md index 2ac6f1c35c41b3..9fd932cc439be2 100644 --- a/doc/api/child_process.md +++ b/doc/api/child_process.md @@ -156,7 +156,7 @@ changes: * `command` {string} The command to run, with space-separated arguments. * `options` {Object} - * `cwd` {string} Current working directory of the child process. + * `cwd` {string|URL} Current working directory of the child process. **Default:** `process.cwd()`. * `env` {Object} Environment key-value pairs. **Default:** `process.env`. * `encoding` {string} **Default:** `'utf8'` @@ -284,7 +284,7 @@ changes: * `file` {string} The name or path of the executable file to run. * `args` {string[]} List of string arguments. * `options` {Object} - * `cwd` {string} Current working directory of the child process. + * `cwd` {string|URL} Current working directory of the child process. * `env` {Object} Environment key-value pairs. **Default:** `process.env`. * `encoding` {string} **Default:** `'utf8'` * `timeout` {number} **Default:** `0` @@ -403,7 +403,7 @@ changes: * `modulePath` {string} The module to run in the child. * `args` {string[]} List of string arguments. * `options` {Object} - * `cwd` {string} Current working directory of the child process. + * `cwd` {string|URL} Current working directory of the child process. * `detached` {boolean} Prepare child to run independently of its parent process. Specific behavior depends on the platform, see [`options.detached`][]). @@ -517,7 +517,7 @@ changes: * `command` {string} The command to run. * `args` {string[]} List of string arguments. * `options` {Object} - * `cwd` {string} Current working directory of the child process. + * `cwd` {string|URL} Current working directory of the child process. * `env` {Object} Environment key-value pairs. **Default:** `process.env`. * `argv0` {string} Explicitly set the value of `argv[0]` sent to the child process. This will be set to `command` if not specified. @@ -865,7 +865,7 @@ changes: * `file` {string} The name or path of the executable file to run. * `args` {string[]} List of string arguments. * `options` {Object} - * `cwd` {string} Current working directory of the child process. + * `cwd` {string|URL} Current working directory of the child process. * `input` {string|Buffer|TypedArray|DataView} The value which will be passed as stdin to the spawned process. Supplying this value will override `stdio[0]`. @@ -928,7 +928,7 @@ changes: * `command` {string} The command to run. * `options` {Object} - * `cwd` {string} Current working directory of the child process. + * `cwd` {string|URL} Current working directory of the child process. * `input` {string|Buffer|TypedArray|DataView} The value which will be passed as stdin to the spawned process. Supplying this value will override `stdio[0]`. @@ -997,7 +997,7 @@ changes: * `command` {string} The command to run. * `args` {string[]} List of string arguments. * `options` {Object} - * `cwd` {string} Current working directory of the child process. + * `cwd` {string|URL} Current working directory of the child process. * `input` {string|Buffer|TypedArray|DataView} The value which will be passed as stdin to the spawned process. Supplying this value will override `stdio[0]`. diff --git a/lib/child_process.js b/lib/child_process.js index 26e1bb33d0c9ef..b030bb9804e513 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -78,6 +78,7 @@ const { validateString, } = require('internal/validators'); const child_process = require('internal/child_process'); +const { URL } = require('internal/url'); const { getValidStdio, setupChannel, @@ -450,9 +451,20 @@ function normalizeSpawnArguments(file, args, options) { else validateObject(options, 'options'); + let cwd = null; + if (!(options.cwd instanceof URL)) { + cwd = options.cwd; + } else if (options.cwd.protocol === 'file:' && !options.cwd.host) { + cwd = options.cwd.pathname; + } else { + throw new ERR_INVALID_ARG_VALUE('options.cwd', + options.cwd, + 'contains invalid protocol or host'); + } + // Validate the cwd, if present. - if (options.cwd != null) { - validateString(options.cwd, 'options.cwd'); + if (cwd != null) { + validateString(cwd, 'options.cwd'); } // Validate detached, if present. @@ -581,7 +593,8 @@ function normalizeSpawnArguments(file, args, options) { envPairs, file, windowsHide: !!options.windowsHide, - windowsVerbatimArguments: !!windowsVerbatimArguments + windowsVerbatimArguments: !!windowsVerbatimArguments, + cwd, }; } diff --git a/test/parallel/test-child-process-cwd.js b/test/parallel/test-child-process-cwd.js index 232f1d0f3d5316..592f14cc00c98b 100644 --- a/test/parallel/test-child-process-cwd.js +++ b/test/parallel/test-child-process-cwd.js @@ -20,6 +20,8 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. 'use strict'; + +const { URL } = require('url'); const common = require('../common'); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); @@ -66,10 +68,25 @@ function testCwd(options, expectPidType, expectCode = 0, expectData) { })); } +{ + assert.throws(() => { + testCwd({ + cwd: new URL(`http://${tmpdir.path}`), + }, 'number', 0, tmpdir.path); + }, /The property 'options\.cwd' contains invalid protocol or host/); + + assert.throws(() => { + testCwd({ + cwd: new URL(`file://host${tmpdir.path}`), + }, 'number', 0, tmpdir.path); + }, /The property 'options\.cwd' contains invalid protocol or host/); +} + // Assume these exist, and 'pwd' gives us the right directory back testCwd({ cwd: tmpdir.path }, 'number', 0, tmpdir.path); const shouldExistDir = common.isWindows ? process.env.windir : '/dev'; testCwd({ cwd: shouldExistDir }, 'number', 0, shouldExistDir); +testCwd({ cwd: new URL(`file://${tmpdir.path}`) }, 'number', 0, tmpdir.path); // Spawn() shouldn't try to chdir() to invalid arg, so this should just work testCwd({ cwd: '' }, 'number'); From 96f63c5755d0cd937eb20a6d0c3b5a27c57ad810 Mon Sep 17 00:00:00 2001 From: XadillaX Date: Mon, 31 May 2021 17:09:49 +0800 Subject: [PATCH 02/11] f --- lib/child_process.js | 14 +++----------- test/parallel/test-child-process-cwd.js | 4 ++-- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index b030bb9804e513..4f4016a9b0e2e1 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -71,6 +71,7 @@ const { ERR_OUT_OF_RANGE, } = errorCodes; const { clearTimeout, setTimeout } = require('timers'); +const { getValidatedPath } = require('internal/fs/utils'); const { isInt32, validateAbortSignal, @@ -451,20 +452,11 @@ function normalizeSpawnArguments(file, args, options) { else validateObject(options, 'options'); - let cwd = null; - if (!(options.cwd instanceof URL)) { - cwd = options.cwd; - } else if (options.cwd.protocol === 'file:' && !options.cwd.host) { - cwd = options.cwd.pathname; - } else { - throw new ERR_INVALID_ARG_VALUE('options.cwd', - options.cwd, - 'contains invalid protocol or host'); - } + let cwd = options.cwd; // Validate the cwd, if present. if (cwd != null) { - validateString(cwd, 'options.cwd'); + cwd = getValidatedPath(cwd, 'options.cwd'); } // Validate detached, if present. diff --git a/test/parallel/test-child-process-cwd.js b/test/parallel/test-child-process-cwd.js index 592f14cc00c98b..ca6b25a3a364df 100644 --- a/test/parallel/test-child-process-cwd.js +++ b/test/parallel/test-child-process-cwd.js @@ -73,13 +73,13 @@ function testCwd(options, expectPidType, expectCode = 0, expectData) { testCwd({ cwd: new URL(`http://${tmpdir.path}`), }, 'number', 0, tmpdir.path); - }, /The property 'options\.cwd' contains invalid protocol or host/); + }, /The URL must be of scheme file/); assert.throws(() => { testCwd({ cwd: new URL(`file://host${tmpdir.path}`), }, 'number', 0, tmpdir.path); - }, /The property 'options\.cwd' contains invalid protocol or host/); + }, /File URL host must be "localhost" or empty on linux/); } // Assume these exist, and 'pwd' gives us the right directory back From de37c098e3d9bcadb393386a2fc64c5dd4e49652 Mon Sep 17 00:00:00 2001 From: XadillaX Date: Mon, 31 May 2021 17:16:53 +0800 Subject: [PATCH 03/11] f --- lib/child_process.js | 1 - test/parallel/test-child-process-cwd.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index 4f4016a9b0e2e1..a5a1f95e7d371d 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -79,7 +79,6 @@ const { validateString, } = require('internal/validators'); const child_process = require('internal/child_process'); -const { URL } = require('internal/url'); const { getValidStdio, setupChannel, diff --git a/test/parallel/test-child-process-cwd.js b/test/parallel/test-child-process-cwd.js index ca6b25a3a364df..179d93524d280f 100644 --- a/test/parallel/test-child-process-cwd.js +++ b/test/parallel/test-child-process-cwd.js @@ -21,13 +21,13 @@ 'use strict'; -const { URL } = require('url'); const common = require('../common'); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); const assert = require('assert'); const { spawn } = require('child_process'); +const { URL } = require('url'); // Spawns 'pwd' with given options, then test // - whether the child pid is undefined or number, From 6d2455fb6f50bf8e6e9508e15f9295af34bace5a Mon Sep 17 00:00:00 2001 From: XadillaX Date: Mon, 31 May 2021 22:22:37 +0800 Subject: [PATCH 04/11] f --- lib/child_process.js | 2 +- test/parallel/test-child-process-cwd.js | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index a5a1f95e7d371d..e3691639a9fddd 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -580,12 +580,12 @@ function normalizeSpawnArguments(file, args, options) { // Make a shallow copy so we don't clobber the user's options object. ...options, args, + cwd, detached: !!options.detached, envPairs, file, windowsHide: !!options.windowsHide, windowsVerbatimArguments: !!windowsVerbatimArguments, - cwd, }; } diff --git a/test/parallel/test-child-process-cwd.js b/test/parallel/test-child-process-cwd.js index 179d93524d280f..229e76b54f7ddb 100644 --- a/test/parallel/test-child-process-cwd.js +++ b/test/parallel/test-child-process-cwd.js @@ -75,11 +75,14 @@ function testCwd(options, expectPidType, expectCode = 0, expectData) { }, 'number', 0, tmpdir.path); }, /The URL must be of scheme file/); - assert.throws(() => { - testCwd({ - cwd: new URL(`file://host${tmpdir.path}`), - }, 'number', 0, tmpdir.path); - }, /File URL host must be "localhost" or empty on linux/); + if (process.platform !== 'win32') { + assert.throws(() => { + testCwd({ + cwd: new URL(`file://host${tmpdir.path}`), + }, 'number', 0, tmpdir.path); + }, new RegExp( + `File URL host must be "localhost" or empty on ${process.platform}`)); + } } // Assume these exist, and 'pwd' gives us the right directory back From d3276d8cb6f736cda84a5cc72d1713b90a568405 Mon Sep 17 00:00:00 2001 From: XadillaX Date: Mon, 31 May 2021 22:24:52 +0800 Subject: [PATCH 05/11] f --- test/parallel/test-child-process-cwd.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-child-process-cwd.js b/test/parallel/test-child-process-cwd.js index 229e76b54f7ddb..ee2f4dda4ff0e0 100644 --- a/test/parallel/test-child-process-cwd.js +++ b/test/parallel/test-child-process-cwd.js @@ -81,7 +81,7 @@ function testCwd(options, expectPidType, expectCode = 0, expectData) { cwd: new URL(`file://host${tmpdir.path}`), }, 'number', 0, tmpdir.path); }, new RegExp( - `File URL host must be "localhost" or empty on ${process.platform}`)); + `File URL host must be "localhost" or empty on ${process.platform}`)); } } From 42ea2721ee7e7f0d1bce4dec4d924af642a794f0 Mon Sep 17 00:00:00 2001 From: XadillaX Date: Mon, 31 May 2021 22:30:51 +0800 Subject: [PATCH 06/11] f --- doc/api/child_process.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/doc/api/child_process.md b/doc/api/child_process.md index 9fd932cc439be2..5c54c2ab9a8d1e 100644 --- a/doc/api/child_process.md +++ b/doc/api/child_process.md @@ -146,6 +146,9 @@ exec('"my script.cmd" a b', (err, stdout, stderr) => {