PR-URL: https://github.com/nodejs/node/pull/24751


Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Notable Changes:

This is a patch release to address a bad backport of the fix for "Slowloris
HTTP Denial of Service" (CVE-2018-12122). Node.js 6.15.0 misapplies the headers
timeout to an entire keep-alive HTTP session, resulting in prematurely
disconnected sockets.

PR-URL: https://github.com/nodejs/node/pull/24803
Refs: https://github.com/nodejs/node/pull/24796
Refs: https://github.com/nodejs/node/issues/24760


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/24749


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

Comparing sparse arrays did not work properly. That is fixed and
tests were added to verify that everything works as expected.

This had an impact on `util.isDeepStrictEqual()` and
`assert.deepStrictEqual()` and their counterpart
`assert.notDeepStrictEqual()`.

PR-URL: https://github.com/nodejs/node/pull/24749


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

Loose map comparison had an logic error. It will now be properly
compared.

PR-URL: https://github.com/nodejs/node/pull/24749


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

The `util.format()` behavior changed recently. Add the changes entry
to document the new BigInt behavior.

PR-URL: https://github.com/nodejs/node/pull/24758


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

IDEs like vscode use older Node.js versions that do not yet support
the new try catch syntax. This makes sure eslint continues to work
in these IDEs as before.

PR-URL: https://github.com/nodejs/node/pull/24753


Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

The current internal isError function checked the toString value
instead of using the more precise `util.types.isNativeError()` check.
The `instanceof` check is not removed due to possible errors that
are not native but still an instance of Error.

PR-URL: https://github.com/nodejs/node/pull/24746


Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

The `console` functions rely on the `util.format()` behavior. It
did not follow the whatwg spec when it comes to symbols in combination
with the %d, %i and %f format specifiers. Using a symbol argument in
combination with one of these specifiers resulted in an error instead
of returning `'NaN'`. This is now fixed by this patch.

PR-URL: https://github.com/nodejs/node/pull/23708
Refs: https://console.spec.whatwg.org/#formatter


Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

This commit adds a test for the validateArguments function in
TextDecoder.

PR-URL: https://github.com/nodejs/node/pull/24736


Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/24745


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Add an error code property to invalid `secureProtocol` method
exceptions.

PR-URL: https://github.com/nodejs/node/pull/24729


Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/24719
Refs: https://github.com/nodejs/node/issues/24645


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/24748
Refs: https://github.com/nodejs/node/pull/24486


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

While running the test suite the progress bar shows former line
endings if the new line is shorter than the former line. The length
was calculated without the line ending. It is now an empty string
to prevent the off by one error instead of using extra whitespace.

PR-URL: https://github.com/nodejs/node/pull/24748
Refs: https://github.com/nodejs/node/pull/24486


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

Fixes: https://github.com/nodejs/node/issues/24470

PR-URL: https://github.com/nodejs/node/pull/24723


Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

`http_parser_execute(..., nullptr, 0)` returns either `0` or `1`. The
expectation is that no error must be returned if it is `0`, and if
it is `1` - a `Error` object must be returned back to user.

The introduction of `llhttp` and the refactor that happened during it
accidentally removed the error-returning code. This commit reverts it
back to its original state.

Fix: #24585
PR-URL: https://github.com/nodejs/node/pull/24738


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

To help debugging the flake with the log from the CI.

PR-URL: https://github.com/nodejs/node/pull/24720
Refs: https://github.com/nodejs/node/issues/24403


Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

document that incoming cookie headers are joined using '; '.

PR-URL: https://github.com/nodejs/node/pull/24740


Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

Move the wrapping of the inspector console in a separate file
for clarity. In addition, save the original console from the
VM explicitly via an exported property
`require('internal/console/inspector').consoleFromVM`
that `require('inspector').console` can alias to it later,
instead of hanging the original console onto `per_thread.js`
during bootstrap and counting on that `per_thread.js`
only gets evaluated once and gets cached.

PR-URL: https://github.com/nodejs/node/pull/24709


Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Since we do not actually use the Console constructor to
instantiate the global console, move the two piece of
code into two different JS files for clarity, and make
console.js a mere re-export of the global console.
The hope is to make the global console, a namespace, more
web-compatible while keeping the Console constructor
available for backwards compatibility.

