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>

This is a change from the ecmascript-modules fork.
There is no change to behavior and we would like to
upstream to reduce the delta between our repos.

Refs: https://github.com/nodejs/ecmascript-modules#9

PR-URL: https://github.com/nodejs/node/pull/24560
Refs: https://github.com/nodejs/ecmascript-modules/pull/9


Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>

Currently the order of error / closing of an h2 stream is consistent
in 10.x, 11.x, and master. There appears to be an unexpected behavior
difference in 8.x. This test will be used to bisect the commit that will
fix this behavior change and ensure there are no future regressions.

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


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

In path.toNamespacePath was a case when the path
was empty string and it wasn't covered in the tests.

I covered this case both in Windows and Unix environments.

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


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

Because most of its code (the getter) has nothing to do with
the actual bootstrapping and it is run per-thread.

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Christopher Hiller <boneskull@boneskull.com>

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


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

Currently two warnings are generated regarding unused variables.
This commit removes the unused variables.

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


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: Luigi Pinca <luigipinca@gmail.com>

This patch refactors the branches for choosing the mode to run
Node.js in `internal/bootstrap/node.js`. Instead of inlining the
decision making all in `startup`, we create a `startExecution()`
function which either detects and start the non-user-code mode,
or prepares for user code execution (worker setup, preloading modules)
and starts user code execution.
We use early returns when we decide the mode to run Node.js in for fewer
indentations and better readability.

This patch also adds a few comments about the command-line switches
and a few TODOs to remove underscore properties on `process` that
are mainly used for bootstrap mode branching. It also includes
a few other refactoring such as inlining functions/variables
that are not reused and removing the default argument of
`evalScript` for better clarity.

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


Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

writeHead supports an array of arrays containing header name and values.
Compatibility between http2 & http1 even though this is not documented.

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

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


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>

On Windows creating a symlink to a directory will not work unless extra
'dir' parameter is passed. This adds a check if link target is a
directory, and if so automatically use 'dir' when creating symlink.

PR-URL: https://github.com/nodejs/node/pull/23724
Refs: https://github.com/nodejs/node/pull/23691


Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

This was essentially a typo that went unnoticed because we
didn’t have tests for this particular situation.

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

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


Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>

Notable Changes:

* **win/msi**: Revert changes to installer causing issues on Windows systems.

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

See: https://github.com/nodejs/readable-stream/issues/387

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


Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

This is a known failure so mark it such, so that
CI is green / amber while the issue is being progressed.

PR-URL: https://github.com/nodejs/node/pull/24712
Refs: https://github.com/nodejs/node/pull/24620
Refs: https://github.com/nodejs/node/issues/24120


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

Makes GYP properly handle .S files.

Fixes: https://github.com/nodejs/node-v8/issues/89

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


Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>