Use common.mustCall() where appropriate, var to const/let,
assert.equal() -> assert.strictEqual(), explicit time provided to
setTimeout()

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


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

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


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

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


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.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>

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>

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


Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

1. The test doesn't attach an event listener for `exit` events and
   removes them before killing. The intention is to fail the tests if
   the processes exit normally. This patch attaches the `exit` event
   handlers.

2. Replace `var`s with `let`s and `const`s.

3. Replace `==` based assertion with `strictEqual` assertion.

4. Use `common.PORT` instead of `5959`.

5. The test used to expect only one string "connecting to
   localhost:5959 ... ok", but the debugger actually emits another
   string, "break in test/fixtures/empty.js:2". This patch asserts if
   both of them are received in the same order.

Refer: https://github.com/nodejs/node/issues/10361
PR-URL: https://github.com/nodejs/node/pull/10455



Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.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>

If searchParams becomes empty, query must be set to null.
Add missing update of context flags.

Fixes: https://github.com/nodejs/node/issues/10480
PR-URL: https://github.com/nodejs/node/pull/10486


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

This patch contains the following changes:

url: make IPv4 parser more spec compliant

* Return int64_t from ParseNumber to prevent overflow for valid big numbers
* Don't throw when there are more than 4 parts (it cannot be an IP
address)
* Correctly interpret the address and don't always throw when there are
numbers > 255

Ref: https://url.spec.whatwg.org/#concept-ipv4-parser
Fixes: https://github.com/nodejs/node/issues/10306

url: percent encode fragment to follow spec change

Ref: https://github.com/whatwg/url/issues/150
Ref: https://github.com/whatwg/url/commit/373dbedbbf0596f723ce8a195923da98b698aeb0

url: fix URL#search setter

The check for empty string must be done before removing the leading '?'.

Ref: https://url.spec.whatwg.org/#dom-url-search

url: set port to null if an empty string is given

This is to follow a spec change.

Ref: https://github.com/whatwg/url/pull/113

url: fix parsing of paths with Windows drive letter

test: update WHATWG URL test fixtures

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


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

On Windows, creating a symlink requires admin privileges.
There were two tests which created symlinks which were failing when run
as non-admin.

test-fs-symlink.js already had a check for privileges on Windows
but it had a couple issues:
1. It assumed that whoami was the one that came with windows.
   However, whoami also ships with Win32 Unix utility ports
   like the distribution with git, which can cause this to get check
   tripped up.
2. On failure, the check would just return from the callback instead of
   exiting
3. whoami was executed asynchronously so the test would run regardless
   of privilege state.

test-fs-options-immutable had no check.

As part of this change, I refactored the privilege checking to
a function in common, and changed both above tests to use the
refactored function.

Also documented this function in test\README.md

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


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

* process.on('exit',...) checks -> common.mustCall()
* remove unused function parameters
* var -> const/let

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


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>

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>

Move non-standard methods to `url` module instead of exposing as
static methods on the `URL` object.

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


Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

Yields about a 25% average performance improvement

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


Reviewed-By: Michaël Zasso <targos@protonmail.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>

child_process.exec*() and child_process.spawn*() (if options.shell is
true) allow trivial arbitrary command execution if code passes
unsanitised user input to it. Add warnings in the docs to make that
clear.

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


Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
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 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>

Collaborators have elevated privileges. The CTC now requires
Collaborator accounts to have two-factor authentication. This changes
wording in the onboarding documentation to make it clear that two-factor
authentication is required and not merely recommended.

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


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@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>

* replace `process.on('exit', ...)` checks with `common.mustCall()`
* assert.equal() -> assert.strictEqual()
* provide duration of 1ms to timer without a duration
* remove unused function argument

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


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>

* Set exposed attributes of the interface enumerable
  and configurable, as required by the spec.
  See: https://heycam.github.io/webidl/#es-attributes
* Make sure `URL#searchParams` returns `[[SameObject]]`
* Add the missing `URL#href` setter
* Reorder the properties to match
  https://url.spec.whatwg.org/#api
* Add tests for the ECMAScript property attributes

PR-URL: https://github.com/nodejs/node/pull/10408
Fixes: https://github.com/nodejs/node/issues/10376


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

PR-URL: https://github.com/nodejs/node/pull/10408
Fixes: https://github.com/nodejs/node/issues/10376


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

Bump configure download path for ICU from 58.1 to 58.2

* This commit just changes the download URL.

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


Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Bump ICU from 58.1 to 58.2

No feature changes. Bug fixes.
* This commit contains the actual ICU source code and data.
* CLDR 30.0.3 ( was 30.0.2)
* Timezone 2016j ( was 2016g )

ICU Details:
http://site.icu-project.org/download/58#TOC-Updates-in-ICU-58.2

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


Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Use of abort() was added in 34febfbf, and changed to ABORT()
in 21826ef2, but conditional+ABORT() is better expressesed
using a CHECK_xxx() macro.

See: https://github.com/nodejs/node/pull/9409#discussion_r93575328

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


Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

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>

This commit uses instanceof instead of Array.isArray() for faster
type checking and avoids calling Object.keys() when the headers are
stored as a 2D array instead of a plain object.

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


Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

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


Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

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


Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Since at least V8 5.4, using function.bind() is now fast enough to
use to avoid recompiling/reoptimizing the same anonymous functions.
These changes especially impact http servers.

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


Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

The new table-based lookups perform significantly better for the
common cases (checking latin1 characters).

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


Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

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


Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

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


Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

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


Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

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


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

The GitHub interface for new pull requests has started prepending
commit messages for single-commit pull requests. It had previously
been appending them. Modify the GitHub template to accommodate the new
behavior.

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


Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>

To ease the use of the AIX binaries, add
/opt/freeware/lib/pthread{/ppc64} into the search path encoded into the
library, so that any version the user has installed from the common
download locations will work out of the box without having to explicitly
set LIBPATH in their environment.

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


Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>