Unsanitized paths containing line feed characters can be used for
header injection and request splitting so reject them with an exception.
There seems to be no reasonable use case for allowing control characters
(characters <= 31) while there are several scenarios where they can be
used to exploit software bugs so reject control characters altogether.
PR-URL: https://github.com/nodejs/node/pull/8923
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: not-an-aardvark <not-an-aardvark@users.noreply.github.com>
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>
In closed environments, self-signed or privately signed certificates are
commonly used, and rejected by Node.js since their root CAs are not
well-known. Allow extending the set of well-known compiled-in CAs via
environment, so they can be set as a matter of policy.
PR-URL: https://github.com/nodejs/node/pull/9139
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
const and let instead var
assert.strictEqual instead assert.equal
PR-URL: https://github.com/nodejs/node/pull/8668
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
We have a tacit rule that for multiline statements, the operator should
be placed before the linebreak. This commit commit fixes the few
violations of this rule in the code base.
This allows us to enable the corresponding ESLint rule.
PR-URL: https://github.com/nodejs/node/pull/10178
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
* assert.equal() -> assert.strictEqual()
* replace template string with a string; no variable substitution or
concatenation or anything like that
PR-URL: https://github.com/nodejs/node/pull/9803
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
- Updated assert.equal to assert.strictEqual
- Updated 'var' to 'const'
- Using template literals
PR-URL: https://github.com/nodejs/node/pull/10036
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
The assert.throws() calls in test-event-emitter-max-listeners.js
should include a constructor or RegExp as a second argument.
PR-URL: https://github.com/nodejs/node/pull/9987
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* var -> const, let
* assert.equal() -> assert.strictEqual()
PR-URL: https://github.com/nodejs/node/pull/9948
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
change equal to strictEqual and var to const
PR-URL: https://github.com/nodejs/node/pull/9941
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Updating tests to use `common.fixturesDir` whenever possible/reasonable.
Left out things like tests for `path` and `require.resolve`.
PR-URL: https://github.com/nodejs/node/pull/6997
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Use asssert.strictEqual to disallow coersion.
PR-URL: https://github.com/nodejs/node/pull/10071
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Refactored `test-tls-server-verify.js` to replace uses of `var` with
`const` and `let`. Also replaced uses of `assert.equal` with
`assert.strictEqual`.
PR-URL: https://github.com/nodejs/node/pull/10076
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Many of the tests use variables to track when callback functions
are invoked or events are emitted. These variables are then
asserted on process exit. This commit replaces this pattern in
straightforward cases with common.mustCall(). This makes the
tests easier to reason about, leads to a net reduction in lines
of code, and uncovered a few bugs in tests. This commit also
replaces some callbacks that should never be called with
common.fail().
PR-URL: https://github.com/nodejs/node/pull/7753
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This checks to see that clearBuffer appropriately decrements the
correct values in _writableState when clearBuffer is invoked in
end.
Fixes: https://github.com/nodejs/node/issues/8687
PR-URL: https://github.com/nodejs/node/pull/9922
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
- Remove assignment of this to variable.
- Add common.mustCall() as needed.
- Move from var to const.
PR-URL: https://github.com/nodejs/node/pull/10547
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This commit removes assignments of this to a variable in the
tests.
PR-URL: https://github.com/nodejs/node/pull/10548
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* use const instead of var
* use common.mustCall to control functions execution
* use assert.strictEqual instead of assert.equal
* use arrow functions
PR-URL: https://github.com/nodejs/node/pull/10503
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: James M Snell <jasnell@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* use const instead of var
* use common.mustCall to control functions execution
* use assert.strictEqual instead of assert.equal
* use arrow functions
PR-URL: https://github.com/nodejs/node/pull/10479
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
PR-URL: https://github.com/nodejs/node/pull/10478
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
- Change var to const
- Remove dependency crypto
PR-URL: https://github.com/nodejs/node/pull/10033
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/10245
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* use const instead of var
* use assert.strictEqual instead assert.equal
PR-URL https://github.com/nodejs/node/pull/10428
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Remove `setTimeout()` in test and instead rely on `common.mustCall()` on
a `timeout` event handler.
The test was flaky on CI. The flakiness was replicable by running the
test under load. This version, in contrast, is robust under load.
Took the opportunity to do some `var` -> `const` while refactoring.
PR-URL: https://github.com/nodejs/node/pull/10404
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* use let and const instead of var
* use assert.strictEqual instead assert.equal
PR-URL: https://github.com/nodejs/node/pull/10396
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Added duration to setTimeout and removed extraneous callback args,
as per Rich Trott's instructions
PR-URL: https://github.com/nodejs/node/pull/10291
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
file: test/parallel/test-domain-uncaught-exception.js
1. There are three setTimeout() in the file and they do not specify a
duration (the second argument), so I change them to setImmediate()
instead.
2. There are four callbacks that take an argument called `err` but that
argument is never used, so I removed them.
PR-URL: https://github.com/nodejs/node/pull/10193
Reviewed-By: Sam Roberts <sam@strongloop.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
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>
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>
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>
Change var to const or let.
Change assert.equal() to assert.strictEqual().
PR-URL: https://github.com/nodejs/node/pull/9990
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Changing var defs to const/let, changing assert.equal to
assert.strictEqual. Wrapping functions called once with
common.mustCall
PR-URL: https://github.com/nodejs/node/pull/10054
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
var -> let / const
added common.mustCall() to callback
assert.equal() -> assert.strictEqual()
PR-URL: https://github.com/nodejs/node/pull/10017
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
- `var` --> `const` as applicable
- `assert.equal` --> `assert.strictEqual`
- `assert(false, ..)` --> `common.fail()`
- `common.mustCall` for functions that need to be called exactly once
- modified an `assert(!signal, 'Worker exited by a signal');` call to
`assert.strictEqual(signal, null);` call as that made more sense
PR-URL: https://github.com/nodejs/node/pull/10049
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Changed var to const, assert.equal to assert.strictEqual, and
used a template string for error output.
PR-URL: https://github.com/nodejs/node/pull/9986
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
convert var to const
PR-URL: https://github.com/nodejs/node/pull/9953
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Use assert.strictEqual instead of assert.equal and assert.ok
PR-URL: https://github.com/nodejs/node/pull/10420
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@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>
Rod Vagg
Ben Noordhuis
Adam Langley
Sam Roberts
Italo A. Casas
Michaël Zasso
Rich Trott
Travis Bretton
Ken Russo
Richard Karmazin
Jason Wohlgemuth
Bryan English
Cesar Hernandez
Kent.Fan
Hutson Betts
cjihrig
Safia Abdalla
Adrian Estrada
Fabrice Tatieze
Chris Story
jBarz
Joyee Cheung
weyj4
Andy Chen
Siddhartha Sahai
Gregory
Lance Ball
Rob Adelmann
malen
Josh Mays
Brian Chirgwin
Amar Zavery
Kevin Cox
Rico Cai
Segu Riluvan
Daniel Sims
Christopher Rokita