Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

child_process: allow options.cwd receive a URL #38862

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 35 additions & 7 deletions doc/api/child_process.md
Expand Up @@ -146,6 +146,10 @@ exec('"my script.cmd" a b', (err, stdout, stderr) => {
<!-- YAML
added: v0.1.90
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/38862
description: The `cwd` option can be a WHATWG `URL` object using
`file:` protocol.
- version: v15.4.0
pr-url: https://github.com/nodejs/node/pull/36308
description: AbortSignal support was added.
Expand All @@ -156,7 +160,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'`
Expand Down Expand Up @@ -271,6 +275,10 @@ controller.abort();
<!-- YAML
added: v0.1.91
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/38862
description: The `cwd` option can be a WHATWG `URL` object using
`file:` protocol.
- version:
- v15.4.0
- v14.17.0
Expand All @@ -284,7 +292,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`
Expand Down Expand Up @@ -376,6 +384,10 @@ controller.abort();
<!-- YAML
added: v0.5.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/38862
description: The `cwd` option can be a WHATWG `URL` object using
`file:` protocol.
- version: v15.13.0
pr-url: https://github.com/nodejs/node/pull/37256
description: timeout was added.
Expand Down Expand Up @@ -403,7 +415,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`][]).
Expand Down Expand Up @@ -487,6 +499,10 @@ if (process.argv[2] === 'child') {
<!-- YAML
added: v0.1.90
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/38862
description: The `cwd` option can be a WHATWG `URL` object using
`file:` protocol.
- version: v15.13.0
pr-url: https://github.com/nodejs/node/pull/37256
description: timeout was added.
Expand Down Expand Up @@ -517,7 +533,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.
Expand Down Expand Up @@ -845,6 +861,10 @@ configuration at startup.
<!-- YAML
added: v0.11.12
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/38862
description: The `cwd` option can be a WHATWG `URL` object using
`file:` protocol.
- version: v10.10.0
pr-url: https://github.com/nodejs/node/pull/22409
description: The `input` option can now be any `TypedArray` or a
Expand All @@ -865,7 +885,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]`.
Expand Down Expand Up @@ -914,6 +934,10 @@ arbitrary command execution.**
<!-- YAML
added: v0.11.12
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/38862
description: The `cwd` option can be a WHATWG `URL` object using
`file:` protocol.
- version: v10.10.0
pr-url: https://github.com/nodejs/node/pull/22409
description: The `input` option can now be any `TypedArray` or a
Expand All @@ -928,7 +952,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]`.
Expand Down Expand Up @@ -974,6 +998,10 @@ metacharacters may be used to trigger arbitrary command execution.**
<!-- YAML
added: v0.11.12
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/38862
description: The `cwd` option can be a WHATWG `URL` object using
`file:` protocol.
- version: v10.10.0
pr-url: https://github.com/nodejs/node/pull/22409
description: The `input` option can now be any `TypedArray` or a
Expand All @@ -997,7 +1025,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]`.
Expand Down
10 changes: 7 additions & 3 deletions lib/child_process.js
Expand Up @@ -71,6 +71,7 @@ const {
ERR_OUT_OF_RANGE,
} = errorCodes;
const { clearTimeout, setTimeout } = require('timers');
const { getValidatedPath } = require('internal/fs/utils');
const {
isInt32,
validateAbortSignal,
Expand Down Expand Up @@ -450,9 +451,11 @@ function normalizeSpawnArguments(file, args, options) {
else
validateObject(options, 'options');

let cwd = options.cwd;

// Validate the cwd, if present.
if (options.cwd != null) {
validateString(options.cwd, 'options.cwd');
if (cwd != null) {
cwd = getValidatedPath(cwd, 'options.cwd');
}

// Validate detached, if present.
Expand Down Expand Up @@ -577,11 +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
windowsVerbatimArguments: !!windowsVerbatimArguments,
};
}

Expand Down
19 changes: 19 additions & 0 deletions test/parallel/test-child-process-cwd.js
Expand Up @@ -20,12 +20,14 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';

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,
Expand Down Expand Up @@ -66,10 +68,27 @@ function testCwd(options, expectPidType, expectCode = 0, expectData) {
}));
}

{
assert.throws(() => {
testCwd({
cwd: new URL(`http://${tmpdir.path}`),
XadillaX marked this conversation as resolved.
Show resolved Hide resolved
}, 'number', 0, tmpdir.path);
}, /The URL must be of scheme file/);

if (process.platform !== 'win32') {
assert.throws(() => {
testCwd({
cwd: new URL(`file://host${tmpdir.path}`),
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
}, 'number', 0, tmpdir.path);
}, /File URL host must be "localhost" or empty on/);
}
}

// 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);
aduh95 marked this conversation as resolved.
Show resolved Hide resolved

// Spawn() shouldn't try to chdir() to invalid arg, so this should just work
testCwd({ cwd: '' }, 'number');
Expand Down