-
Notifications
You must be signed in to change notification settings - Fork 28.9k
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: improve killSignal validations #10423
child_process: improve killSignal validations #10423
Conversation
How much does this overlap or conflict with #8312? |
#8312 validates the type of data passed as input to |
c6fb81b
to
4aab8fa
Compare
lib/internal/util.js
Outdated
return OS_SIGNALS[signal]; | ||
|
||
throw new Error('Unknown signal: ' + signal); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should avoid in
, hasOwnProperty()
, and unnecessarily checking the type again when the secondary condition fails. How about something like this:
exports.convertToValidSignal = function convertToValidSignal(signal) {
if (typeof signal === 'number') {
if (OS_SIGNALS_TO_NAMES_MAPPING[signal])
return signal;
} else if (typeof signal === 'string') {
var val = OS_SIGNALS[signal];
if (val !== undefined)
return val;
}
throw new Error('Unknown signal: ' + signal);
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unnecessarily checking the type again when the secondary condition fails
If the if
fails, then typeof
will be checked again, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used changes in the else if
part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, hasOwnProperty
check is necessary here. The undefined
check will pass if the key is an inherited property of OS_SIGNALS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set the prototype for OS_SIGNALS
to Object.create(null)
and it won't be a problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the test is necessary, there should be no inherited properties.
More to the point...add a unit test asserting that there are no inherited properties - try to .kill() with some strings which you think might be inherited method names, for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sam-github You mean this test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thefourtheye so, we are in agreement then that the has own property check is not necessary. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sam-github The test which I mentioned above asserts that an error is thrown if an inherited property is used. But if #10458 lands, then we can get rid of the hasOwnProperty
check.
lib/internal/child_process.js
Outdated
if (signal === undefined) { | ||
throw new Error('Unknown signal: ' + sig); | ||
} | ||
const signal = convertToValidSignal(sig === 0 ? 0 : (!sig ? 'SIGTERM' : sig)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about combining the ternary expressions?:
!sig && sig !== 0 ? 'SIGTERM' : sig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack
4aab8fa
to
22d71b5
Compare
22d71b5
to
25ded59
Compare
25ded59
to
492260a
Compare
492260a
to
0b51a05
Compare
Rebased. CI Run: https://ci.nodejs.org/job/node-test-pull-request/5877/ |
lib/internal/util.js
Outdated
@@ -1,8 +1,9 @@ | |||
'use strict'; | |||
|
|||
const binding = process.binding('util'); | |||
const prefix = `(${process.release.name}:${process.pid}) `; | |||
const OS_SIGNALS = process.binding('constants').os.signals; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you change this to camelCase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack!
lib/internal/util.js
Outdated
@@ -179,3 +180,18 @@ exports.toLength = function toLength(argument) { | |||
const len = toInteger(argument); | |||
return len <= 0 ? 0 : Math.min(len, Number.MAX_SAFE_INTEGER); | |||
}; | |||
|
|||
const OS_SIGNALS_TO_NAMES_MAPPING = Object.create(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment here regarding camelCase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack!
lib/internal/util.js
Outdated
@@ -179,3 +180,18 @@ exports.toLength = function toLength(argument) { | |||
const len = toInteger(argument); | |||
return len <= 0 ? 0 : Math.min(len, Number.MAX_SAFE_INTEGER); | |||
}; | |||
|
|||
const OS_SIGNALS_TO_NAMES_MAPPING = Object.create(null); | |||
Object.keys(OS_SIGNALS).forEach((signal) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should create this mapping lazily. It might help to switch to a for
loop to speed things up too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack!
@@ -190,7 +190,7 @@ if (!common.isWindows) { | |||
pass('killSignal', undefined); | |||
pass('killSignal', null); | |||
pass('killSignal', 'SIGKILL'); | |||
pass('killSignal', 500); | |||
fail('killSignal', 500, unknownSignalErr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add a case that passes with a number passed in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack!
@@ -0,0 +1,22 @@ | |||
'use strict'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could probably get away with dropping this test and add any necessary value checks to test-child-process-spawnsync-validation-errors.js
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack!
lib/internal/child_process.js
Outdated
if (signal === undefined) { | ||
throw new Error('Unknown signal: ' + sig); | ||
} | ||
const signal = convertToValidSignal(!sig && sig !== 0 ? 'SIGTERM' : sig); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should take this chance to change the check to sig == undefined
or sig === undefined
for defaulting to SIGTERM
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It falls back to SIGTERM
now also, right? !sig
will be true
when sig
is undefined
and it is not zero, so SIGTERM
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant we could tighten this up to not convert falsy values like false
or NaN
to 'SIGTERM'
if we wrote this like:
const signal = convertToValidSignal(sig === undefined ? 'SIGTERM' : sig);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love it. Simple and easy to reason. Updated the PR now.
lib/child_process.js
Outdated
|
||
if (options.killSignal === 0) | ||
throw new RangeError('"killSignal" cannot be 0'); | ||
|
||
options.killSignal = convertToValidSignal(options.killSignal); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you rebase this, can you move the convertToValidSignal()
calls into validateKillSignal()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack!
@thefourtheye this is looking pretty good. Are you still interested in working on it? |
0b51a05
to
9c46e6b
Compare
@cjihrig Yes :-) And I addressed your comments. PTAL again :-) |
9c46e6b
to
227bfd5
Compare
lib/child_process.js
Outdated
@@ -622,13 +611,8 @@ function validateMaxBuffer(maxBuffer) { | |||
|
|||
function validateKillSignal(killSignal) { | |||
if (typeof killSignal === 'string' || typeof killSignal === 'number') { | |||
killSignal = lookupSignal(killSignal); | |||
|
|||
if (killSignal === 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prior to this, zero could be passed to ChildProcess#kill()
, but not as the kill signal in execFile()
and company. That seems like something we might want to maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think zero is a special case only in ChildProcess#kill
. So I special cased it there.
lib/internal/util.js
Outdated
@@ -1,8 +1,9 @@ | |||
'use strict'; | |||
|
|||
const binding = process.binding('util'); | |||
const prefix = `(${process.release.name}:${process.pid}) `; | |||
const OsSignals = process.binding('constants').os.signals; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you call this osSignals
, or just signals
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack!
lib/internal/util.js
Outdated
} | ||
|
||
exports.convertToValidSignal = function convertToValidSignal(signal) { | ||
if (typeof signal === 'number' && signal in getSignalsToNamesMapping()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know there were previous comments about using in
and hasOwnProperty()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack! But this depends on #10458 now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only a couple small nits left from me.
lib/internal/util.js
Outdated
|
||
let signalsToNamesMapping; | ||
function getSignalsToNamesMapping() { | ||
if (signalsToNamesMapping === undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you reverse the if
logic and return if signalsToNamesMapping
exists. That can save some indentation on the following lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack!
Is this one ready to land? |
As it is, the `killSignal` is just retrieved from an object and used. If the signal passed is actually one of the inherited properties of that object, Node.js will die. For example, ➜ node -e "child_process.spawnSync('ls', {killSignal: 'toString'})" Assertion failed: (0), function uv_close, file ....core.c, line 166. [1] 58938 abort node -e "child_process.spawnSync(...)" 1. This patch makes sure that the signal is actually a own property of the constants object. 2. Extends the killSignal validation to all the other functions.
3956b2e
to
174a984
Compare
@thefourtheye Is this still |
@thefourtheye Do you want to go ahead and land this then? :) |
Landed in d75fdd9 |
As it is, the `killSignal` is just retrieved from an object and used. If the signal passed is actually one of the inherited properties of that object, Node.js will die. For example, ➜ node -e "child_process.spawnSync('ls', {killSignal: 'toString'})" Assertion failed: (0), function uv_close, file ....core.c, line 166. [1] 58938 abort node -e "child_process.spawnSync(...)" 1. This patch makes sure that the signal is actually a own property of the constants object. 2. Extends the killSignal validation to all the other functions. PR-URL: #10423 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
* **Async Hooks** * The `async_hooks` module has landed in core [[`4a7233c178`](nodejs@4a7233c178)] [nodejs#12892](nodejs#12892). * **Buffer** * Using the `--pending-deprecation` flag will cause Node.js to emit a deprecation warning when using `new Buffer(num)` or `Buffer(num)`. [[`d2d32ea5a2`](nodejs@d2d32ea5a2)] [nodejs#11968](nodejs#11968). * `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances [[`7eb1b4658e`](nodejs@7eb1b4658e)] [nodejs#12141](nodejs#12141). * Many `Buffer` methods now accept `Uint8Array` as input [[`beca3244e2`](nodejs@beca3244e2)] [nodejs#10236](nodejs#10236). * **Child Process** * Argument and kill signal validations have been improved [[`97a77288ce`](nodejs@97a77288ce)] [nodejs#12348](nodejs#12348), [[`d75fdd96aa`](nodejs@d75fdd96aa)] [nodejs#10423](nodejs#10423). * Child Process methods accept `Uint8Array` as input [[`627ecee9ed`](nodejs@627ecee9ed)] [nodejs#10653](nodejs#10653). * **Console** * Error events emitted when using `console` methods are now supressed. [[`f18e08d820`](nodejs@f18e08d820)] [nodejs#9744](nodejs#9744). * **Dependencies** * The npm client has been updated to 5.0.0 [[`3c3b36af0f`](nodejs@3c3b36af0f)] [nodejs#12936](nodejs#12936). * V8 has been updated to 5.8 with forward ABI stability to 6.0 [[`60d1aac8d2`](nodejs@60d1aac8d2)] [nodejs#12784](nodejs#12784). * **Domains** * Native `Promise` instances are now `Domain` aware [[`84dabe8373`](nodejs@84dabe8373)] [nodejs#12489](nodejs#12489). * **Errors** * We have started assigning static error codes to errors generated by Node.js. This has been done through multiple commits and is still a work in progress. * **File System** * The utility class `fs.SyncWriteStream` has been deprecated [[`7a55e34ef4`](nodejs@7a55e34ef4)] [nodejs#10467](nodejs#10467). * The deprecated `fs.read()` string interface has been removed [[`3c2a9361ff`](nodejs@3c2a9361ff)] [nodejs#9683](nodejs#9683). * **HTTP** * Improved support for userland implemented Agents [[`90403dd1d0`](nodejs@90403dd1d0)] [nodejs#11567](nodejs#11567). * Outgoing Cookie headers are concatenated into a single string [[`d3480776c7`](nodejs@d3480776c7)] [nodejs#11259](nodejs#11259). * The `httpResponse.writeHeader()` method has been deprecated [[`fb71ba4921`](nodejs@fb71ba4921)] [nodejs#11355](nodejs#11355). * New methods for accessing HTTP headers have been added to `OutgoingMessage` [[`3e6f1032a4`](nodejs@3e6f1032a4)] [nodejs#10805](nodejs#10805). * **Lib** * All deprecation messages have been assigned static identifiers [[`5de3cf099c`](nodejs@5de3cf099c)] [nodejs#10116](nodejs#10116). * The legacy `linkedlist` module has been removed [[`84a23391f6`](nodejs@84a23391f6)] [nodejs#12113](nodejs#12113). * **N-API** * Experimental support for the new N-API API has been added [[`56e881d0b0`](nodejs@56e881d0b0)] [nodejs#11975](nodejs#11975). * **Process** * Process warning output can be redirected to a file using the `--redirect-warnings` command-line argument [[`03e89b3ff2`](nodejs@03e89b3ff2)] [nodejs#10116](nodejs#10116). * Process warnings may now include additional detail [[`dd20e68b0f`](nodejs@dd20e68b0f)] [nodejs#12725](nodejs#12725). * **REPL** * REPL magic mode has been deprecated [[`3f27f02da0`](nodejs@3f27f02da0)] [nodejs#11599](nodejs#11599). * **Src** * `NODE_MODULE_VERSION` has been updated to 57 (nodejs@ec7cbaf266)] [nodejs#12995](nodejs#12995). * Add `--pending-deprecation` command-line argument and `NODE_PENDING_DEPRECATION` environment variable [[`a16b570f8c`](nodejs@a16b570f8c)] [nodejs#11968](nodejs#11968). * The `--debug` command-line argument has been deprecated. Note that using `--debug` will enable the *new* Inspector-based debug protocol as the legacy Debugger protocol previously used by Node.js has been removed. [[`010f864426`](nodejs@010f864426)] [nodejs#12949](nodejs#12949). * Throw when the `-c` and `-e` command-line arguments are used at the same time [[`a5f91ab230`](nodejs@a5f91ab230)] [nodejs#11689](nodejs#11689). * Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line arguments are used at the same time. [[`8a7db9d4b5`](nodejs@8a7db9d4b5)] [nodejs#12087](nodejs#12087). * **Stream** * `Stream` now supports `destroy()` and `_destroy()` APIs [[`b6e1d22fa6`](nodejs@b6e1d22fa6)] [nodejs#12925](nodejs#12925). * `Stream` now supports the `_final()` API [[`07c7f198db`](nodejs@07c7f198db)] [nodejs#12828](nodejs#12828). * **TLS** * The `rejectUnauthorized` option now defaults to `true` [[`348cc80a3c`](nodejs@348cc80a3c)] [nodejs#5923](nodejs#5923). * The `tls.createSecurePair()` API now emits a runtime deprecation [[`a2ae08999b`](nodejs@a2ae08999b)] [nodejs#11349](nodejs#11349). * A runtime deprecation will now be emitted when `dhparam` is less than 2048 bits [[`d523eb9c40`](nodejs@d523eb9c40)] [nodejs#11447](nodejs#11447). * **URL** * The WHATWG URL implementation is now a fully-supported Node.js API [[`d080ead0f9`](nodejs@d080ead0f9)] [nodejs#12710](nodejs#12710). * **Util** * `Symbol` keys are now displayed by default when using `util.inspect()` [[`5bfd13b81e`](nodejs@5bfd13b81e)] [nodejs#9726](nodejs#9726). * `toJSON` errors will be thrown when formatting `%j` [[`455e6f1dd8`](nodejs@455e6f1dd8)] [nodejs#11708](nodejs#11708). * Convert `inspect.styles` and `inspect.colors` to prototype-less objects [[`aab0d202f8`](nodejs@aab0d202f8)] [nodejs#11624](nodejs#11624). * The new `util.promisify()` API has been added [[`99da8e8e02`](nodejs@99da8e8e02)] [nodejs#12442](nodejs#12442). * **Zlib** * Support `Uint8Array` in Zlib convenience methods [[`91383e47fd`](nodejs@91383e47fd)] [nodejs#12001](nodejs#12001). * Zlib errors now use `RangeError` and `TypeError` consistently [[`b514bd231e`](nodejs@b514bd231e)] [nodejs#11391](nodejs#11391).
* **Async Hooks** * The `async_hooks` module has landed in core [[`4a7233c178`](4a7233c178)] [#12892](#12892). * **Buffer** * Using the `--pending-deprecation` flag will cause Node.js to emit a deprecation warning when using `new Buffer(num)` or `Buffer(num)`. [[`d2d32ea5a2`](d2d32ea5a2)] [#11968](#11968). * `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances [[`7eb1b4658e`](7eb1b4658e)] [#12141](#12141). * Many `Buffer` methods now accept `Uint8Array` as input [[`beca3244e2`](beca3244e2)] [#10236](#10236). * **Child Process** * Argument and kill signal validations have been improved [[`97a77288ce`](97a77288ce)] [#12348](#12348), [[`d75fdd96aa`](d75fdd96aa)] [#10423](#10423). * Child Process methods accept `Uint8Array` as input [[`627ecee9ed`](627ecee9ed)] [#10653](#10653). * **Console** * Error events emitted when using `console` methods are now supressed. [[`f18e08d820`](f18e08d820)] [#9744](#9744). * **Dependencies** * The npm client has been updated to 5.0.0 [[`3c3b36af0f`](3c3b36af0f)] [#12936](#12936). * V8 has been updated to 5.8 with forward ABI stability to 6.0 [[`60d1aac8d2`](60d1aac8d2)] [#12784](#12784). * **Domains** * Native `Promise` instances are now `Domain` aware [[`84dabe8373`](84dabe8373)] [#12489](#12489). * **Errors** * We have started assigning static error codes to errors generated by Node.js. This has been done through multiple commits and is still a work in progress. * **File System** * The utility class `fs.SyncWriteStream` has been deprecated [[`7a55e34ef4`](7a55e34ef4)] [#10467](#10467). * The deprecated `fs.read()` string interface has been removed [[`3c2a9361ff`](3c2a9361ff)] [#9683](#9683). * **HTTP** * Improved support for userland implemented Agents [[`90403dd1d0`](90403dd1d0)] [#11567](#11567). * Outgoing Cookie headers are concatenated into a single string [[`d3480776c7`](d3480776c7)] [#11259](#11259). * The `httpResponse.writeHeader()` method has been deprecated [[`fb71ba4921`](fb71ba4921)] [#11355](#11355). * New methods for accessing HTTP headers have been added to `OutgoingMessage` [[`3e6f1032a4`](3e6f1032a4)] [#10805](#10805). * **Lib** * All deprecation messages have been assigned static identifiers [[`5de3cf099c`](5de3cf099c)] [#10116](#10116). * The legacy `linkedlist` module has been removed [[`84a23391f6`](84a23391f6)] [#12113](#12113). * **N-API** * Experimental support for the new N-API API has been added [[`56e881d0b0`](56e881d0b0)] [#11975](#11975). * **Process** * Process warning output can be redirected to a file using the `--redirect-warnings` command-line argument [[`03e89b3ff2`](03e89b3ff2)] [#10116](#10116). * Process warnings may now include additional detail [[`dd20e68b0f`](dd20e68b0f)] [#12725](#12725). * **REPL** * REPL magic mode has been deprecated [[`3f27f02da0`](3f27f02da0)] [#11599](#11599). * **Src** * `NODE_MODULE_VERSION` has been updated to 57 (ec7cbaf266)] [#12995](#12995). * Add `--pending-deprecation` command-line argument and `NODE_PENDING_DEPRECATION` environment variable [[`a16b570f8c`](a16b570f8c)] [#11968](#11968). * The `--debug` command-line argument has been deprecated. Note that using `--debug` will enable the *new* Inspector-based debug protocol as the legacy Debugger protocol previously used by Node.js has been removed. [[`010f864426`](010f864426)] [#12949](#12949). * Throw when the `-c` and `-e` command-line arguments are used at the same time [[`a5f91ab230`](a5f91ab230)] [#11689](#11689). * Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line arguments are used at the same time. [[`8a7db9d4b5`](8a7db9d4b5)] [#12087](#12087). * **Stream** * `Stream` now supports `destroy()` and `_destroy()` APIs [[`b6e1d22fa6`](b6e1d22fa6)] [#12925](#12925). * `Stream` now supports the `_final()` API [[`07c7f198db`](07c7f198db)] [#12828](#12828). * **TLS** * The `rejectUnauthorized` option now defaults to `true` [[`348cc80a3c`](348cc80a3c)] [#5923](#5923). * The `tls.createSecurePair()` API now emits a runtime deprecation [[`a2ae08999b`](a2ae08999b)] [#11349](#11349). * A runtime deprecation will now be emitted when `dhparam` is less than 2048 bits [[`d523eb9c40`](d523eb9c40)] [#11447](#11447). * **URL** * The WHATWG URL implementation is now a fully-supported Node.js API [[`d080ead0f9`](d080ead0f9)] [#12710](#12710). * **Util** * `Symbol` keys are now displayed by default when using `util.inspect()` [[`5bfd13b81e`](5bfd13b81e)] [#9726](#9726). * `toJSON` errors will be thrown when formatting `%j` [[`455e6f1dd8`](455e6f1dd8)] [#11708](#11708). * Convert `inspect.styles` and `inspect.colors` to prototype-less objects [[`aab0d202f8`](aab0d202f8)] [#11624](#11624). * The new `util.promisify()` API has been added [[`99da8e8e02`](99da8e8e02)] [#12442](#12442). * **Zlib** * Support `Uint8Array` in Zlib convenience methods [[`91383e47fd`](91383e47fd)] [#12001](#12001). * Zlib errors now use `RangeError` and `TypeError` consistently [[`b514bd231e`](b514bd231e)] [#11391](#11391).
* **Async Hooks** * The `async_hooks` module has landed in core [[`4a7233c178`](4a7233c178)] [#12892](#12892). * **Buffer** * Using the `--pending-deprecation` flag will cause Node.js to emit a deprecation warning when using `new Buffer(num)` or `Buffer(num)`. [[`d2d32ea5a2`](d2d32ea5a2)] [#11968](#11968). * `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances [[`7eb1b4658e`](7eb1b4658e)] [#12141](#12141). * Many `Buffer` methods now accept `Uint8Array` as input [[`beca3244e2`](beca3244e2)] [#10236](#10236). * **Child Process** * Argument and kill signal validations have been improved [[`97a77288ce`](97a77288ce)] [#12348](#12348), [[`d75fdd96aa`](d75fdd96aa)] [#10423](#10423). * Child Process methods accept `Uint8Array` as input [[`627ecee9ed`](627ecee9ed)] [#10653](#10653). * **Console** * Error events emitted when using `console` methods are now supressed. [[`f18e08d820`](f18e08d820)] [#9744](#9744). * **Dependencies** * The npm client has been updated to 5.0.0 [[`3c3b36af0f`](3c3b36af0f)] [#12936](#12936). * V8 has been updated to 5.8 with forward ABI stability to 6.0 [[`60d1aac8d2`](60d1aac8d2)] [#12784](#12784). * **Domains** * Native `Promise` instances are now `Domain` aware [[`84dabe8373`](84dabe8373)] [#12489](#12489). * **Errors** * We have started assigning static error codes to errors generated by Node.js. This has been done through multiple commits and is still a work in progress. * **File System** * The utility class `fs.SyncWriteStream` has been deprecated [[`7a55e34ef4`](7a55e34ef4)] [#10467](#10467). * The deprecated `fs.read()` string interface has been removed [[`3c2a9361ff`](3c2a9361ff)] [#9683](#9683). * **HTTP** * Improved support for userland implemented Agents [[`90403dd1d0`](90403dd1d0)] [#11567](#11567). * Outgoing Cookie headers are concatenated into a single string [[`d3480776c7`](d3480776c7)] [#11259](#11259). * The `httpResponse.writeHeader()` method has been deprecated [[`fb71ba4921`](fb71ba4921)] [#11355](#11355). * New methods for accessing HTTP headers have been added to `OutgoingMessage` [[`3e6f1032a4`](3e6f1032a4)] [#10805](#10805). * **Lib** * All deprecation messages have been assigned static identifiers [[`5de3cf099c`](5de3cf099c)] [#10116](#10116). * The legacy `linkedlist` module has been removed [[`84a23391f6`](84a23391f6)] [#12113](#12113). * **N-API** * Experimental support for the new N-API API has been added [[`56e881d0b0`](56e881d0b0)] [#11975](#11975). * **Process** * Process warning output can be redirected to a file using the `--redirect-warnings` command-line argument [[`03e89b3ff2`](03e89b3ff2)] [#10116](#10116). * Process warnings may now include additional detail [[`dd20e68b0f`](dd20e68b0f)] [#12725](#12725). * **REPL** * REPL magic mode has been deprecated [[`3f27f02da0`](3f27f02da0)] [#11599](#11599). * **Src** * `NODE_MODULE_VERSION` has been updated to 57 (ec7cbaf266)] [#12995](#12995). * Add `--pending-deprecation` command-line argument and `NODE_PENDING_DEPRECATION` environment variable [[`a16b570f8c`](a16b570f8c)] [#11968](#11968). * The `--debug` command-line argument has been deprecated. Note that using `--debug` will enable the *new* Inspector-based debug protocol as the legacy Debugger protocol previously used by Node.js has been removed. [[`010f864426`](010f864426)] [#12949](#12949). * Throw when the `-c` and `-e` command-line arguments are used at the same time [[`a5f91ab230`](a5f91ab230)] [#11689](#11689). * Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line arguments are used at the same time. [[`8a7db9d4b5`](8a7db9d4b5)] [#12087](#12087). * **Stream** * `Stream` now supports `destroy()` and `_destroy()` APIs [[`b6e1d22fa6`](b6e1d22fa6)] [#12925](#12925). * `Stream` now supports the `_final()` API [[`07c7f198db`](07c7f198db)] [#12828](#12828). * **TLS** * The `rejectUnauthorized` option now defaults to `true` [[`348cc80a3c`](348cc80a3c)] [#5923](#5923). * The `tls.createSecurePair()` API now emits a runtime deprecation [[`a2ae08999b`](a2ae08999b)] [#11349](#11349). * A runtime deprecation will now be emitted when `dhparam` is less than 2048 bits [[`d523eb9c40`](d523eb9c40)] [#11447](#11447). * **URL** * The WHATWG URL implementation is now a fully-supported Node.js API [[`d080ead0f9`](d080ead0f9)] [#12710](#12710). * **Util** * `Symbol` keys are now displayed by default when using `util.inspect()` [[`5bfd13b81e`](5bfd13b81e)] [#9726](#9726). * `toJSON` errors will be thrown when formatting `%j` [[`455e6f1dd8`](455e6f1dd8)] [#11708](#11708). * Convert `inspect.styles` and `inspect.colors` to prototype-less objects [[`aab0d202f8`](aab0d202f8)] [#11624](#11624). * The new `util.promisify()` API has been added [[`99da8e8e02`](99da8e8e02)] [#12442](#12442). * **Zlib** * Support `Uint8Array` in Zlib convenience methods [[`91383e47fd`](91383e47fd)] [#12001](#12001). * Zlib errors now use `RangeError` and `TypeError` consistently [[`b514bd231e`](b514bd231e)] [#11391](#11391).
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
child_process
Description of change
As it is, the
killSignal
is just retrieved from an object and used.If the signal passed is actually one of the inherited properties of
that object, Node.js will die. For example,
This patch makes sure that the signal is actually a own property of
the constants object.
Extends the killSignal validation to all the other functions.