Create a new pull request by comparing changes

AUTHORS

@@ -3434,5 +3434,8 @@ Austin Kelleher <austin.kell47@gmail.com>
 apeltop <sunshine@ptokos.com>
 Livia Medeiros <74449973+LiviaMedeiros@users.noreply.github.com>
 Nikolaos Papaspyrou <nikolaos@chromium.org>
+Matt Probert <1196252+mattpr@users.noreply.github.com>
+Roch Devost <roch.devost@gmail.com>
+Kohei Ueno <kohei.ueno119@gmail.com>
 
 # Generated by tools/update-authors.js

deps/cares/src/lib/ares_expand_name.c

@@ -64,14 +64,16 @@ static int ares__isprint(int ch)
  *  - underscores which are used in SRV records.
  *  - Forward slashes such as are used for classless in-addr.arpa 
  *    delegation (CNAMEs)
+ *  - Asterisks may be used for wildcard domains in CNAMEs as seen in the
+ *    real world.
  * While RFC 2181 section 11 does state not to do validation,
  * that applies to servers, not clients.  Vulnerabilities have been
  * reported when this validation is not performed.  Security is more
  * important than edge-case compatibility (which is probably invalid
  * anyhow). */
 static int is_hostnamech(int ch)
 {
-  /* [A-Za-z0-9-._/]
+  /* [A-Za-z0-9-*._/]
    * Don't use isalnum() as it is locale-specific
    */
   if (ch >= 'A' && ch <= 'Z')
@@ -80,7 +82,7 @@ static int is_hostnamech(int ch)
     return 1;
   if (ch >= '0' && ch <= '9')
     return 1;
-  if (ch == '-' || ch == '.' || ch == '_' || ch == '/')
+  if (ch == '-' || ch == '.' || ch == '_' || ch == '/' || ch == '*')
     return 1;
 
   return 0;

doc/api/child_process.md

@@ -36,8 +36,11 @@ identical to the behavior of pipes in the shell. Use the `{ stdio: 'ignore' }`
 option if the output will not be consumed.
 
 The command lookup is performed using the `options.env.PATH` environment
-variable if it is in the `options` object. Otherwise, `process.env.PATH` is
-used.
+variable if `env` is in the `options` object. Otherwise, `process.env.PATH` is
+used. If `options.env` is set without `PATH`, lookup on Unix is performed
+on a default search path search of `/usr/bin:/bin` (see your operating system's
+manual for execvpe/execvp), on Windows the current processes environment
+variable `PATH` is used.
 
 On Windows, environment variables are case-insensitive. Node.js
 lexicographically sorts the `env` keys and uses the first one that
@@ -807,6 +810,9 @@ pipes between the parent and child. The value is one of the following:
    `child_process` object as [`subprocess.stdio[fd]`][`subprocess.stdio`]. Pipes
    created for fds 0, 1, and 2 are also available as [`subprocess.stdin`][],
    [`subprocess.stdout`][] and [`subprocess.stderr`][], respectively.
+   Currently, these are not actual Unix pipes and therefore the child process
+   can not use them by their descriptor files,
+   e.g. `/dev/fd/2` or `/dev/stdout`.
 2. `'overlapped'`: Same as `'pipe'` except that the `FILE_FLAG_OVERLAPPED` flag
    is set on the handle. This is necessary for overlapped I/O on the child
    process's stdio handles. See the

doc/api/documentation.md

@@ -40,9 +40,13 @@ The stability indices are as follows:
 
 <!-- separator -->
 
-> Stability: 3 - Legacy. The feature is no longer recommended for use. While it
-> likely will not be removed, and is still covered by semantic-versioning
-> guarantees, use of the feature should be avoided.
+> Stability 3 - Legacy. Although this feature is unlikely to be removed and is
+> still covered by semantic-versioning guarantees, it is no longer actively
+> maintained, and other alternatives are available.
+
+Features are marked as legacy rather than being deprecated if their use does no
+harm, and they are widely relied upon within the npm ecosystem. Bugs found in
+legacy features are unlikely to be fixed.
 
 Use caution when making use of Experimental features, particularly within
 modules. Users may not be aware that experimental features are being used.

doc/api/stream.md

@@ -2532,7 +2532,7 @@ const fs = require('fs');
 async function run() {
   await pipeline(
     fs.createReadStream('lowercase.txt'),
-    async function* (source, signal) {
+    async function* (source, { signal }) {
       source.setEncoding('utf8');  // Work with strings rather than `Buffer`s.
       for await (const chunk of source) {
         yield await processChunk(chunk, { signal });
@@ -2556,7 +2556,7 @@ const fs = require('fs');
 
 async function run() {
   await pipeline(
-    async function * (signal) {
+    async function* ({ signal }) {
       await someLongRunningfn({ signal });
       yield 'asd';
     },

doc/api/url.md

@@ -1559,11 +1559,18 @@ A `TypeError` is thrown if `urlString` is not a string.
 
 A `URIError` is thrown if the `auth` property is present but cannot be decoded.
 
-Use of the legacy `url.parse()` method is discouraged. Users should
-use the WHATWG `URL` API. Because the `url.parse()` method uses a
-lenient, non-standard algorithm for parsing URL strings, security
-issues can be introduced. Specifically, issues with [host name spoofing][] and
-incorrect handling of usernames and passwords have been identified.
+`url.parse()` uses a lenient, non-standard algorithm for parsing URL
+strings. It is prone to security issues such as [host name spoofing][]
+and incorrect handling of usernames and passwords.
+
+`url.parse()` is an exception to most of the legacy APIs. Despite its security
+concerns, it is legacy and not deprecated because it is:
+
+* Faster than the alternative WHATWG `URL` parser.
+* Easier to use with regards to relative URLs than the alternative WHATWG `URL` API.
+* Widely relied upon within the npm ecosystem.
+
+Use with caution.
 
 ### `url.resolve(from, to)`
 

lib/internal/modules/esm/formats.js

@@ -29,6 +29,10 @@ if (experimentalWasmModules) {
   extensionFormatMap['.wasm'] = legacyExtensionFormatMap['.wasm'] = 'wasm';
 }
 
+/**
+ * @param {string} mime
+ * @returns {string | null}
+ */
 function mimeToFormat(mime) {
   if (
     RegExpPrototypeTest(

lib/internal/modules/esm/get_format.js

@@ -32,6 +32,10 @@ const protocolHandlers = ObjectAssign(ObjectCreate(null), {
   'node:'() { return 'builtin'; },
 });
 
+/**
+ * @param {URL} parsed
+ * @returns {string | null}
+ */
 function getDataProtocolModuleFormat(parsed) {
   const { 1: mime } = RegExpPrototypeExec(
     /^([^/]+\/[^;,]+)(?:[^,]*?)(;base64)?,/,
@@ -41,6 +45,12 @@ function getDataProtocolModuleFormat(parsed) {
   return mimeToFormat(mime);
 }
 
+/**
+ * @param {URL} url
+ * @param {{parentURL: string}} context
+ * @param {boolean} ignoreErrors
+ * @returns {string}
+ */
 function getFileProtocolModuleFormat(url, context, ignoreErrors) {
   const ext = extname(url.pathname);
   if (ext === '.js') {
@@ -59,6 +69,11 @@ function getFileProtocolModuleFormat(url, context, ignoreErrors) {
   return getLegacyExtensionFormat(ext) ?? null;
 }
 
+/**
+ * @param {URL} url
+ * @param {{parentURL: string}} context
+ * @returns {Promise<string> | undefined} only works when enabled
+ */
 function getHttpProtocolModuleFormat(url, context) {
   if (experimentalNetworkImports) {
     return PromisePrototypeThen(
@@ -70,13 +85,23 @@ function getHttpProtocolModuleFormat(url, context) {
   }
 }
 
+/**
+ * @param {URL | URL['href']} url
+ * @param {{parentURL: string}} context
+ * @returns {Promise<string> | string | undefined} only works when enabled
+ */
 function defaultGetFormatWithoutErrors(url, context) {
   const parsed = new URL(url);
   if (!ObjectPrototypeHasOwnProperty(protocolHandlers, parsed.protocol))
     return null;
   return protocolHandlers[parsed.protocol](parsed, context, true);
 }
 
+/**
+ * @param {URL | URL['href']} url
+ * @param {{parentURL: string}} context
+ * @returns {Promise<string> | string | undefined} only works when enabled
+ */
 function defaultGetFormat(url, context) {
   const parsed = new URL(url);
   return ObjectPrototypeHasOwnProperty(protocolHandlers, parsed.protocol) ?

lib/internal/modules/esm/resolve.js

@@ -79,6 +79,7 @@ const DEFAULT_CONDITIONS_SET = new SafeSet(DEFAULT_CONDITIONS);
  * @typedef {string | string[] | Record<string, unknown>} Exports
  * @typedef {'module' | 'commonjs'} PackageType
  * @typedef {{
+ *   pjsonPath: string,
  *   exports?: ExportConfig;
  *   name?: string;
  *   main?: string;

lib/url.js

@@ -177,7 +177,9 @@ function isIpv6Hostname(hostname) {
 //   as IPv6 by isIpv6Hostname above
 //
 // [1]: https://url.spec.whatwg.org/#forbidden-host-code-point
-const forbiddenHostChars = /[\t\n\r #%/:<>?@[\\\]^|]/;
+const forbiddenHostChars = /[\0\t\n\r #%/:<>?@[\\\]^|]/;
+// For IPv6, permit '[', ']', and ':'.
+const forbiddenHostCharsIpv6 = /[\0\t\n\r #%/<>?@\\^|]/;
 
 Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
   validateString(url, 'url');
@@ -400,27 +402,33 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
       this.hostname = this.hostname.toLowerCase();
     }
 
-    if (!ipv6Hostname && this.hostname !== '') {
-      // IDNA Support: Returns a punycoded representation of "domain".
-      // It only converts parts of the domain name that
-      // have non-ASCII characters, i.e. it doesn't matter if
-      // you call it with a domain that already is ASCII-only.
-
-      // Use lenient mode (`true`) to try to support even non-compliant
-      // URLs.
-      this.hostname = toASCII(this.hostname, true);
-
-      // Prevent two potential routes of hostname spoofing.
-      // 1. If this.hostname is empty, it must have become empty due to toASCII
-      //    since we checked this.hostname above.
-      // 2. If any of forbiddenHostChars appears in this.hostname, it must have
-      //    also gotten in due to toASCII. This is since getHostname would have
-      //    filtered them out otherwise.
-      // Rather than trying to correct this by moving the non-host part into
-      // the pathname as we've done in getHostname, throw an exception to
-      // convey the severity of this issue.
-      if (this.hostname === '' || forbiddenHostChars.test(this.hostname)) {
-        throw new ERR_INVALID_URL(url);
+    if (this.hostname !== '') {
+      if (ipv6Hostname) {
+        if (forbiddenHostCharsIpv6.test(this.hostname)) {
+          throw new ERR_INVALID_URL(url);
+        }
+      } else {
+        // IDNA Support: Returns a punycoded representation of "domain".
+        // It only converts parts of the domain name that
+        // have non-ASCII characters, i.e. it doesn't matter if
+        // you call it with a domain that already is ASCII-only.
+
+        // Use lenient mode (`true`) to try to support even non-compliant
+        // URLs.
+        this.hostname = toASCII(this.hostname, true);
+
+        // Prevent two potential routes of hostname spoofing.
+        // 1. If this.hostname is empty, it must have become empty due to toASCII
+        //    since we checked this.hostname above.
+        // 2. If any of forbiddenHostChars appears in this.hostname, it must have
+        //    also gotten in due to toASCII. This is since getHostname would have
+        //    filtered them out otherwise.
+        // Rather than trying to correct this by moving the non-host part into
+        // the pathname as we've done in getHostname, throw an exception to
+        // convey the severity of this issue.
+        if (this.hostname === '' || forbiddenHostChars.test(this.hostname)) {
+          throw new ERR_INVALID_URL(url);
+        }
       }
     }
 

src/crypto/crypto_ec.cc

@@ -733,7 +733,11 @@ Maybe<void> ExportJWKEcKey(
   BignumPointer x(BN_new());
   BignumPointer y(BN_new());
 
-  EC_POINT_get_affine_coordinates(group, pub, x.get(), y.get(), nullptr);
+  if (!EC_POINT_get_affine_coordinates(group, pub, x.get(), y.get(), nullptr)) {
+    ThrowCryptoError(env, ERR_get_error(),
+                     "Failed to get elliptic-curve point coordinates");
+    return Nothing<void>();
+  }
 
   if (target->Set(
           env->context(),

src/crypto/crypto_hmac.cc

@@ -124,8 +124,11 @@ void Hmac::HmacDigest(const FunctionCallbackInfo<Value>& args) {
   unsigned int md_len = 0;
 
   if (hmac->ctx_) {
-    HMAC_Final(hmac->ctx_.get(), md_value, &md_len);
+    bool ok = HMAC_Final(hmac->ctx_.get(), md_value, &md_len);
     hmac->ctx_.reset();
+    if (!ok) {
+      return ThrowCryptoError(env, ERR_get_error(), "Failed to finalize HMAC");
+    }
   }
 
   Local<Value> error;

src/node_buffer.cc

@@ -220,9 +220,8 @@ inline MUST_USE_RESULT Maybe<bool> ParseArrayIndex(Environment* env,
     return Just(false);
 
   // Check that the result fits in a size_t.
-  const uint64_t kSizeMax = static_cast<uint64_t>(static_cast<size_t>(-1));
   // coverity[pointless_expression]
-  if (static_cast<uint64_t>(tmp_i) > kSizeMax)
+  if (static_cast<uint64_t>(tmp_i) > std::numeric_limits<size_t>::max())
     return Just(false);
 
   *ret = static_cast<size_t>(tmp_i);

src/node_file.cc

@@ -277,13 +277,13 @@ inline void FileHandle::Close() {
                        detail.fd);
     if (env->filehandle_close_warning()) {
       env->set_filehandle_close_warning(false);
-      ProcessEmitDeprecationWarning(
+      USE(ProcessEmitDeprecationWarning(
           env,
           "Closing a FileHandle object on garbage collection is deprecated. "
           "Please close FileHandle objects explicitly using "
           "FileHandle.prototype.close(). In the future, an error will be "
           "thrown if a file descriptor is closed during garbage collection.",
-          "DEP0137").IsNothing();
+          "DEP0137"));
     }
   }, CallbackFlags::kUnrefed);
 }

src/node_url.cc

@@ -230,7 +230,7 @@ unsigned hex2bin(const T ch) {
     return 10 + (ch - 'A');
   if (ch >= 'a' && ch <= 'f')
     return 10 + (ch - 'a');
-  return static_cast<unsigned>(-1);
+  UNREACHABLE();
 }
 
 std::string PercentDecode(const char* input, size_t len) {

test/es-module/test-esm-basic-imports.mjs

@@ -1,7 +1,10 @@
 import '../common/index.mjs';
 import assert from 'assert';
 import ok from '../fixtures/es-modules/test-esm-ok.mjs';
-import okShebang from './test-esm-shebang.mjs';
+import * as okShebangNs from './test-esm-shebang.mjs';
+// encode the '.'
+import * as okShebangPercentNs from './test-esm-shebang%2emjs';
 
 assert(ok);
-assert(okShebang);
+assert(okShebangNs.default);
+assert.strict.equal(okShebangNs, okShebangPercentNs);

test/parallel/test-http-outgoing-internal-headernames-getter.js

@@ -2,6 +2,7 @@
 const common = require('../common');
 
 const { OutgoingMessage } = require('http');
+const assert = require('assert');
 
 const warn = 'OutgoingMessage.prototype._headerNames is deprecated';
 common.expectWarning('DeprecationWarning', warn, 'DEP0066');
@@ -11,3 +12,12 @@ common.expectWarning('DeprecationWarning', warn, 'DEP0066');
   const outgoingMessage = new OutgoingMessage();
   outgoingMessage._headerNames; // eslint-disable-line no-unused-expressions
 }
+
+{
+  // Tests _headerNames getter result after setting a header.
+  const outgoingMessage = new OutgoingMessage();
+  outgoingMessage.setHeader('key', 'value');
+  const expect = Object.create(null);
+  expect.key = 'key';
+  assert.deepStrictEqual(outgoingMessage._headerNames, expect);
+}

test/parallel/test-set-incoming-message-header.js

@@ -0,0 +1,27 @@
+'use strict';
+
+require('../common');
+const { IncomingMessage } = require('http');
+const assert = require('assert');
+
+// Headers setter function set a header correctly
+{
+  const im = new IncomingMessage();
+  im.headers = { key: 'value' };
+  assert.deepStrictEqual(im.headers, { key: 'value' });
+}
+
+// Trailers setter function set a header correctly
+{
+  const im = new IncomingMessage();
+  im.trailers = { key: 'value' };
+  assert.deepStrictEqual(im.trailers, { key: 'value' });
+}
+
+// _addHeaderLines function set a header correctly
+{
+  const im = new IncomingMessage();
+  im.headers = { key1: 'value1' };
+  im._addHeaderLines(['key2', 'value2'], 2);
+  assert.deepStrictEqual(im.headers, { key1: 'value1', key2: 'value2' });
+}

test/parallel/test-url-parse-invalid-input.js

@@ -37,6 +37,10 @@ assert.throws(() => { url.parse('http://%E0%A4%A@fail'); },
                 return e.code === undefined;
               });
 
+assert.throws(() => { url.parse('http://[127.0.0.1\x00c8763]:8000/'); },
+              { code: 'ERR_INVALID_URL', input: 'http://[127.0.0.1\x00c8763]:8000/' }
+);
+
 if (common.hasIntl) {
   // An array of Unicode code points whose Unicode NFKD contains a "bad
   // character".