GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — child_process: improve killSignal validations · nodejs/node@d75fdd9

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

lib
- child_process.js
- internal
  - child_process.js
  - util.js
test/parallel
- test-child-process-spawnsync-validation-errors.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

`‎lib/child_process.js‎`

Copy file name to clipboardExpand all lines: lib/child_process.js

+7-23Lines changed: 7 additions & 23 deletions

Original file line number	Diff line number	Diff line change
`@@ -22,9 +22,8 @@`
`22`	`22`	`'use strict';`
`23`	`23`
`24`	`24`	`const util = require('util');`
`25`		`-const internalUtil = require('internal/util');`
	`25`	`+const { deprecate, convertToValidSignal } = require('internal/util');`
`26`	`26`	`const debug = util.debuglog('child_process');`
`27`		`-const constants = process.binding('constants').os.signals;`
`28`	`27`
`29`	`28`	`const uv = process.binding('uv');`
`30`	`29`	`const spawn_sync = process.binding('spawn_sync');`
`@@ -181,6 +180,8 @@ exports.execFile = function(file /, args, options, callback/) {`
`181`	`180`	`// Validate maxBuffer, if present.`
`182`	`181`	`validateMaxBuffer(options.maxBuffer);`
`183`	`182`
	`183`	`+ options.killSignal = sanitizeKillSignal(options.killSignal);`
	`184`	`+`
`184`	`185`	`var child = spawn(file, args, {`
`185`	`186`	`cwd: options.cwd,`
`186`	`187`	`env: options.env,`
`@@ -332,7 +333,7 @@ exports.execFile = function(file /, args, options, callback/) {`
`332`	`333`	`return child;`
`333`	`334`	`};`
`334`	`335`
`335`		`-const _deprecatedCustomFds = internalUtil.deprecate(`
	`336`	`+const _deprecatedCustomFds = deprecate(`
`336`	`337`	`function deprecateCustomFds(options) {`
`337`	`338`	`options.stdio = options.customFds.map(function mapCustomFds(fd) {`
`338`	`339`	`return fd === -1 ? 'pipe' : fd;`
`@@ -474,18 +475,6 @@ var spawn = exports.spawn = function(/file, args, options/) {`
`474`	`475`	`return child;`
`475`	`476`	`};`
`476`	`477`
`477`		`-`
`478`		`-function lookupSignal(signal) {`
`479`		`- if (typeof signal === 'number')`
`480`		`- return signal;`
`481`		`-`
`482`		`- if (!(signal in constants))`
`483`		`- throw new Error('Unknown signal: ' + signal);`
`484`		`-`
`485`		`- return constants[signal];`
`486`		`-}`
`487`		`-`
`488`		`-`
`489`	`478`	`function spawnSync(/file, args, options/) {`
`490`	`479`	`var opts = normalizeSpawnArguments.apply(null, arguments);`
`491`	`480`
`@@ -506,7 +495,7 @@ function spawnSync(/file, args, options/) {`
`506`	`495`	`options.envPairs = opts.envPairs;`
`507`	`496`
`508`	`497`	`// Validate and translate the kill signal, if present.`
`509`		`- options.killSignal = validateKillSignal(options.killSignal);`
	`498`	`+ options.killSignal = sanitizeKillSignal(options.killSignal);`
`510`	`499`
`511`	`500`	`options.stdio = _validateStdio(options.stdio \|\| 'pipe', true).stdio;`
`512`	`501`
`@@ -632,15 +621,10 @@ function validateMaxBuffer(maxBuffer) {`
`632`	`621`	`}`
`633`	`622`
`634`	`623`
`635`		`-function validateKillSignal(killSignal) {`
	`624`	`+function sanitizeKillSignal(killSignal) {`
`636`	`625`	`if (typeof killSignal === 'string' \|\| typeof killSignal === 'number') {`
`637`		`- killSignal = lookupSignal(killSignal);`
`638`		`-`
`639`		`- if (killSignal === 0)`
`640`		`- throw new RangeError('"killSignal" cannot be 0');`
	`626`	`+ return convertToValidSignal(killSignal);`
`641`	`627`	`} else if (killSignal != null) {`
`642`	`628`	`throw new TypeError('"killSignal" must be a string or number');`
`643`	`629`	`}`
`644`		`-`
`645`		`- return killSignal;`
`646`	`630`	`}`

Collapse file

`‎lib/internal/child_process.js‎`

Copy file name to clipboardExpand all lines: lib/internal/child_process.js

