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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

also added regression tests

PR-URL: https://github.com/nodejs/node/pull/13111
Fixes: https://github.com/nodejs/node/issues/13108


Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>

This change ensures that the WebSocket server can be stopped
(and restarted if needed) buy calling process._debugEnd.

PR-URL: https://github.com/nodejs/node/pull/12777
Fixes: https://github.com/nodejs/node/issues/12559


Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>

Fixed sequential test-net-connect-local-error by swapping port
and localPort in net.connect options.

PR-URL: https://github.com/nodejs/node/pull/13064
Fixes: https://github.com/nodejs/node/issues/13055


Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>

This commit fixes two coverity warnings for unchecked
return values.

Refs: https://github.com/nodejs/node/pull/13050
PR-URL: https://github.com/nodejs/node/pull/13116


Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

When a transform stream's callback is called more than once,
an error is emitted with a somewhat confusing message. This
commit hopes to improve the quality of the error message.

Fixes: https://github.com/nodejs/node/issues/12513
PR-URL: https://github.com/nodejs/node/pull/12520


Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Remove common.PORT from
test-dgram-send-callback-buffer-length-empty-address to eliminate
possibility that a dynamic port used in another test will collide
with common.PORT.

PR-URL: https://github.com/nodejs/node/pull/12944
Refs: https://github.com/nodejs/node/issues/12376


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>

Remove common.PORT from test-dgram-send-callback-buffer-length to
eliminate possibility that a dynamic port used in another test will
collide with common.PORT.

PR-URL: https://github.com/nodejs/node/pull/12943
Refs: https://github.com/nodejs/node/issues/12376


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>

Remove common.PORT from test-dgram-send-address-types to eliminate
possibility that a dynamic port used in another test will collide
with common.PORT.

PR-URL: https://github.com/nodejs/node/pull/13007
Refs: https://github.com/nodejs/node/issues/12376


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Replace common.PORT with available port assigned by the operating
system in test-dgram-send-callback-buffer.

PR-URL: https://github.com/nodejs/node/pull/12942
Refs: https://github.com/nodejs/node/issues/12376


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>

Replace common.PORT with available port assigned by the operating
system in test-dgram-send-callback-buffer-empty-address.

PR-URL: https://github.com/nodejs/node/pull/12929
Refs: https://github.com/nodejs/node/issues/12376


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

Adds destroy() and _destroy() methods to Readable, Writable, Duplex
and Transform. It also standardizes the behavior and the implementation
of destroy(), which has been inconsistent in userland and core.
This PR also updates all the subsystems of core to use the new
destroy().

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


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