PR-URL: https://github.com/nodejs/node/pull/24709


Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

This is a broken commit: it's here so that git interpret this
as a file move and preserve most of the history of the Console
constructor.

PR-URL: https://github.com/nodejs/node/pull/24709


Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/24473


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>

The motivation for this commit is to make the the onticketkeycallback
function more readable.

PR-URL: https://github.com/nodejs/node/pull/24718


Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/24495


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Correctness-wise, this removes side effects in the href setter if
parsing fails. Style-wise, this allows removing the parse() wrapper
function around C++ _parse().

Also fix an existing bug with whitespace trimming in C++ that was
previously circumvented by additionally trimming the input in
JavaScript.

Fixes: https://github.com/nodejs/node/issues/24345
Refs: https://github.com/nodejs/node/pull/24218#issuecomment-438476591

PR-URL: https://github.com/nodejs/node/pull/24495


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Fixes: https://github.com/nodejs/node/issues/24211

PR-URL: https://github.com/nodejs/node/pull/24495


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

This reverts commit 5e1bf058, as it
causes major performance regressions during object construction.

Refs: https://github.com/nodejs/node/pull/24218

PR-URL: https://github.com/nodejs/node/pull/24495


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Fix the timer logic to be the same as v10.30.0.

Fixes: https://github.com/nodejs/node/issues/24203

PR-URL: https://github.com/nodejs/node/pull/24214


Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

PR-URL: https://github.com/nodejs/node/pull/24520


Reviewed-By: Anna Henningsen <anna@addaleax.net>

Avoids a maintenance hazard when reviewers assume that
`hostlessProtocol` and `slashedProtocol` are disjoint.

The following may be counter-intuitive:

```js
// These objects seem to have no keys in common
const hostlessProtocol = { 'javascript': true };
const slashedProtocol = { 'http': true };
// A reasonable reviewer may assumes bothTrue is never truthy
function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}
// But
console.log(Boolean(bothTrue('constructor')));  // true
```

This change uses SafeSet instead of plain-old objects.

----

Rejected alternative:

We could have used object with a `null` prototype as lookup tables
so that `lowerProto` is never treated as a key into `Object.prototype`.

```js
const hostlessProtocol = { __proto__: null, 'javascript': true };
const slashedProtocol = { __proto__: null, 'http': true };

function bothTrue(lowerProto) {
  return hostlessProtocol[lowerProto] && slashedProtocol[lowerProto];
}

console.log(Boolean(bothTrue('constructor')));  // false
```

PR-URL: https://github.com/nodejs/node/pull/24703


Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Fixes: https://github.com/nodejs/node/issues/24567

PR-URL: https://github.com/nodejs/node/pull/24574


Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

Ref: https://github.com/nodejs/build/pull/1596

PR-URL: https://github.com/nodejs/node/pull/24669


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/24670


Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Elevate namespace for repeated entities.
Resolved conflicts.

PR-URL: https://github.com/nodejs/node/pull/24578


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Currently the following compiler warnings is generated:
../src/node_serdes.cc:400:18:
warning: unused variable 'context' [-Wunused-variable]
  Local<Context> context = ctx->env()->context();
                 ^
1 warning generated.

This commit removes the unused variable.

PR-URL: https://github.com/nodejs/node/pull/24713


Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

The test fails consistently on windows-fanned with vs2017.
mark it as flaky while the issue is being progressed, and
to keep CI green / amber.

Ref: https://github.com/nodejs/node/issues/23621
PR-URL: https://github.com/nodejs/node/pull/24714


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/24657


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>

This patch:

- Moves the C++ binding/addon related code out of
  node_internals.h/node.cc and into dedicated files
  node_binding.h/node_binding.cc, and only puts the code resued
  by other files into the header.
- Introduce a node::binding namespace so that code exposed to
  other files can be easily recognized.

PR-URL: https://github.com/nodejs/node/pull/24701


Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>

Instead of registering it in a global scope of a native module
and expecting that it only gets evaluated when the module is actually
compiled and run and will not be evaluated because the module can
be cached, directly register the DOMException constructor
onto Environment during bootstrap for clarity, since this is
a side effect that has to happen during bootstrap.

PR-URL: https://github.com/nodejs/node/pull/24708


Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>