Document and test the existing support for generic Duplex streams
in the TLS module.

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


Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/17610
Refs: https://github.com/nodejs/node/pull/17054#discussion_r155406755


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

The mustCall is actually only necessary in case it is used as
callback. Otherwise it works as a must call on its own.

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


Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>

The function was never documented and now throws a TypeError if used.

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


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

This commit changes node's handling of two-byte characters in
the path component of an http URL. Previously, node would just
strip the higher byte when generating the request. So this code:

```
http.request({host: "example.com", port: "80", "/Ｎ"})
```

would request `http://example.com/.`
(`.` is the character for the byte `0x2e`).

This is not useful and can in some cases lead to filter evasion.
With this change, the code generates `ERR_UNESCAPED_CHARACTERS`,
just like space and control characters already did.

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


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

Add a comment explaining the test (especailly why it forks 80 processes.
Use destructuring and an arrow function callback.

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


Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>

Using port 0 to request an open port from the operating system is
sufficient in openssl-client-cert-engine/test.js.

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


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

common.PORT should not be used in parallelized tests. (There can be a
port collision if another tests requests an arbitrary open port from the
operating system and ends up getting common.PORT before a test that uses
common.PORT uses the port.) In such a situation, throw an error.

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


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

common.PORT is no longer used in parallelized tests and should not be.
Remove code that accommodates parallelized tests.

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


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

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


Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

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


Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

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


Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

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


Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

- Remove redundant `hasCrypto` checks
- Use `common.mustCall()`
- Use arrow functions
- Deduplicate HTTP & HTTPS code

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


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

Arrow functions cannot be called with the new keyword,
convert to ES6 classes instead.

PR-URL: https://github.com/nodejs/node/pull/17579
Refs: https://github.com/nodejs/node/pull/17364


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

Attaching WS session will now include a roundtrip onto the main thread
to make sure there is no other session (e.g. JS bindings)

This change also required refactoring WS socket implementation to better
support scenarios like this.

Fixes: https://github.com/nodejs/node/issues/16852
PR-URL: https://github.com/nodejs/node/pull/17085


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

Add lint rule to validate that common.expectsError(fn, err) is being
used instead of assert.throws(fn, common.expectsError(err));

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


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

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


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

autocannon appears to have trouble recognizing URLs that contain true
or false within them. Use 0 or 1 instead to represent the same.

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


Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

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


Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

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


Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>

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


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

This commit removes the include of node.h which does not seem to be
used.

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


Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

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


Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/17538
Fixes: https://github.com/nodejs/node/issues/17536


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>

Replace code optimized for older versions of V8 with more
straightforward code in checkIsHttpToken().

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


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

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


Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

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


Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

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


Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

cctest build target wasn't defining the HAVE_OPENSSL macro when
node_use_openssl was true, causing inconsistencies on most
`node::Environment` member's addresses. For example, if someone
wanted to access the context of an environment by using
`node::Environment::context()`, the object returned by the
function was pointing to an invalid address.

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


Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

This commit removes the explicit namespace qualifiers in
Connection::Initialize, and SecureContext:Initialize which should not
necessary in a member function (using an unqualified name should resolve
by searching the class of the member function).

This is consistent with the other intialize functions in node_crypto
like Hmac::Initialize, Hash::Initialize, Sign::Initialize,
Verify::Initialize, DiffieHellman::Initialize, ECDH::Initialize,
and InitCrypto.

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


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

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


Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

There is a perfectly serviceable ERR_get_error function which avoids
having to sniff through the OpenSSL ring buffer like that. It does
return the errors in the opposite order, but that's easily fixed with
std::reverse.

Note this behavior is slightly different in that an ERR_get_error loop
will ultimately clear the error queue, but this is desirable. Leaving
the error queue uncleared means errors in subsequent operations may get
mixed up and cause issues.

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


Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

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


Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

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


Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

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


Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>

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


Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

The square module is described as exporting a constructor,
which would mean it would need to be invoked with the
new keyword in bar.js after requiring it. Otherwise it's
technically a factory function, not a constructor.

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


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

Fix a regression that was introduced in commit 5886e204 ("inspector:
track async stacks when necessary") and that I overlooked during review:
the persistent handle with the callback must be rematerialized *after*
the `v8::HandleScope` is created, not before.

Apparently `test/sequential/test-inspector-async-call-stack.js` has
no test coverage for this scenario and I'm out of good ideas on how
to create a concise and reliable test case.

Fixes: https://github.com/nodejs/node/issues/17496
PR-URL: https://github.com/nodejs/node/pull/17539


Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

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


Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>