Skip to content

Commit

Permalink
child_process: allow typed arrays for input
Browse files Browse the repository at this point in the history
doc:    Update child_process docs, stating typed arrays are allowed.
error:  Update error message for `ERR_INVALID_SYNC_FORK_INPUT`
lib:    Use isArrayBufferView instead of isUint8Array
test:   Update test-child-process-spawnsync-input to test for all
        typed arrays and data view.

PR-URL: #22409
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
  • Loading branch information
SirR4T authored and addaleax committed Aug 27, 2018
1 parent 31b3273 commit 68dff4a
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 22 deletions.
27 changes: 21 additions & 6 deletions doc/api/child_process.md
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,10 @@ configuration at startup.
<!-- YAML
added: v0.11.12
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/22409
description: The `input` option can now be any `TypedArray` or a
`DataView`.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/21316
description: The `windowsHide` option now defaults to `true`.
Expand All @@ -706,8 +710,9 @@ changes:
* `args` {string[]} List of string arguments.
* `options` {Object}
* `cwd` {string} Current working directory of the child process.
* `input` {string|Buffer|Uint8Array} The value which will be passed as stdin
to the spawned process. Supplying this value will override `stdio[0]`.
* `input` {string|Buffer|TypedArray|DataView} The value which will be passed
as stdin to the spawned process. Supplying this value will override
`stdio[0]`.
* `stdio` {string|Array} Child's stdio configuration. `stderr` by default will
be output to the parent process' stderr unless `stdio` is specified.
**Default:** `'pipe'`.
Expand Down Expand Up @@ -753,6 +758,10 @@ arbitrary command execution.**
<!-- YAML
added: v0.11.12
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/22409
description: The `input` option can now be any `TypedArray` or a
`DataView`.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/21316
description: The `windowsHide` option now defaults to `true`.
Expand All @@ -767,8 +776,9 @@ changes:
* `command` {string} The command to run.
* `options` {Object}
* `cwd` {string} Current working directory of the child process.
* `input` {string|Buffer|Uint8Array} The value which will be passed as stdin
to the spawned process. Supplying this value will override `stdio[0]`.
* `input` {string|Buffer|TypedArray|DataView} The value which will be passed
as stdin to the spawned process. Supplying this value will override
`stdio[0]`.
* `stdio` {string|Array} Child's stdio configuration. `stderr` by default will
be output to the parent process' stderr unless `stdio` is specified.
**Default:** `'pipe'`.
Expand Down Expand Up @@ -810,6 +820,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/22409
description: The `input` option can now be any `TypedArray` or a
`DataView`.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/21316
description: The `windowsHide` option now defaults to `true`.
Expand All @@ -831,8 +845,9 @@ changes:
* `args` {string[]} List of string arguments.
* `options` {Object}
* `cwd` {string} Current working directory of the child process.
* `input` {string|Buffer|Uint8Array} The value which will be passed as stdin
to the spawned process. Supplying this value will override `stdio[0]`.
* `input` {string|Buffer|TypedArray|DataView} The value which will be passed
as stdin to the spawned process. Supplying this value will override
`stdio[0]`.
* `argv0` {string} Explicitly set the value of `argv[0]` sent to the child
process. This will be set to `command` if not specified.
* `stdio` {string|Array} Child's stdio configuration.
Expand Down
4 changes: 2 additions & 2 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -1249,8 +1249,8 @@ For example when a function is expected to return a promise.
<a id="ERR_INVALID_SYNC_FORK_INPUT"></a>
### ERR_INVALID_SYNC_FORK_INPUT

A `Buffer`, `Uint8Array` or `string` was provided as stdio input to a
synchronous fork. See the documentation for the [`child_process`][] module
A `Buffer`, `TypedArray`, `DataView` or `string` was provided as stdio input to
an asynchronous fork. See the documentation for the [`child_process`][] module
for more information.