I can't see that the forward declaration of class Connection is needed
and wanted to raise this in case it was overlooked after a previous
change.

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Currently it is possible to call TLSWrap() without using new and the
following error message:
FATAL ERROR: v8::Object::SetAlignedPointerInInternalField() Internal
field out of bounds
 1: node::Abort()
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 2: node::OnFatalError(char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 3: v8::Utils::ReportApiFailure(char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 4: v8::Utils::ApiCheck(bool, char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 5: v8::InternalFieldOK(v8::internal::Handle<v8::internal::JSReceiver>,
int, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 6: v8::Object::SetAlignedPointerInInternalField(int, void*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 7: node::TLSWrap::Initialize(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)::$_0::operator()(v8::FunctionCallbackInfo<v8::Value>
const&) const [/Users/danielbevenius/work/nodejs/node/out/Debug/node]

This commit adds a IsConstructCall check which will produce the
following error message:
/Users/danielbevenius/work/nodejs/node/out/Debug/node[2212]:
../src/tls_wrap.cc:936:auto node::TLSWrap::Initialize(Local<v8::Object>,
Local<v8::Value>, Local<v8::Context>)::(anonymous
class)::operator()(const FunctionCallbackInfo<v8::Value> &) const:
Assertion `args.IsConstructCall()' failed.
 1: node::Abort()
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 2: node::Assert(char const* const (*) [4])
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 3: node::TLSWrap::Initialize(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)::$_0::operator()(v8::FunctionCallbackInfo<v8::Value>
const&) const [/Users/danielbevenius/work/nodejs/node/out/Debug/node]

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


Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

I think the code would be a little clearer if the return statement
was removed here since End() does not return any value and neither
does ParseHeader.

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


Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

I can't find any usage of Buffer in this file and think the include
can be removed.

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Some isolated build or test VMs don't have a "nobody" user, causing the
parallel/test-process-setuid-setgid test to fail. Add logic to allow for
that situation.

PR-URL: https://github.com/nodejs/node/pull/13112
Fixes: https://github.com/nodejs/node/issues/13071


Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Imprecision in process.hrtime() in some situations can result in a zero
duration being used as a denominator in benchmark tests. This would
almost certainly never happen in real benchmarks. It is only likely in
very short benchmarks like the type we run in our test suite to just
make sure that the benchmark code is runnable.

So, if the environment variable that we use in tests to indicate "allow
ludicrously short benchmarks" is set, convert a zero duration for
a benchmark to 1 nano-second.

PR-URL: https://github.com/nodejs/node/pull/13110
Fixes: https://github.com/nodejs/node/issues/13102
Fixes: https://github.com/nodejs/node/issues/12433


Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Add `--set key=""` to cut down the time it takes to run
test-benchmark-http by about a third.

Alphabetize options in `--set` list.

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


Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

This commit fixes the Node process crashing when constructors of classes
of the zlib module are given invalid options.

* Throw an Error when the zlib library rejects the value of windowBits,
  instead of crashing with an assertion.

* Treat windowBits and memLevel options consistently with other ones and
  don't crash when non-numeric values are given.

* Fix bugs in the validation logic:
  - Don't conflate 0 and undefined when checking if a field of an
    options object exists.
  - Treat NaN and Infinity values the same way as values of invalid
    types instead of allowing to actually set zlib options to NaN or
    Infinity.

PR-URL: https://github.com/nodejs/node/pull/13098
Fixes: https://github.com/nodejs/node/issues/13082


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

This fixes the async_hooks.AsyncHook constructor such that it throws an
error when provided with falsy values other than undefined.

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

Edit to the comment in the stack trace capture, highlighting the use of
the constructorOpt argument in errors.md

Fixes: https://github.com/nodejs/node/issues/12289
PR-URL: https://github.com/nodejs/node/pull/12962


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben.bridgewater@fintura.de>

Fix the bug that you start process without network at first, but it
connected lately, `dns.resolve` will stay failed with ECONNREFUSED
because c-ares servers fallback to 127.0.0.1 at the very beginning.

If c-ares servers "127.0.0.1" is detected and its not set by user self,
and last query is not OK, recreating `ares_channel` operation will be
triggered to reload servers.

Fixes: https://github.com/nodejs/node/issues/1644
PR-URL: https://github.com/nodejs/node/pull/13076


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Port the disabled readline test to make it runnable in our usual test
suite.

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


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

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


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

This reverts 4cb5f3da

Based on community feedback I think we should consider reverting this
change. We should explore how this could be solved via linting rules.

Refs: https://github.com/nodejs/node/pull/12562
PR-URL: https://github.com/nodejs/node/pull/12976


Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

The usage of common.PORT could cause undesired port collisions when run
in parallel. The following test was moved to sequential.
test-net-reconnect-error.js

PR-URL: https://github.com/nodejs/node/pull/13033
Refs: https://github.com/nodejs/node/issues/12376


Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>

This commit contains three separate changes:

- Always return a string from ToUnicode no matter if an error occurred.
- Disable CheckHyphens boolean flag. This flag will soon be enabled in
  the URL Standard, but is implemented manually by selectively ignoring
  certain errors.
- Enable CheckBidi boolean flag per URL Standard update.

This allows domain names with hyphens at 3 and 4th position, as well as
those with leading and trailing hyphens. They are technically invalid,
but seen in the wild.

Tests are updated and simplified accordingly.

PR-URL: https://github.com/nodejs/node/pull/12966
Fixes: https://github.com/nodejs/node/issues/12965
Refs: https://github.com/whatwg/url/pull/309


Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/12846
Fixes: https://github.com/nodejs/node/issues/12825
Refs: https://github.com/w3c/web-platform-tests/pull/5792


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

* Improve comments describing test.
* Remove unnecessary `common.noop`
* Remove confusing scoping of `c` identifier

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


Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

* Add common.mustCall() for error handlers.
* Alphabetize modules

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


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>

When using an Agent for HTTPS, `TLSSocket`s are reused and need to
have the ability to `asyncReset` from JS.

PR-URL: https://github.com/nodejs/node/pull/13092
Fixes: https://github.com/nodejs/node/issues/13045


Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Realized that we don't actually point people to the file to
include in order to access N-API functions.  Add that.

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


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

Unlike most N-API functions, `napi_get_last_error_info()` should not
clear the last error code when successful, because a pointer to (not
a copy of) the error info structure is returned via an out parameter.

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

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


Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Call uc_init() after u_setDataDirectory() to find out if the data
directory is actually valid.

This commit removes parallel/test-intl-no-icu-data, added in commit
46345b93 ("src: make --icu-data-dir= switch testable").  It no longer
works now that an invalid --icu-data-dir= argument is rejected.
Coverage is now provided by parallel/test-icu-data-dir.

Fixes: https://github.com/nodejs/node/issues/13043
Refs: https://github.com/nodejs/node-gyp/issues/1199


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>

According to the explanation in #3635#issuecomment-157714683
And as a continuation to #5422 we also ignore EMFILE
"No more file descriptors are available,so no more files can be opened"

PR-URL: https://github.com/nodejs/node/pull/12698
Fixes: https://github.com/nodejs/node/issues/10286
Refs: https://github.com/nodejs/node/issues/3635#issuecomment-157714683
Refs: https://github.com/nodejs/node/pull/5178
Refs: https://github.com/nodejs/node/pull/5179
Refs: https://github.com/nodejs/node/pull/4005
Refs: https://github.com/nodejs/node/pull/5121
Refs: https://github.com/nodejs/node/pull/5422
Refs: https://github.com/nodejs/node/pull/12621#issuecomment-297701459


Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

This commit attaches a Symbol to the result of
net._normalizeArgs(). This prevents normal arrays from being
passed to the internal Socket.prototype.connect() bypass logic.

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


Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>