+3-14Lines changed: 3 additions & 14 deletions

Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,6 @@ const EventEmitter = require('events');`
`5`	`5`	`const net = require('net');`
`6`	`6`	`const dgram = require('dgram');`
`7`	`7`	`const util = require('util');`
`8`		`-const constants = process.binding('constants').os.signals;`
`9`	`8`	`const assert = require('assert');`
`10`	`9`
`11`	`10`	`const Process = process.binding('process_wrap').Process;`
`@@ -17,6 +16,7 @@ const TCP = process.binding('tcp_wrap').TCP;`
`17`	`16`	`const UDP = process.binding('udp_wrap').UDP;`
`18`	`17`	`const SocketList = require('internal/socket_list');`
`19`	`18`	`const { isUint8Array } = process.binding('util');`
	`19`	`+const { convertToValidSignal } = require('internal/util');`
`20`	`20`
`21`	`21`	`const errnoException = util._errnoException;`
`22`	`22`	`const SocketListSend = SocketList.SocketListSend;`
`@@ -362,19 +362,8 @@ function onErrorNT(self, err) {`
`362`	`362`
`363`	`363`
`364`	`364`	`ChildProcess.prototype.kill = function(sig) {`
`365`		`- var signal;`
`366`		`-`
`367`		`- if (sig === 0) {`
`368`		`- signal = 0;`
`369`		`- } else if (!sig) {`
`370`		`- signal = constants['SIGTERM'];`
`371`		`- } else {`
`372`		`- signal = constants[sig];`
`373`		`- }`
`374`		`-`
`375`		`- if (signal === undefined) {`
`376`		`- throw new Error('Unknown signal: ' + sig);`
`377`		`- }`
	`365`	`+ const signal = sig === 0 ? sig :`
	`366`	`+ convertToValidSignal(sig === undefined ? 'SIGTERM' : sig);`
`378`	`367`
`379`	`368`	`if (this._handle) {`
`380`	`369`	`var err = this._handle.kill(signal);`

Collapse file

`‎lib/internal/util.js‎`

Copy file name to clipboardExpand all lines: lib/internal/util.js

+26Lines changed: 26 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,7 @@`
`1`	`1`	`'use strict';`
`2`	`2`
`3`	`3`	`const binding = process.binding('util');`
	`4`	`+const signals = process.binding('constants').os.signals;`
`4`	`5`
`5`	`6`	`const kArrowMessagePrivateSymbolIndex = binding['arrow_message_private_symbol'];`
`6`	`7`	`const kDecoratedPrivateSymbolIndex = binding['decorated_private_symbol'];`
`@@ -179,3 +180,28 @@ exports.createClassWrapper = function createClassWrapper(type) {`
`179`	`180`	`fn.prototype = type.prototype;`
`180`	`181`	`return fn;`
`181`	`182`	`};`
	`183`	`+`
	`184`	`+let signalsToNamesMapping;`
	`185`	`+function getSignalsToNamesMapping() {`
	`186`	`+ if (signalsToNamesMapping !== undefined)`
	`187`	`+ return signalsToNamesMapping;`
	`188`	`+`
	`189`	`+ signalsToNamesMapping = Object.create(null);`
	`190`	`+ for (const key in signals) {`
	`191`	`+ signalsToNamesMapping[signals[key]] = key;`
	`192`	`+ }`
	`193`	`+`
	`194`	`+ return signalsToNamesMapping;`
	`195`	`+}`
	`196`	`+`
	`197`	`+exports.convertToValidSignal = function convertToValidSignal(signal) {`
	`198`	`+ if (typeof signal === 'number' && getSignalsToNamesMapping()[signal])`
	`199`	`+ return signal;`
	`200`	`+`
	`201`	`+ if (typeof signal === 'string') {`
	`202`	`+ const signalName = signals[signal.toUpperCase()];`
	`203`	`+ if (signalName) return signalName;`
	`204`	`+ }`
	`205`	`+`
	`206`	`+ throw new Error('Unknown signal: ' + signal);`
	`207`	`+};`

Collapse file

`‎test/parallel/test-child-process-spawnsync-validation-errors.js‎`

Copy file name to clipboardExpand all lines: test/parallel/test-child-process-spawnsync-validation-errors.js

+18-3Lines changed: 18 additions & 3 deletions

Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,7 @@`
`2`	`2`	`const common = require('../common');`
`3`	`3`	`const assert = require('assert');`
`4`	`4`	`const spawnSync = require('child_process').spawnSync;`
	`5`	`+const signals = process.binding('constants').os.signals;`
`5`	`6`
`6`	`7`	`function pass(option, value) {`
`7`	`8`	`// Run the command with the specified option. Since it's not a real command,`
`@@ -184,18 +185,32 @@ if (!common.isWindows) {`
`184`	`185`	`{`
`185`	`186`	`// Validate the killSignal option`
`186`	`187`	`const typeErr = /^TypeError: "killSignal" must be a string or number$/;`
`187`		`- const rangeErr = /^RangeError: "killSignal" cannot be 0$/;`
`188`	`188`	`const unknownSignalErr = /^Error: Unknown signal:/;`
`189`	`189`
`190`	`190`	`pass('killSignal', undefined);`
`191`	`191`	`pass('killSignal', null);`
`192`	`192`	`pass('killSignal', 'SIGKILL');`
`193`		`- pass('killSignal', 500);`
`194`		`- fail('killSignal', 0, rangeErr);`
`195`	`193`	`fail('killSignal', 'SIGNOTAVALIDSIGNALNAME', unknownSignalErr);`
`196`	`194`	`fail('killSignal', true, typeErr);`
`197`	`195`	`fail('killSignal', false, typeErr);`
`198`	`196`	`fail('killSignal', [], typeErr);`
`199`	`197`	`fail('killSignal', {}, typeErr);`
`200`	`198`	`fail('killSignal', common.noop, typeErr);`
	`199`	`+`
	`200`	`+ // Invalid signal names and numbers should fail`
	`201`	`+ fail('killSignal', 500, unknownSignalErr);`
	`202`	`+ fail('killSignal', 0, unknownSignalErr);`
	`203`	`+ fail('killSignal', -200, unknownSignalErr);`
	`204`	`+ fail('killSignal', 3.14, unknownSignalErr);`
	`205`	`+`
	`206`	`+ Object.getOwnPropertyNames(Object.prototype).forEach((property) => {`
	`207`	`+ fail('killSignal', property, unknownSignalErr);`
	`208`	`+ });`
	`209`	`+`
	`210`	`+ // Valid signal names and numbers should pass`
	`211`	`+ for (const signalName in signals) {`
	`212`	`+ pass('killSignal', signals[signalName]);`
	`213`	`+ pass('killSignal', signalName);`
	`214`	`+ pass('killSignal', signalName.toLowerCase());`
	`215`	`+ }`
`201`	`216`	`}`

0 commit comments

Comments

0 (0)

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.