<a id="ERR_INVALID_THIS"></a>
Expand Down
9 changes: 6 additions & 3 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const util = require('util');
const {
deprecate, convertToValidSignal, getSystemErrorName
} = require('internal/util');
const { isUint8Array } = require('internal/util/types');
const { isArrayBufferView } = require('internal/util/types');
const debug = util.debuglog('child_process');
const { Buffer } = require('buffer');
const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
Expand Down Expand Up @@ -572,13 +572,16 @@ function spawnSync(/* file, args, options */) {
var input = options.stdio[i] && options.stdio[i].input;
if (input != null) {
var pipe = options.stdio[i] = util._extend({}, options.stdio[i]);
if (isUint8Array(input)) {
if (isArrayBufferView(input)) {
pipe.input = input;
} else if (typeof input === 'string') {
pipe.input = Buffer.from(input, options.encoding);
} else {
throw new ERR_INVALID_ARG_TYPE(`options.stdio[${i}]`,
['Buffer', 'Uint8Array', 'string'],
['Buffer',
'TypedArray',
'DataView',
'string'],
input);
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const { UDP } = process.binding('udp_wrap');
const SocketList = require('internal/socket_list');
const { owner_symbol } = require('internal/async_hooks').symbols;
const { convertToValidSignal } = require('internal/util');
const { isUint8Array } = require('internal/util/types');
const { isArrayBufferView } = require('internal/util/types');
const spawn_sync = process.binding('spawn_sync');
const { HTTPParser } = internalBinding('http_parser');
const { freeParser } = require('_http_common');
Expand Down Expand Up @@ -928,7 +928,7 @@ function _validateStdio(stdio, sync) {
wrapType: getHandleWrapType(handle),
handle: handle
});
} else if (isUint8Array(stdio) || typeof stdio === 'string') {
} else if (isArrayBufferView(stdio) || typeof stdio === 'string') {
if (!sync) {
cleanup();
throw new ERR_INVALID_SYNC_FORK_INPUT(util.inspect(stdio));
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,8 @@ E('ERR_INVALID_RETURN_VALUE', (input, name, value) => {
` function but got ${type}.`;
}, TypeError);
E('ERR_INVALID_SYNC_FORK_INPUT',
'Asynchronous forks do not support Buffer, Uint8Array or string input: %s',
'Asynchronous forks do not support ' +
'Buffer, TypedArray, DataView or string input: %s',
TypeError);
E('ERR_INVALID_THIS', 'Value of "this" must be of type %s', TypeError);
E('ERR_INVALID_TUPLE', '%s must be an iterable %s tuple', TypeError);
Expand Down
21 changes: 13 additions & 8 deletions test/parallel/test-child-process-spawnsync-input.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,16 +100,21 @@ checkSpawnSyncRet(ret);
assert.deepStrictEqual(ret.stdout, options.input);
assert.deepStrictEqual(ret.stderr, Buffer.from(''));

options = {
input: Uint8Array.from(Buffer.from('hello world'))
};
// common.getArrayBufferViews expects a buffer
// with length an multiple of 8
const msgBuf = Buffer.from('hello world'.repeat(8));
for (const arrayBufferView of common.getArrayBufferViews(msgBuf)) {
options = {
input: arrayBufferView
};

ret = spawnSync('cat', [], options);
ret = spawnSync('cat', [], options);

checkSpawnSyncRet(ret);
// Wrap options.input because Uint8Array and Buffer have different prototypes.
assert.deepStrictEqual(ret.stdout, Buffer.from(options.input));
assert.deepStrictEqual(ret.stderr, Buffer.from(''));
checkSpawnSyncRet(ret);

assert.deepStrictEqual(ret.stdout, msgBuf);
assert.deepStrictEqual(ret.stderr, Buffer.from(''));
}

verifyBufOutput(spawnSync(process.execPath, args));

Expand Down

0 comments on commit 68dff4a

Please sign in to comment.