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: improve killSignal validations #10423

Conversation

thefourtheye
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected 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,

➜  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.

@nodejs-github-bot nodejs-github-bot added child_process Issues and PRs related to the child_process subsystem. util Issues and PRs related to the built-in util module. labels Dec 23, 2016
@bnoordhuis
Copy link
Member

How much does this overlap or conflict with #8312?

@thefourtheye
Copy link
Contributor Author

#8312 validates the type of data passed as input to killSignal, but this patch fixes the problem in lookupSignal method itself.

@thefourtheye thefourtheye force-pushed the child-process-signal-validation branch 2 times, most recently from c6fb81b to 4aab8fa Compare December 23, 2016 12:32
return OS_SIGNALS[signal];

throw new Error('Unknown signal: ' + signal);
};
Copy link
Contributor

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);
};

Copy link
Contributor Author

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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

Copy link
Contributor

@mscdex mscdex Dec 26, 2016

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mscdex I think it would be possible if something like #10458 lands.

Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

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. 👍

Copy link
Contributor Author

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.

if (signal === undefined) {
throw new Error('Unknown signal: ' + sig);
}
const signal = convertToValidSignal(sig === 0 ? 0 : (!sig ? 'SIGTERM' : sig));
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack

@thefourtheye
Copy link
Contributor Author

@@ -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;
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack!

@@ -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);
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack!

@@ -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) => {
Copy link
Contributor

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.

Copy link
Contributor Author

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);
Copy link
Contributor

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.

Copy link
Contributor Author

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';
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack!

if (signal === undefined) {
throw new Error('Unknown signal: ' + sig);
}
const signal = convertToValidSignal(!sig && sig !== 0 ? 'SIGTERM' : sig);
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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);

Copy link
Contributor Author

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.


if (options.killSignal === 0)
throw new RangeError('"killSignal" cannot be 0');

options.killSignal = convertToValidSignal(options.killSignal);
Copy link
Contributor

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().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack!

@cjihrig
Copy link
Contributor

cjihrig commented Feb 11, 2017

@thefourtheye this is looking pretty good. Are you still interested in working on it?

@thefourtheye
Copy link
Contributor Author

@cjihrig Yes :-) And I addressed your comments. PTAL again :-)

@@ -622,13 +611,8 @@ function validateMaxBuffer(maxBuffer) {

function validateKillSignal(killSignal) {
if (typeof killSignal === 'string' || typeof killSignal === 'number') {
killSignal = lookupSignal(killSignal);

if (killSignal === 0)
Copy link
Contributor

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.

Copy link
Contributor Author

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.

@@ -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;
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack!

}

exports.convertToValidSignal = function convertToValidSignal(signal) {
if (typeof signal === 'number' && signal in getSignalsToNamesMapping())
Copy link
Contributor

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().

Copy link
Contributor Author

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

Copy link
Contributor

@cjihrig cjihrig left a 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.


let signalsToNamesMapping;
function getSignalsToNamesMapping() {
if (signalsToNamesMapping === undefined) {
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack!

@jasnell
Copy link
Member

jasnell commented Mar 22, 2017

Is this one ready to land?

@cjihrig
Copy link
Contributor

cjihrig commented Mar 22, 2017

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.
@thefourtheye thefourtheye force-pushed the child-process-signal-validation branch from 3956b2e to 174a984 Compare April 1, 2017 08:56
@thefourtheye
Copy link
Contributor Author

@thefourtheye
Copy link
Contributor Author

@jasnell @cjihrig I updated the PR so that the signal names are case insensitive. PTAL.

@addaleax
Copy link
Member

addaleax commented Apr 1, 2017

@thefourtheye Is this still blocked? By what?

@thefourtheye thefourtheye removed the blocked PRs that are blocked by other issues or PRs. label Apr 2, 2017
@thefourtheye
Copy link
Contributor Author

@addaleax It was blocked by #10458, now that it landed, this is okay.

@addaleax addaleax added this to the 8.0.0 milestone Apr 2, 2017
@addaleax
Copy link
Member

addaleax commented Apr 2, 2017

@thefourtheye Do you want to go ahead and land this then? :)

@thefourtheye
Copy link
Contributor Author

Landed in d75fdd9

@thefourtheye thefourtheye deleted the child-process-signal-validation branch April 4, 2017 07:36
thefourtheye added a commit that referenced this pull request Apr 4, 2017
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]>
@jasnell jasnell mentioned this pull request Apr 4, 2017
jasnell added a commit to jasnell/node that referenced this pull request May 29, 2017
* **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).
jasnell added a commit that referenced this pull request May 30, 2017
* **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).
jasnell added a commit that referenced this pull request May 30, 2017
* **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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants