After processing all the callbacks in the destroy_ids vector make sure
to clear() it otherwise the DestroyIdsCb() won't run again.
PR-URL: https://github.com/nodejs/node/pull/10400
Fixes: b49b496 "async_wrap: call destroy() callback in uv_idle_t"
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
SecureContext::AddRootCerts only parses the root certificates once and
keeps the result in root_cert_store, a global X509_STORE. This change
addresses the following issues:
1. SecureContext::AddCACert would add certificates to whatever
X509_STORE was being used, even if that happened to be root_cert_store.
Thus adding a CA certificate to a SecureContext would also cause it to
be included in unrelated SecureContexts.
2. AddCRL would crash if neither AddRootCerts nor AddCACert had been
called first.
3. Calling AddCACert without calling AddRootCerts first, and with an
input that didn't contain any certificates, would leak an X509_STORE.
4. AddCRL would add the CRL to whatever X509_STORE was being used. Thus,
like AddCACert, unrelated SecureContext objects could be affected.
The following, non-obvious behaviour remains: calling AddRootCerts
doesn't /add/ them, rather it sets the CA certs to be the root set and
overrides any previous CA certificates.
Points 1–3 are probably unimportant because the SecureContext is
typically configured by `createSecureContext` in `lib/_tls_common.js`.
This function either calls AddCACert or AddRootCerts and only calls
AddCRL after setting up CA certificates. Point four could still apply in
the unlikely case that someone configures a CRL without explicitly
configuring the CAs.
PR-URL: https://github.com/nodejs/node/pull/9409
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
This commit addresses an inconsistency with eight tests. These
tests use the assert module, but named the variable ASSERT. This
goes against the project's typical coding style, and negatively
impacts global find and replace updates.
PR-URL: https://github.com/nodejs/node/pull/10544
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* add RegExp as second argument to assert.throws()
* replace process.on('exit', ...) boolean checks with common.mustCall()
* assert.equal() -> assert.strictEqual()
* add 1 ms duration as second argument to setTimeout()
* var -> const
PR-URL: https://github.com/nodejs/node/pull/10532
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* provide a RegExp for second argument to `assert.throws()`
* remove unused function arguments
* provide duration of 1 ms for `setTimeout()` calls that do not have a
duration
PR-URL: https://github.com/nodejs/node/pull/10530
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
test-http-client-timeout-with-data has failed here and there in CI on
FreeBSD and OS X. The test has a socket timeout set to 50ms and a timer
set for 100ms. However, they are not necessarily set in the same tick of
the event loop and their ordering is therefore not guaranteed.
Instead of using a timer, this change listens for an event on the
listener to know when the socket timeout has occurred and then runs the
code originally in the timer.
Additional refactoring: Replaced `process.on('exit', ...)` checks with
`common.mustCall()` and replaced usage of `assert.equal()` with
`assert.strictEqual()`.
PR-URL: https://github.com/nodejs/node/pull/10431
Reviewed-By: James M Snell <jasnell@gmail.com>
* use common.mustCall to control the functions execution automatically
* use let and const instead of var
* use assert.strictEqual instead of assert.equal
* use arrow functions
* remove console.error and unnecessary variables
PR-URL: https://github.com/nodejs/node/pull/10397
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Remove console.log statement. Replace error check with
assert.ifError().
PR-URL: https://github.com/nodejs/node/pull/10331
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* use const and let instead of var
* use common.mustCall to control functions executions
* use assert.strictEqual instead of assert.equal
* use assert.ifError to handle errors
* use arrow functions
* remove unnecessary variables
PR-URL: https://github.com/nodejs/node/pull/10440
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* use const instead of var
* use common.mustCall to control the functions execution automatically
* use assert.strictEqual instead of assert.equal
* use assert.notStrictEqual instead of assert.notEqual
* use arrow functions
PR-URL: https://github.com/nodejs/node/pull/10367
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* use const instead of var
* use assert.strictEqual instead of assert.equal
PR-URL: https://github.com/nodejs/node/pull/10429
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* use common.mustCall to control the functions execution automatically
* use const instead of var
* use assert.strictEqual instead assert.equal
* use assert.ifError instead of throw error
PR-URL: https://github.com/nodejs/node/pull/10297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* use const instead of var for required modules
* use assert.strictEqual instead of assert.equal
PR-URL: https://github.com/nodejs/node/pull/10318
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
In this test, I changed the var declarations to be either a let or a
const. For some of the callbacks, I added a mustCall check to ensure
that the functions have run. I also changed assert.equal() to
assert.strictEqual().
PR-URL: https://github.com/nodejs/node/pull/9962
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
- var -> const where possible
- assert.equal -> assert.strictEqual
- passed the setTimeout function a second parameter for readability
- used assert.strictEqual for assert(!c) as it is expected to be 0 and
not some other value
PR-URL: https://github.com/nodejs/node/pull/10321
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
test-timers-same-timeout-wrong-list-deleted was flaky under load because
there is no guarantee that a timer will fire within a given period of
time. It had an exit handler that checked that the process was finishing
in less than twice as much as a timer was set for. Under load, the
timer could take over 200ms to fire even if it was set for 100ms, so
this was causing the test to be flaky on CI from time to time.
However, that timing check is unnecessary to identify the regression
that the test was written for. When run with a version of Node.js that
does not contain the fix that accompanied the test in its initial
commit, an assertion indicating that there were still timers in the
active timer list fired. So, this commit removes the exit handler timing
check and relies on the existing robust active timers list length check.
This allows us to move the test back to parallel because it does not
seem to fail under load anymore.
The test was refactored slightly, removing duplicated code to a
function, using `assert.strictEqual()` instead of `assert.equal()`,
changing a 10ms timer to 1ms, and improving the messages provided by
assertions.
Fixes: https://github.com/nodejs/node/issues/8459
PR-URL: https://github.com/nodejs/node/pull/10362
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
10.5 error Please use assert.strictEqual() instead of assert.equal()
PR-URL: https://github.com/nodejs/node/pull/10047
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/9988
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* use common.mustCall to control the functions execution automatically
* use let and const instead of var
* use assert.strictEqual instead assert.equal
PR-URL: https://github.com/nodejs/node/pull/10350
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
In file /test/parallel/test-cwd-enoent-repl.js at
line: 26:3 and 27:3 assert.equal was used.
This commit changes use of assert.equal to assert.strictEqual.
PR-URL: https://github.com/nodejs/node/pull/9952
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add test for requiriing an invalid package path.
PR-URL: https://github.com/nodejs/node/pull/9903
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
In preparation for enabling an ESLint rule, use consistent block
spacing. This changes only six files in the code base as block spacing
is consistent throughout the rest of the code base.
Before: function(c) {data += c;}
After: function(c) { data += c; }
PR-URL: https://github.com/nodejs/node/pull/10377
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Julian Duque <julianduquej@gmail.com>
* use common.mustCall() and eliminate exit handler
* provide timer durtion of 1ms where previously omitted
* var -> const
PR-URL: https://github.com/nodejs/node/pull/10315
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
* use const and let instead of var
* use assert.strictEqual instead of assert.equal
* use assert.strictEqual instead of assert.ok
* use assert.ifError
PR-URL: https://github.com/nodejs/node/pull/10312
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
* use const instead of var for required modules
* use assert.strictEqual instead of assert.equal
* remove unnecessary process.nextTick
PR-URL: https://github.com/nodejs/node/pull/10273
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Race condition caused occasional failure on CI. Chained callbacks used
to remove race condition.
PR-URL: https://github.com/nodejs/node/pull/10293
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* use const instead of var for required modules
* use assert.strictEqual instead of assert.equal
* use assert.strictEqual instead of assert.ok
PR-URL: https://github.com/nodejs/node/pull/10275
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Refactor to take advantage of block scoping to isolate tests. Checks in
exit handlers now reside with the relevant test block. Where test cases
start and end is more clear.
Also: Some use of `common.mustCall()` and improved wrapping/indentation.
PR-URL: https://github.com/nodejs/node/pull/10246
Reviewed-By: Michaël Zasso <targos@protonmail.com>
* use assert.strictEqual() instead of assert.equal()
PR-URL: https://github.com/nodejs/node/pull/9999
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* use common.mustCall()
* use String.prototype.includes() instead of String.prototype.indexOf()
PR-URL: https://github.com/nodejs/node/pull/9989
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
test-http-client-timeout-option-listeners is flaky due to depending on
completing operations before a 100ms socket timeout. The socket timeout
is an integral part of the test but can be very large. Set to the
maximum allowable value.
PR-URL: https://github.com/nodejs/node/pull/10224
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
* specify constructor for assert.throws()
* load additional modules only if crypto check passes
* normalize some potentially confusing indentation
* provided actual first and expected second in assertions
PR-URL: https://github.com/nodejs/node/pull/10232
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
test-dgram-exclusive-implicit-bind is written assuming that dgram
messages are received with 100% reliability. While missing a dgram
message sent to localhost is rare, we do see it as evidenced by CI
failures from time to time.
The test has been rewritten to send dgram messages over and over until
the test requirements have been met.
Additional incidental refactoring includes:
* var -> const
* use of common.mustCall() instead of exit listener + boolean
PR-URL: https://github.com/nodejs/node/pull/10212
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
- replace var with const.
- remove successes var.
- use assert.ifError() for handling all errors.
- wrap all callbacks with common.mustCall().
PR-URL: https://github.com/nodejs/node/pull/10176
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
- Replace assert.equal() to assert.strictEqual()
- Replace var with const where applicable
- Removed firstBodyChunk which is never used
- Remove the process.on('exit', ...) and replace its functionality by
- Using common.mustCall() where applicable
PR-URL: https://github.com/nodejs/node/pull/10229
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
In test-http-incoming-pipelined-socket-destory:
* setTimeout() with no duration -> setImmediate()
* eliminate unneeded exit listener
* use common.mustCall()
* var -> const/let
PR-URL: https://github.com/nodejs/node/pull/10189
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Document all TLSSocket options:
- All the secure context options are valid options
to a secureContext
- isServer modifies the default value of requestCert
Describe all tls.connect() variants:
- tls.connect(path) was undocumented
- tls.connect(port) was underdocumented, and its relationship to
tls.connect(options) was obscure
Socket passed to tls.connect is user managed:
- Replace https://github.com/nodejs/node/pull/8996
Add documentation to:
- describe and add tests for the pfx and key variants, and describe how
and when passphrase is used.
- describe tls cert and ca options
- describe buffer forms of tls crl option
- describe tls cipher option and defaults
- fix link to Crypto Constants
- describe that honorCipherOrder sets SSL_OP_CIPHER_SERVER_PREFERENCE.
- describe tls ecdhCurve/dhparam options
- describe tls secureProtocol option
- describe tls secureOptions
- describe tls sessionIdContext
De-deduplicate secure context docs:
The secure context options were documented 4 times, making it difficult
to understand where the options come from, where they are supported,
and under what conditions they are used.
The multiple copies were inconsistent and contradictory in their
descriptions of the options, and also inconsistent in whether the
options would be documented at all.
Cut through this gordian knot by linking all APIs that use the
secureContext options to the single source of truth about the options.
PR-URL: https://github.com/nodejs/node/pull/9800
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Either the options or the listener argument to tls.createServer() was
optional, but not both. This makes no sense, so align the argument
checking and documentation with net.createServer(), which accepts the
same option sequence, and which tls.createServer() is modelled on.
PR-URL: https://github.com/nodejs/node/pull/9800
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Gregory
Lance Ball
Trevor Norris
Adam Langley
cjihrig
Rich Trott
Adrian Estrada
Rob Adelmann
Daniel Sims
Emanuel Buholzer
Segu Riluvan
Ashita Nagesh
Neeraj Sharma
Duy Le
mfrance
Aaron Williams
hveldstra
larissayvette
Prieto, Marcos
Sam Roberts