From d3ed6c153c7561cb9eaf1593dfe41f33584a7a6a Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Wed, 1 Jul 2026 08:40:40 +0200 Subject: [PATCH 1/8] tls: report negotiated TLS groups When OpenSSL reports no peer temporary key object for TLS key agreement, fall back to the negotiated TLS Supported Group name. This lets getEphemeralKeyInfo() identify PQ, hybrid, and other group-based key agreement as TLSGroup without synthesizing a size. Keep existing DH and ECDH results unchanged when OpenSSL provides a recognizable temporary key. Fixes: https://github.com/nodejs/node/issues/59452 Signed-off-by: Filip Skokan PR-URL: https://github.com/nodejs/node/pull/64119 Reviewed-By: Anna Henningsen Reviewed-By: Yagiz Nizipli --- deps/ncrypto/ncrypto.cc | 11 ++++ deps/ncrypto/ncrypto.h | 1 + doc/api/tls.md | 45 +++++++++------ src/crypto/crypto_common.cc | 11 +++- src/env_properties.h | 1 + .../test-tls-client-getephemeralkeyinfo.js | 57 ++++++++++++++++++- 6 files changed, 106 insertions(+), 20 deletions(-) diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index 81af3ded563777..d62485e626c158 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -3086,6 +3086,17 @@ EVPKeyPointer SSLPointer::getPeerTempKey() const { return EVPKeyPointer(raw_key); } +std::optional SSLPointer::getNegotiatedGroup() const { +#if OPENSSL_VERSION_PREREQ(3, 5) + if (!ssl_) return std::nullopt; + const char* group = SSL_get0_group_name(get()); + if (group == nullptr) return std::nullopt; + return group; +#else + return std::nullopt; +#endif +} + std::optional SSLPointer::getCipherName() const { auto cipher = getCipher(); if (cipher == nullptr) return std::nullopt; diff --git a/deps/ncrypto/ncrypto.h b/deps/ncrypto/ncrypto.h index a6befbf7cf6794..76d79652607139 100644 --- a/deps/ncrypto/ncrypto.h +++ b/deps/ncrypto/ncrypto.h @@ -1198,6 +1198,7 @@ class SSLPointer final { std::optional getServerName() const; X509View getCertificate() const; EVPKeyPointer getPeerTempKey() const; + std::optional getNegotiatedGroup() const; const SSL_CIPHER* getCipher() const; bool isServer() const; diff --git a/doc/api/tls.md b/doc/api/tls.md index 28126d021bd7a3..95335e6f275099 100644 --- a/doc/api/tls.md +++ b/doc/api/tls.md @@ -131,7 +131,8 @@ the character "E" appended to the traditional abbreviations): Perfect forward secrecy using ECDHE is enabled by default. The `ecdhCurve` option can be used when creating a TLS server to customize the list of supported -ECDH curves to use. See [`tls.createServer()`][] for more info. +ECDH curves for TLSv1.2 and below, and the list of supported TLS groups for +TLSv1.3. See [`tls.createServer()`][] for more info. DHE is disabled by default but can be enabled alongside ECDHE by setting the `dhparam` option to `'auto'`. Custom DHE parameters are also supported but @@ -1196,12 +1197,19 @@ added: v5.0.0 * Returns: {Object} -Returns an object representing the type, name, and size of parameter of -an ephemeral key exchange in [perfect forward secrecy][] on a client -connection. It returns an empty object when the key exchange is not -ephemeral. As this is only supported on a client socket; `null` is returned -if called on a server socket. The supported types are `'DH'` and `'ECDH'`. The -`name` property is available only when type is `'ECDH'`. +Returns an object describing ephemeral key agreement in [perfect forward +secrecy][] on a client connection. It returns an empty object when the key +agreement is not ephemeral. As this is only supported on a client socket; +`null` is returned if called on a server socket. The supported types are `'DH'`, +`'ECDH'`, and `'TLSGroup'`. For `'DH'` and `'ECDH'`, the object describes peer +temporary key parameters. For `'TLSGroup'`, the object identifies the negotiated +TLS Supported Group used for key agreement when a peer temporary key object is +not available. + +The `name` property is available only when type is `'ECDH'` or `'TLSGroup'`. The +`size` property is not available when type is `'TLSGroup'`. For `'TLSGroup'`, +`name` is the negotiated TLS Supported Group name. Standardized TLS group names +and code points are listed in the [IANA TLS Supported Groups registry][]. For example: `{ type: 'ECDH', name: 'prime256v1', size: 256 }`. @@ -2019,12 +2027,16 @@ changes: required for non-ECDHE [perfect forward secrecy][]. If omitted or invalid, the parameters are silently discarded and DHE ciphers will not be available. [ECDHE][]-based [perfect forward secrecy][] will still be available. - * `ecdhCurve` {string} A string describing a named curve or a colon separated - list of curve NIDs or names, for example `P-521:P-384:P-256`, to use for - ECDH key agreement. Set to `auto` to select the - curve automatically. Use [`crypto.getCurves()`][] to obtain a list of - available curve names. On recent releases, `openssl ecparam -list_curves` - will also display the name and description of each available elliptic curve. + * `ecdhCurve` {string} A string describing a named curve, TLS group, or + colon-separated list of named curves or TLS groups to use for key agreement, + for example `P-521:P-384:P-256`, `X25519`, or `X25519MLKEM768`. The + historical name of this option refers to ECDH key agreement in TLSv1.2 and + below. In TLSv1.3, this option configures the TLS Supported Groups and + key share groups offered or accepted by the TLS stack. Set to `auto` to + select the group automatically. Use [`crypto.getCurves()`][] to obtain a + list of available elliptic curve names. For TLS group names, use + `openssl list -tls-groups` or consult the [IANA TLS Supported Groups + registry][]. **Default:** [`tls.DEFAULT_ECDH_CURVE`][]. * `honorCipherOrder` {boolean} Attempt to use the server's cipher suite preferences instead of the client's. When `true`, causes @@ -2442,9 +2454,9 @@ changes: description: Default value changed to `'auto'`. --> -The default curve name to use for ECDH key agreement in a tls server. The -default value is `'auto'`. See [`tls.createSecureContext()`][] for further -information. +The default named curve or TLS group list to use for key agreement in a TLS +server. The default value is `'auto'`. See [`tls.createSecureContext()`][] for +further information. ## `tls.DEFAULT_MAX_VERSION` @@ -2492,6 +2504,7 @@ added: v0.11.3 [Chrome's 'modern cryptography' setting]: https://www.chromium.org/Home/chromium-security/education/tls#TOC-Cipher-Suites [DHE]: https://en.wikipedia.org/wiki/Diffie%E2%80%93Hellman_key_exchange [ECDHE]: https://en.wikipedia.org/wiki/Elliptic_curve_Diffie%E2%80%93Hellman +[IANA TLS Supported Groups registry]: https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-parameters-8 [Modifying the default TLS cipher suite]: #modifying-the-default-tls-cipher-suite [Mozilla's publicly trusted list of CAs]: https://hg.mozilla.org/mozilla-central/raw-file/tip/security/nss/lib/ckfw/builtins/certdata.txt [OCSP request]: https://en.wikipedia.org/wiki/OCSP_stapling diff --git a/src/crypto/crypto_common.cc b/src/crypto/crypto_common.cc index 4f117f160a9673..0db5d0eaac8fc3 100644 --- a/src/crypto/crypto_common.cc +++ b/src/crypto/crypto_common.cc @@ -215,13 +215,15 @@ MaybeLocal GetEphemeralKey(Environment* env, const SSLPointer& ssl) { Undefined(env->isolate()), // name Undefined(env->isolate()), // size }; - EVPKeyPointer key = ssl.getPeerTempKey(); + + bool found = false; if (EVPKeyPointer key = ssl.getPeerTempKey()) { int kid = key.id(); switch (kid) { case EVP_PKEY_DH: { values[0] = env->dh_string(); values[2] = Integer::New(env->isolate(), key.bits()); + found = true; break; } case EVP_PKEY_EC: @@ -237,10 +239,17 @@ MaybeLocal GetEphemeralKey(Environment* env, const SSLPointer& ssl) { values[0] = env->ecdh_string(); values[1] = OneByteString(env->isolate(), curve_name); values[2] = Integer::New(env->isolate(), key.bits()); + found = true; break; } } } + if (!found) { + if (auto name = ssl.getNegotiatedGroup()) { + values[0] = env->tls_group_string(); + values[1] = OneByteString(env->isolate(), name.value()); + } + } return scope.EscapeMaybe(NewDictionaryInstance(env->context(), tmpl, values)); } diff --git a/src/env_properties.h b/src/env_properties.h index 2e6bc65f89641c..e3179287dce443 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -363,6 +363,7 @@ V(target_string, "target") \ V(thread_id_string, "threadId") \ V(thread_name_string, "threadName") \ + V(tls_group_string, "TLSGroup") \ V(ticketkeycallback_string, "onticketkeycallback") \ V(timeout_string, "timeout") \ V(time_to_first_byte_string, "timeToFirstByte") \ diff --git a/test/parallel/test-tls-client-getephemeralkeyinfo.js b/test/parallel/test-tls-client-getephemeralkeyinfo.js index 2107d024012c4d..ea6dec7bdc4629 100644 --- a/test/parallel/test-tls-client-getephemeralkeyinfo.js +++ b/test/parallel/test-tls-client-getephemeralkeyinfo.js @@ -18,9 +18,6 @@ const tls = require('tls'); const key = fixtures.readKey('agent2-key.pem'); const cert = fixtures.readKey('agent2-cert.pem'); -// TODO(@sam-github) test works with TLS1.3, rework test to add -// 'ECDH' with 'TLS_AES_128_GCM_SHA256', - function loadDHParam(n) { return fixtures.readKey(`dh${n}.pem`); } @@ -89,3 +86,57 @@ test(256, 'ECDH', 'prime256v1', 'ECDHE-RSA-AES256-GCM-SHA384'); test(521, 'ECDH', 'secp521r1', 'ECDHE-RSA-AES256-GCM-SHA384'); test(253, 'ECDH', 'X25519', 'ECDHE-RSA-AES256-GCM-SHA384'); test(448, 'ECDH', 'X448', 'ECDHE-RSA-AES256-GCM-SHA384'); + +function testTLS13Group(size, type, name) { + const options = { + key, + cert, + ecdhCurve: name, + minVersion: 'TLSv1.3', + maxVersion: 'TLSv1.3', + }; + + const server = tls.createServer(options, common.mustCall((conn) => { + assert.strictEqual(conn.getEphemeralKeyInfo(), null); + conn.end(); + })); + + server.on('close', common.mustSucceed()); + + server.listen(0, common.mustCall(() => { + const client = tls.connect({ + port: server.address().port, + rejectUnauthorized: false, + ecdhCurve: name, + minVersion: 'TLSv1.3', + maxVersion: 'TLSv1.3', + }, common.mustCall(() => { + const ekeyinfo = client.getEphemeralKeyInfo(); + assert.strictEqual(ekeyinfo.type, type); + assert.strictEqual(ekeyinfo.size, size); + assert.strictEqual(ekeyinfo.name, name); + server.close(); + })); + client.on('secureConnect', common.mustCall()); + })); +} + +testTLS13Group(253, 'ECDH', 'X25519'); + +if (hasOpenSSL(3, 5)) { + const tls13Groups = [ + 'MLKEM512', + 'MLKEM768', + 'MLKEM1024', + 'SecP256r1MLKEM768', + 'X25519MLKEM768', + 'SecP384r1MLKEM1024', + ]; + + if (hasOpenSSL(4, 0)) { + tls13Groups.push('curveSM2'); + tls13Groups.push('curveSM2MLKEM768'); + } + + tls13Groups.forEach((name) => testTLS13Group(undefined, 'TLSGroup', name)); +} From 9c9c06fb931768288b4b7d0de7ffa33cad17fc6c Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 1 Jul 2026 11:26:09 +0200 Subject: [PATCH 2/8] tools: update c-ares updater script MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Antoine du Hamel PR-URL: https://github.com/nodejs/node/pull/64194 Reviewed-By: Richard Lau Reviewed-By: Joyee Cheung Reviewed-By: Luigi Pinca Reviewed-By: René --- tools/dep_updaters/c-ares.kbx | Bin 0 -> 4217 bytes tools/dep_updaters/update-c-ares.sh | 33 ++++++++++++++-------------- 2 files changed, 17 insertions(+), 16 deletions(-) create mode 100644 tools/dep_updaters/c-ares.kbx diff --git a/tools/dep_updaters/c-ares.kbx b/tools/dep_updaters/c-ares.kbx new file mode 100644 index 0000000000000000000000000000000000000000..720f1df36917d7ccb3c568ccc1ce3599967682f2 GIT binary patch literal 4217 zcmb7`2T)Vpw#QFGfY7B11TcV7q$5oT9WNsFqXtQ6LZ~8yE>%FJNfo3?Zz4siN>iF3 zRS+VfDT4Ig5ri9f&wJn8`R2WOYi94W_S)r~+55Nu>i_@%2Y~>vrV0`R5X@0UwDG*0 z`~PLc-eAy$7YG25p8yzO<6T?c7LjdQ(VIx8Lw$n&e#1f6Mi>D*f&ie=k0w@X9JAwy4ZxU96Ts;4yB78y9c_aKa*&=(2puj0}bPoEU zrTx{oWm0;SoR(>RSKD@aX9)-T!J9t)9X$CD1MQYV=Dx$5U%xEnF6(ax3jXDp`tpSC zm8WVnFPlyD$hP#^y7YX`kV>cIoE@@4-qn8iPSX3k4+rf-<|8I{l2ppNU1#ph58q4X z4#hRa!UN2=H&5r<`B{%?KmJ}-@=n62<6X3d1(Aw^rS_3z;-%<7NR1V7R8wxQVyxNc za9Qj#@)_t3AYW2coe7KC_oDWK(BcL zz|PeU1Ula!n7xOGtE`BKn2e;bm^h)x$Owx|$%={HkQS?jt68}?*gC@vJZxR8ZLxN6 zgv~`j$==GxN7&s~;WgnP2up!4mI5L{(hzzOIlxH_KHqOJjEI8U0D&(D1LS0v{v5i@OVTsF>YS3NW1BfTMkQ7h{O>n`^zKtv6iaTkvoh>@^&C=eEC5Hqr6US4J68MG+g|<;-|(AHEe3-c7}l@1FOb+^_y7$@h4n)-+QW zw4m_>KV3fR0ocKz?Al$ZyTdjLE?DE(oWtcg6?8N>!}V?IRhtlca#G1!Esu(%_~?i! zUI2e(K|K}ud4x#yI&WcTXPwbup#ak5K;{ud`b3LU*!x#)TL^HaVT1G7C)%|%j=v<6 zEo^ot-#>`^yQaBnPf4OXI2J)={PF_TNYE7sGl3|wKSWXfPjZ&;k)9ctgcsjWe79BO zrM6}U_Osc_6xAo$h%$1o&!EgKO*M&h&AKk6Ap)VIu0Ar#>@4!(uc`8}RW^_z zIgK#8eteZNC%(dQ>{>Euw$b|O&WlrnS6P}=ku`iaR;Sz|GHvu^Or+D5 zKxD6pHqCw)>SQet)@d$6C8cl3kkVw1uLyBDp;+3K@Fp(QYy9lKG9BJzU=WJx@e}EM z@6kLh^mty48$G<-^(bnGdOG<``l!cgFey`pjg>9ikrtYI#)!>jAm*=kG@$p9?Ad1! zc6#hO6r8yU`Dn@Go$vUXKO^o})vUn1HyKrL@-T~!NB4TRw}q?zkb`qBA3Ba@x+N@| z-PLK8$5sgw3IYJ~AO9`8Sp;7cAOhIN18nBH1+Aa;y5~`I%?`;K(!S{-Xe9hxC_Pu4 zbLJJ|JI(~d<64J|X{+b36ol}F?2el29Ny@ZOnXE<`vf;sNcTwSc$v_LJfBCf3rPm! z2_F2sNqE!WO7eNvzhoIq1;JQo@q#V^gYDiB(Hw=C$;+dH8I*(B0rrImZS&*u?J^4vXp3YmVp@}}!I>B4a ztj~6)6_`z^ul_JI6pUgY);XdKL+9@1-1O8LNo6@K@>fd6dQ)h>3<=Eby=-2&PsHD; zCyBcD31Rp4zKWOxU?%%h#q~HyTr0uaNgB#ZJvdQ2qG4m+9pIWLsJHWK$#KwYy|=JL z{Ma_g@5kdfRtn+CpCX@X%{q&TZU`E%gwe#3D>`bW2v@ljm2*qTa(oHWBH~LmyZ1f{ zw!+wFWEsbR_#k{}IYV0UtgKf+F7;RC8fJW5$9%RWMwI1f^_KfQ3)e1EmP%nB{dRl9 zW~;0SZmIg&Y^F$1%hs|P6Eoint zUhtdcceL-OYfnAu<()#u;>_gh9WkqPh3*X`n8e&C?PZ{-TKI@jxRwh zC6J<4wSLv|`%7%{<3(y0%ZZZNm}o(}ybuHkGme)jmE2YO%Fn!+C%a|Hq2c#1Js$Bh zDjIWe&vz|kgXD(vw@YeY#XLm{eAJ6O<=vi;JP&eWP|jc23&;bbrB2rps(H(wWS%Vp zUPZbOND63;mMvzht(tR!2U$@Qvn;Oz?^NU-aX;#n?-V~=aWKqjsmSQx3@%mAsAn4T zt_-M+TWN+kk#C|^Ti@gKbO$Rky_j_bS7#e$(2Za2*qM!J)tAuRO>Q%E9|iq z*1Q@5GcKpyIOTW9F?C@#S*IC}W>|4nfFxT?boqW&*O$*TgEU6jL%s?R=SHd<(Ay2A zAlY!cMHwj>y=JRB7(+wrl0?O{{BTfK7_Gk4LeH8MYP{dw04q^cYksILk*WD{uHya- ze#nYmCu|8V&-Tl#qVKewR1;4-aA+da#ma>{Y5!PnMazbK* z0`UI@IUQy&3=BN_H+5cc2n!QAz)qO+cT#@{PK2y>x7vNzjgUMJHGpP}Jm zH$u#mcCb-AKcStrw%tsU#;a5#T|Y6N!#@q!Qq};#!LHO za-IeVjIz{veOeD5%0U5UY)*)QnosX6BT#8vcO+sXEu%(EMT}Yq7aew z>uYngePa*>k$Ptq*N{82*}aa{Pcim>u1g!(v|PpVgvWurP{TbDp2IaE)iL!u5lAl(L`q&gNp8|P1)T{xzd9bU$*5hDrC}if|dR0LBTO@z|P^RI=UXGRP6r&)`C(-486 zq>*5$|C@LsFzn(F?~l&1-l92%Xs21+rkprasxvn{`Qp_uYRcPPjIYq*WIW53Pi^nF zzQtS1@YGWzBE}>~Olb^@4JIBGDzy;X(#s%?s+>Ze6_!y`pljk8@66S0Rzv#Q67w91q>Bc!WyRSAdG9%(Ecy6$By zE<@d#kNeh7@&}YpDvg5Y@23mOWI8KiJ&)oUbI#CmD$!Ect**7YTVK||>~sFAGGQcw z_N@9=uc4Wji=0co+`v*&8Ex0;^6?a?7cN27atdfdf0R(OK-)W|TxlvHZzDzeUo$~p z!jrP)cAhu)A*38hTofBagi3qBS#fSN6C{_yP7M`9aLL31x1bgVOY6EjiJDzxo_0)W zyIft name.endsWith('.tar.gz')); if(!browser_download_url || !name) throw new Error('No tarball found'); -console.log(`${tag_name} ${browser_download_url} ${name}`); +console.log(`${tag_name.replace(/^v/, '')} ${browser_download_url}`); EOF )" -IFS=' ' read -r NEW_VERSION NEW_VERSION_URL ARES_TARBALL < /dev/null || mktemp -d -t 'tmp') +ARES_TARBALL="$WORKSPACE/c-ares.tgz" cleanup () { EXIT_CODE=$? @@ -47,29 +48,29 @@ cleanup () { trap cleanup INT TERM EXIT -cd "$WORKSPACE" - echo "Fetching c-ares source archive" curl -sL -o "$ARES_TARBALL" "$NEW_VERSION_URL" -log_and_verify_sha256sum "c-ares" "$ARES_TARBALL" -gzip -dc "$ARES_TARBALL" | tar xf - -rm -- "$ARES_TARBALL" -FOLDER=$(ls -d -- */) +echo "Verifying PGP signature" +curl -sL -o "$ARES_TARBALL.asc" "$NEW_VERSION_URL.asc" +gpgv --keyring "$BASE_DIR/tools/dep_updaters/c-ares.kbx" "${ARES_TARBALL}.asc" "${ARES_TARBALL}" + +log_and_verify_sha256sum "c-ares" "$ARES_TARBALL" +tar -C "$WORKSPACE" -xzf "$ARES_TARBALL" -mv -- "$FOLDER" cares +ARES_FOLDER=$(find "$WORKSPACE" -mindepth 1 -maxdepth 1 -type d -print -quit) echo "Removing tests" -rm -rf "$WORKSPACE/cares/test" +rm -rf "$ARES_FOLDER/test" echo "Copying existing .gitignore, config, gyp and gn files" -cp -R "$DEPS_DIR/cares/config" "$WORKSPACE/cares" -cp "$DEPS_DIR/cares/.gitignore" "$WORKSPACE/cares" -cp "$DEPS_DIR/cares/cares.gyp" "$WORKSPACE/cares" -cp "$DEPS_DIR/cares/"*.gn "$DEPS_DIR/cares/"*.gni "$WORKSPACE/cares" +cp -R "$DEPS_DIR/cares/config" "$ARES_FOLDER" +cp "$DEPS_DIR/cares/.gitignore" "$ARES_FOLDER" +cp "$DEPS_DIR/cares/cares.gyp" "$ARES_FOLDER" +cp "$DEPS_DIR/cares/"*.gn "$DEPS_DIR/cares/"*.gni "$ARES_FOLDER" echo "Replacing existing c-ares" -replace_dir "$DEPS_DIR/cares" "$WORKSPACE/cares" +replace_dir "$DEPS_DIR/cares" "$ARES_FOLDER" echo "Updating cares.gyp" "$NODE" "$ROOT/tools/dep_updaters/update-c-ares.mjs" From fd3419178e40efbb38e934dcaf543486df79b14e Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 1 Jul 2026 12:44:52 +0200 Subject: [PATCH 3/8] http: document and validate options.path when it's in absolute-form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When `options.path` passed to `http.request()` contains an absolute URL, `http.request` has been sending it directly as the request target in the HTTP 1.1 message. If the receiving server is a proxy, the proxy server typically forwards the request to the destination specified in the request target and ignores the Host header. This means eventually the request can be forwarded to a destination that is not consistent with `options.host`, depending on how the receiving server behaves. Mimatched Host header and request target also violates RFC 9112 Section 3.2, which we have been entirely leaving to the users to verify. This patch documents this behavior and warns that the user needs to ensure the `path`, `option` and `headers` conform to the RFC. If the receiving server is known to be a proxy server because the request is routed by Node.js' built-in HTTP proxy support, we now do a best-effort check to verify that the authority in `options.path` (if absolute), Host headers and `options.host` agree at request construction time. Node.js will give up on the require target rewriting and throw an error when they don't match at request construction. Signed-off-by: Joyee Cheung PR-URL: https://github.com/nodejs/node/pull/64108 Reviewed-By: Matteo Collina Reviewed-By: Tim Perry Reviewed-By: Ulises Gascón --- doc/api/http.md | 14 ++ lib/_http_client.js | 196 +++++++++++++++--- ...-request-absolute-path-authority-match.mjs | 70 +++++++ ...p-proxy-request-authority-construction.mjs | 82 ++++++++ ...st-http-proxy-request-origin-form-path.mjs | 45 ++++ ...t-http-proxy-request-validation-errors.mjs | 119 +++++++++++ test/common/proxy-server.js | 8 +- 7 files changed, 500 insertions(+), 34 deletions(-) create mode 100644 test/client-proxy/test-http-proxy-request-absolute-path-authority-match.mjs create mode 100644 test/client-proxy/test-http-proxy-request-authority-construction.mjs create mode 100644 test/client-proxy/test-http-proxy-request-origin-form-path.mjs create mode 100644 test/client-proxy/test-http-proxy-request-validation-errors.mjs diff --git a/doc/api/http.md b/doc/api/http.md index f23908d70ae90f..a48e747e8647b6 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -4096,6 +4096,18 @@ changes: E.G. `'/index.html?page=12'`. An exception is thrown when the request path contains illegal characters. Currently, only spaces are rejected but that may change in the future. **Default:** `'/'`. + The content in `path` is sent as the [request target][] in the HTTP 1.1 message. + When `path` is an absolute URL, this means the request target in the message in [absolute form][]. + If the receiving server is a proxy, the server typically forwards the request to the + destination specified in the request target, and ignores the `Host` header. + The user needs to make sure that `path`, `host` and the Host headers conform to the + requirement of the [request target][] in the HTTP specification. + When the receiving server is known to be a proxy because the request is routed through + [Built-in Proxy Support][], `http.request` will additionally perform a best-effort + check to see that the `host` option or `Host` in `headers` agrees with the authority + in `path` during the initial construction of the request. It gives up rewriting the + request target for proxying and throws an error if they don't match at request + construction time, though there won't be checks for later header mutations done by the user. * `port` {number} Port of remote server. **Default:** `defaultPort` if set, else `80`. * `protocol` {string} Protocol to use. **Default:** `'http:'`. @@ -4792,5 +4804,7 @@ const agent2 = new http.Agent({ proxyEnv: process.env }); [`writable.destroyed`]: stream.md#writabledestroyed [`writable.uncork()`]: stream.md#writableuncork [`writable.write()`]: stream.md#writablewritechunk-encoding-callback +[absolute form]: https://datatracker.ietf.org/doc/html/rfc9112#section-3.2.2 [information event]: #event-information [initial delay]: net.md#socketsetkeepaliveenable-initialdelay-interval-count +[request target]: https://datatracker.ietf.org/doc/html/rfc9112#section-3.2 diff --git a/lib/_http_client.js b/lib/_http_client.js index 73d7b84c17a8fd..e6bd38aa35ac9d 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -84,6 +84,7 @@ const { validateInteger, validateBoolean, validateOneOf, + validatePort, validateString, } = require('internal/validators'); const { getTimerDuration } = require('internal/timers'); @@ -139,15 +140,134 @@ class HTTPClientAsyncResource { } } +// The only documented shape is [k, v, k, v, ...]. Here we also accept [[k, v], [k, v], ...]. +// for backward compatibility, and reject others. Also reject if there are duplicate Host entries. +// Returns the Host header value, or undefined if absent. +function getHostFromHeaderArray(headers) { + let host; + const isPairs = headers.length > 0 && ArrayIsArray(headers[0]); + if (isPairs) { + for (let i = 0; i < headers.length; i++) { + const entry = headers[i]; + if (!ArrayIsArray(entry)) { + throw new ERR_INVALID_ARG_VALUE(`options.headers[${i}]`, typeof entry, + 'must be an array when headers is passed as an array of pairs'); + } + if (`${entry[0]}`.toLowerCase() === 'host') { + if (host !== undefined) { + throw new ERR_INVALID_ARG_VALUE('options.headers', '(redacted)', + 'must not contain duplicate Host headers'); + } + host = `${entry[1]}`; + } + } + } else { + for (let i = 0; i + 1 < headers.length; i += 2) { + if (`${headers[i]}`.toLowerCase() === 'host') { + if (host !== undefined) { + throw new ERR_INVALID_ARG_VALUE('options.headers', '(redacted)', + 'must not contain duplicate Host headers'); + } + host = `${headers[i + 1]}`; + } + } + } + return host; +} + +function authoritiesMatch(canonicalHost, hostFromHeader) { + let parsed; + try { + parsed = new URL(`http://${hostFromHeader}`); + } catch { + return false; + } + if (parsed.username || parsed.password || + parsed.pathname !== '/' || parsed.search || parsed.hash) { + return false; + } + return parsed.host === canonicalHost; +} + +// https://datatracker.ietf.org/doc/html/rfc9112#section-3.2 +// When the request target is in absolute-form, ensure it is consistent with +// the request authority: same scheme, no userinfo, and an authority +// component agree with options.host[:port]. +function validateRequestAuthority(pathOption, proxyAuthority, userHostHeader, headerArray) { + validatePort(proxyAuthority.port, 'options.port', true); + pathOption = `${pathOption}`; + const requestBase = new URL(`http://${proxyAuthority.host}`); + requestBase.port = proxyAuthority.port; + + const result = { requestBase }; + if (headerArray !== undefined) { + const host = getHostFromHeaderArray(headerArray); + // Since we don't mutate the header array to normalize the Host value, unlike + // in the case of other shapes of headers provided, we check that it is identical + // to the authority from the requestBase. + if (host !== undefined && host !== requestBase.host) { + throw new ERR_INVALID_ARG_VALUE( + 'Host in options.headers', host, + `must match the request authority (${requestBase.host})`); + } + } else if (userHostHeader !== undefined) { + if (!authoritiesMatch(requestBase.host, userHostHeader)) { + throw new ERR_INVALID_ARG_VALUE( + 'Host in options.headers', userHostHeader, + `must match the request authority (${requestBase.host})`); + } + } + + // Per RFC 9112 Section 3.2, if request target is in absolute-form its authority + // must agree with the request authority. + let requestURL; + let isAbsoluteForm = false; + try { + requestURL = new URL(pathOption); + isAbsoluteForm = true; + } catch { + if (pathOption.charCodeAt(0) !== 0x2F) { + throw new ERR_INVALID_ARG_VALUE( + 'options.path', pathOption, 'must be in absolute-form or start with /'); + } + requestURL = new URL(requestBase.origin + pathOption); + } + result.requestURL = requestURL; + if (!isAbsoluteForm) { + return result; + } + + if (requestURL.username || requestURL.password) { + requestURL.username = ''; + requestURL.password = ''; + throw new ERR_INVALID_ARG_VALUE( + 'options.path', requestURL.href, 'must not contain userinfo, use options.auth instead'); + } + + if (requestURL.protocol !== 'http:') { + throw new ERR_INVALID_ARG_VALUE( + 'options.path', requestURL.protocol, 'must use http: scheme when specified as an absolute URL'); + } + + if (requestBase.host !== requestURL.host) { + throw new ERR_INVALID_ARG_VALUE( + 'options.path', requestURL, `must match the request authority (${requestBase.host})`); + } + + return result; +} + // When proxying a HTTP request, the following needs to be done: -// https://datatracker.ietf.org/doc/html/rfc7230#section-5.3.2 +// https://datatracker.ietf.org/doc/html/rfc9112#section-3.2.2 // 1. Rewrite the request path to absolute-form. // 2. Add proxy-connection and proxy-authorization headers appropriately. // // This function checks whether the request should be rewritten for proxying // and modifies the headers as well as req.path if necessary. // The handling of the proxy server connection is done in createConnection. -function rewriteForProxiedHttp(req, reqOptions) { +// It also validates that the Host header and absolute-form path authority match the +// request authority specified by reqOptions. +function rewriteForProxiedHttp(req, reqOptions, proxyAuthority, userHostHeader, headerArray) { if (req._header) { debug('request._header is already sent, skipping rewriteForProxiedHttp', reqOptions); return false; @@ -165,6 +285,25 @@ function rewriteForProxiedHttp(req, reqOptions) { if (!shouldUseProxy) { return false; } + + // Per RFC 9112 Section 3.2.2, we don't need to rewrite CONNECT or OPTIONS * requests. + let requestURL; + if (req.method !== 'CONNECT' && !(req.method === 'OPTIONS' && req.path === '*')) { + // Validate Host header values agree with the request authority before mutating req, + // so a rejected request doesn't leave proxy-* headers stuck on the outgoing header store. + // XXX(joyeecheung): This validates whether the request conforms to the RFC, but here + // we only do it for proxied requests for backward compatibility. For non-proxied requests, + // ensuring thst the request is well formed has been entirely left to the user. + const result = validateRequestAuthority(req.path, proxyAuthority, userHostHeader, headerArray); + if (headerArray === undefined) { + const currentHost = req.getHeader('host'); + if (currentHost !== undefined && currentHost !== result.requestBase.host) { + req.setHeader('Host', result.requestBase.host); + } + } + requestURL = result.requestURL; + } + // Add proxy headers. const { auth, href } = agent[kProxyConfig]; if (auth) { @@ -176,15 +315,10 @@ function rewriteForProxiedHttp(req, reqOptions) { req.setHeader('proxy-connection', 'close'); } - // Convert the path to absolute-form. - // https://datatracker.ietf.org/doc/html/rfc7230#section-5.3.2 - const requestHost = req.getHeader('host') || 'localhost'; - const requestBase = `http://${requestHost}`; - const requestURL = new URL(req.path, requestBase); - if (reqOptions.port) { - requestURL.port = reqOptions.port; + if (requestURL !== undefined) { + // Convert the path to absolute-form. The authority is built from options. + req.path = requestURL.href; } - req.path = requestURL.href; debug(`updated request for HTTP proxy ${href} with ${req.path} `, req[kOutHeaders]); return true; }; @@ -360,6 +494,21 @@ function ClientRequest(input, options, cb) { } } + let hostHeaderFromOptions = host; + // For the Host header, ensure that IPv6 addresses are enclosed + // in square brackets, as defined by URI formatting + // https://tools.ietf.org/html/rfc3986#section-3.2.2 + const posColon = hostHeaderFromOptions.indexOf(':'); + if (posColon !== -1 && + hostHeaderFromOptions.includes(':', posColon + 1) && + hostHeaderFromOptions.charCodeAt(0) !== 91/* '[' */) { + hostHeaderFromOptions = `[${hostHeaderFromOptions}]`; + } + const proxyAuthority = { host: hostHeaderFromOptions, port }; + + if (port && +port !== defaultPort) { + hostHeaderFromOptions += ':' + port; + } const headersArray = ArrayIsArray(options.headers); if (!headersArray) { if (options.headers) { @@ -372,23 +521,12 @@ function ClientRequest(input, options, cb) { } } - if (host && !this.getHeader('host') && setHost) { - let hostHeader = host; - - // For the Host header, ensure that IPv6 addresses are enclosed - // in square brackets, as defined by URI formatting - // https://tools.ietf.org/html/rfc3986#section-3.2.2 - const posColon = hostHeader.indexOf(':'); - if (posColon !== -1 && - hostHeader.includes(':', posColon + 1) && - hostHeader.charCodeAt(0) !== 91/* '[' */) { - hostHeader = `[${hostHeader}]`; - } + // Save the Host header before the implicit auto-set below, so the + // proxy validator can tell user-explicit values from Node-generated ones. + const userHostHeader = this.getHeader('host'); - if (port && +port !== defaultPort) { - hostHeader += ':' + port; - } - this.setHeader('Host', hostHeader); + if (host && !this.getHeader('host') && setHost) { + this.setHeader('Host', hostHeaderFromOptions); } if (options.auth && !this.getHeader('Authorization')) { @@ -401,14 +539,14 @@ function ClientRequest(input, options, cb) { throw new ERR_HTTP_HEADERS_SENT('render'); } - rewriteForProxiedHttp(this, optsWithoutSignal); + rewriteForProxiedHttp(this, optsWithoutSignal, proxyAuthority, userHostHeader); this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n', this[kOutHeaders]); } else { - rewriteForProxiedHttp(this, optsWithoutSignal); + rewriteForProxiedHttp(this, optsWithoutSignal, proxyAuthority, userHostHeader); } } else { - rewriteForProxiedHttp(this, optsWithoutSignal); + rewriteForProxiedHttp(this, optsWithoutSignal, proxyAuthority, undefined, options.headers); this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n', options.headers); } diff --git a/test/client-proxy/test-http-proxy-request-absolute-path-authority-match.mjs b/test/client-proxy/test-http-proxy-request-absolute-path-authority-match.mjs new file mode 100644 index 00000000000000..d544f0d2c6286f --- /dev/null +++ b/test/client-proxy/test-http-proxy-request-absolute-path-authority-match.mjs @@ -0,0 +1,70 @@ +// This tests that proxied HTTP requests succeed end-to-end when the +// absolute-form request-target matches the request authority derived from options. + +import * as common from '../common/index.mjs'; +import assert from 'node:assert'; +import http from 'node:http'; +import { once } from 'events'; +import { createProxyServer } from '../common/proxy-server.js'; + +const server = http.createServer(common.mustCall((req, res) => { + res.end('Hello world'); +}, 6)); +server.on('error', common.mustNotCall()); +server.listen(0); +await once(server, 'listening'); + +const { proxy, logs } = createProxyServer(); +proxy.listen(0); +await once(proxy, 'listening'); + +const port = server.address().port; +const serverHost = `localhost:${port}`; +const requestUrl = `http://${serverHost}/test`; + +const agent = new http.Agent({ + proxyEnv: { + HTTP_PROXY: `http://localhost:${proxy.address().port}`, + }, +}); + +async function roundTrip(options) { + const req = http.request({ agent, ...options }); + req.end(); + const [res] = await once(req, 'response'); + res.setEncoding('utf8'); + let body = ''; + for await (const chunk of res) body += chunk; + assert.strictEqual(body, 'Hello world'); +} + +const baseAbsolute = { host: 'localhost', port, path: requestUrl }; + +const options = [ + // No user-supplied headers. + baseAbsolute, + // Object form with an explicit Host that matches. + { ...baseAbsolute, headers: { Host: serverHost } }, + // Flat array form. + { ...baseAbsolute, headers: ['Host', serverHost] }, + // Array-of-pairs form. + { ...baseAbsolute, headers: [['Host', serverHost]] }, + // Contains defaultPort that matches options.port. + { host: 'localhost', port, defaultPort: port, path: '/test' }, + // Stringifiable non-string path object. + { host: 'localhost', port, path: { toString() { return '/test'; } } }, +]; + +for (const opts of options) { + await roundTrip(opts); + // Check what the proxy server received. + const log = logs.pop(); + assert.strictEqual(logs.length, 0); + assert.strictEqual(log.method, 'GET'); + assert.strictEqual(log.url, requestUrl); + assert.strictEqual(log.headers.host, serverHost); +} + +server.close(); +proxy.close(); +agent.destroy(); diff --git a/test/client-proxy/test-http-proxy-request-authority-construction.mjs b/test/client-proxy/test-http-proxy-request-authority-construction.mjs new file mode 100644 index 00000000000000..873c3ee3ae104b --- /dev/null +++ b/test/client-proxy/test-http-proxy-request-authority-construction.mjs @@ -0,0 +1,82 @@ +// Verify that the request path and Host header are constructed correctly +// for proxied requests across different option combinations. + +import '../common/index.mjs'; +import assert from 'node:assert'; +import http from 'node:http'; + +function makeAgent() { + return new http.Agent({ + proxyEnv: { HTTP_PROXY: 'http://localhost:1' }, + }); +} + +function check(options, expectedPath, expectedHost) { + const agent = makeAgent(); + const req = http.request({ agent, ...options }); + req.on('error', () => {}); + assert.strictEqual(req.path, expectedPath, `path for ${JSON.stringify(options)}`); + assert.strictEqual(req.getHeader('host'), expectedHost, `Host for ${JSON.stringify(options)}`); + req.destroy(); + agent.destroy(); +} + +const cases = [ + // [options, expectedPath, expectedHost] + + // OPTIONS * and CONNECT bypass path rewriting. + [{ host: 'localhost', port: 3000, method: 'OPTIONS', path: '*' }, + '*', 'localhost:3000'], + [{ host: 'example.com', port: 443, method: 'CONNECT', path: 'example.com:443' }, + 'example.com:443', 'example.com:443'], + + // Basic cases: implicit Host, various port/defaultPort combos. + [{ host: 'localhost', port: 3000, path: '/a' }, + 'http://localhost:3000/a', 'localhost:3000'], + [{ host: 'localhost', port: 80, path: '/b' }, + 'http://localhost/b', 'localhost'], + [{ host: 'example.com', path: '/c' }, + 'http://example.com/c', 'example.com'], + [{ host: 'localhost', port: '3000', path: '/d' }, + 'http://localhost:3000/d', 'localhost:3000'], + + // defaultPort suppresses port in auto-set Host, canonicalization corrects it. + [{ host: 'localhost', port: 80, defaultPort: 8080, path: '/e' }, + 'http://localhost/e', 'localhost'], + [{ host: 'localhost', port: 3000, defaultPort: 3000, path: '/f' }, + 'http://localhost:3000/f', 'localhost:3000'], + + // User-explicit Host header: canonicalized to match request target. + [{ host: 'localhost', port: 80, defaultPort: 8080, path: '/g', headers: { Host: 'localhost:80' } }, + 'http://localhost/g', 'localhost'], + [{ host: 'localhost', port: 80, defaultPort: 8080, path: '/h', headers: { Host: 'localhost' } }, + 'http://localhost/h', 'localhost'], + [{ host: 'localhost', port: 80, path: '/i', headers: { Host: 'localhost:80' } }, + 'http://localhost/i', 'localhost'], + [{ host: 'localhost', port: 3000, defaultPort: 3000, path: '/j', headers: { Host: 'localhost:3000' } }, + 'http://localhost:3000/j', 'localhost:3000'], + [{ host: 'localhost', port: 3000, path: '/k', headers: { Host: 'LOCALHOST:3000' } }, + 'http://localhost:3000/k', 'localhost:3000'], + + // setHost=false with user-provided Host. + [{ host: 'localhost', port: 3000, setHost: false, path: '/l', headers: { Host: 'localhost:3000' } }, + 'http://localhost:3000/l', 'localhost:3000'], + + // IPv6. + [{ host: '::1', port: 3000, path: '/m' }, + 'http://[::1]:3000/m', '[::1]:3000'], + [{ host: '::1', port: 80, path: '/n' }, + 'http://[::1]/n', '[::1]'], + + // Mixed-case host option: URL normalizes to lowercase. + [{ host: 'LocalHost', port: 3000, path: '/o' }, + 'http://localhost:3000/o', 'localhost:3000'], + + // Absolute-form path matching authority. + [{ host: 'localhost', port: 3000, path: 'http://localhost:3000/p', headers: { Host: 'localhost:3000' } }, + 'http://localhost:3000/p', 'localhost:3000'], +]; + +for (const [options, expectedPath, expectedHost] of cases) { + check(options, expectedPath, expectedHost); +} diff --git a/test/client-proxy/test-http-proxy-request-origin-form-path.mjs b/test/client-proxy/test-http-proxy-request-origin-form-path.mjs new file mode 100644 index 00000000000000..011276de03895a --- /dev/null +++ b/test/client-proxy/test-http-proxy-request-origin-form-path.mjs @@ -0,0 +1,45 @@ +// This tests that origin-form paths are preserved when proxied, including +// paths starting with //. + +import * as common from '../common/index.mjs'; +import assert from 'node:assert'; +import http from 'node:http'; +import { once } from 'events'; +import { createProxyServer } from '../common/proxy-server.js'; + +const server = http.createServer(common.mustCall((req, res) => { + res.end('Hello world'); +}, 1)); +server.on('error', common.mustNotCall()); +server.listen(0); +await once(server, 'listening'); + +const { proxy, logs } = createProxyServer(); +proxy.listen(0); +await once(proxy, 'listening'); + +const port = server.address().port; +const serverHost = `localhost:${port}`; + +const agent = new http.Agent({ + proxyEnv: { + HTTP_PROXY: `http://localhost:${proxy.address().port}`, + }, +}); + +const req = http.request({ agent, host: 'localhost', port, path: '//foo/bar' }); +req.end(); +const [res] = await once(req, 'response'); +res.setEncoding('utf8'); +let body = ''; +for await (const chunk of res) body += chunk; +assert.strictEqual(body, 'Hello world'); + +assert.strictEqual(logs.length, 1); +assert.strictEqual(logs[0].method, 'GET'); +assert.strictEqual(logs[0].url, `http://${serverHost}//foo/bar`); +assert.strictEqual(logs[0].headers.host, serverHost); + +server.close(); +proxy.close(); +agent.destroy(); diff --git a/test/client-proxy/test-http-proxy-request-validation-errors.mjs b/test/client-proxy/test-http-proxy-request-validation-errors.mjs new file mode 100644 index 00000000000000..8607b60fb3875c --- /dev/null +++ b/test/client-proxy/test-http-proxy-request-validation-errors.mjs @@ -0,0 +1,119 @@ +// This tests that proxied HTTP requests fail validation before sending any data. + +import * as common from '../common/index.mjs'; +import assert from 'node:assert'; +import http from 'node:http'; +import { once } from 'events'; +import { createProxyServer } from '../common/proxy-server.js'; + +const target = http.createServer(common.mustNotCall()); +target.listen(0); +await once(target, 'listening'); +target.on('error', common.mustNotCall()); + +const { proxy, logs } = createProxyServer(); +proxy.listen(0); +await once(proxy, 'listening'); + +const agent = new http.Agent({ + proxyEnv: { + HTTP_PROXY: `http://localhost:${proxy.address().port}`, + }, +}); + +const port = target.address().port; +const base = { host: 'localhost', port, path: '/test', agent }; + +function throwsWith(message, cases) { + for (const { name, options } of cases) { + assert.throws(() => { + http.request(options, common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE', message }, name); + } +} + +// Path authority or Host header disagrees with options.host:port. +throwsWith(/must match the request authority/, [ + { name: 'absolute path with different host', + options: { ...base, path: 'http://example.test/test' } }, + { name: 'object Host header with different host', + options: { ...base, headers: { Host: 'bad.example' } } }, + { name: 'object Host header with different port', + options: { ...base, headers: { Host: 'localhost:1' } } }, + { name: 'array Host header with different host', + options: { ...base, headers: ['Host', 'bad.example'] } }, + { name: 'array-of-pairs Host header with different host', + options: { ...base, headers: [['Host', 'bad.example']] } }, + { name: 'array Host header omitting port for non-default port', + options: { ...base, headers: ['Host', 'localhost'] } }, + { name: 'absolute path omitting port for non-default port', + options: { ...base, path: 'http://localhost/test' } }, + { name: 'object Host header omitting port when defaultPort suppresses it', + options: { ...base, defaultPort: port, headers: { Host: 'localhost' } } }, +]); + +// Absolute-form path uses a non-http scheme. +throwsWith(/must use http: scheme/, [ + { name: 'absolute path with https: scheme', + options: { ...base, path: `https://localhost:${port}/test` } }, +]); + +// Absolute-form path contains userinfo. +throwsWith(/must not contain userinfo/, [ + { name: 'absolute path with userinfo', + options: { ...base, path: `http://user:pass@localhost:${port}/test` } }, +]); + +// Origin-form path must start with /. +throwsWith(/must be in absolute-form or start with \//, [ + { name: 'path without leading slash but includes @', + options: { ...base, path: '@other.example/test' } }, +]); + +// Host header value is not a bare authority (contains userinfo, path, query, or fragment). +throwsWith(/must match the request authority/, [ + { name: 'Host with userinfo', + options: { ...base, headers: { Host: `user@localhost:${port}` } } }, + { name: 'Host with path', + options: { ...base, headers: { Host: `localhost:${port}/path` } } }, + { name: 'Host with query', + options: { ...base, headers: { Host: `localhost:${port}?x` } } }, + { name: 'Host with fragment', + options: { ...base, headers: { Host: `localhost:${port}#x` } } }, +]); + +// Multiple Host headers (invalid per RFC 9110 Section 5.3). +throwsWith(/must not contain duplicate Host headers/, [ + { name: 'flat array with duplicate Host', + options: { ...base, headers: ['Host', `localhost:${port}`, 'Host', 'bad.example'] } }, + { name: 'array-of-pairs with duplicate Host', + options: { ...base, headers: [['Host', `localhost:${port}`], ['Host', 'bad.example']] } }, + { name: 'case-insensitive duplicate Host', + options: { ...base, headers: ['host', `localhost:${port}`, 'HOST', 'bad.example'] } }, +]); + +// Non-array entry in array-of-pairs form. +throwsWith(/must be an array when headers is passed as an array of pairs/, [ + { name: 'object with numeric keys smuggling a Host', + options: { ...base, headers: [['Host', `localhost:${port}`], { 0: 'Host', 1: 'bad.example' }] } }, +]); + +// Invalid port. +for (const { name, badPort } of [ + { name: 'port >= 65536', badPort: 99999 }, + { name: 'port === 65536', badPort: 65536 }, +]) { + assert.throws(() => { + http.request({ ...base, port: badPort }, common.mustNotCall()); + }, { code: 'ERR_SOCKET_BAD_PORT' }, name); +} + +assert.throws(() => { + http.request({ ...base, method: 'GET', path: '*' }, common.mustNotCall()); +}, { code: 'ERR_INVALID_ARG_VALUE', message: /must be in absolute-form or start with \// }); + +assert.deepStrictEqual(logs, []); + +target.close(); +proxy.close(); +agent.destroy(); diff --git a/test/common/proxy-server.js b/test/common/proxy-server.js index c4cd19fe610b81..a2f8bd12e6257e 100644 --- a/test/common/proxy-server.js +++ b/test/common/proxy-server.js @@ -34,13 +34,11 @@ function createProxyServer(options = {}) { } proxy.on('request', (req, res) => { logRequest(logs, req); - const { hostname, port } = new URL(`http://${req.headers.host}`); - const targetPort = port || 80; - + // Route based on the absolute-form request-target. const url = new URL(req.url); const options = { - hostname: hostname.startsWith('[') ? hostname.slice(1, -1) : hostname, - port: targetPort, + hostname: url.hostname.startsWith('[') ? url.hostname.slice(1, -1) : url.hostname, + port: url.port || 80, path: url.pathname + url.search, // Convert back to relative URL. method: req.method, headers: { From 29ca67351dccc628604c1a87764976a94ab6ec70 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 1 Jul 2026 12:45:14 +0200 Subject: [PATCH 4/8] build: add manually-dispatched stress-test workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a GitHub action workflow that can be manually dispatched to stress-run tests on a PR/branch to verify flakiness. Signed-off-by: Joyee Cheung PR-URL: https://github.com/nodejs/node/pull/64118 Reviewed-By: Filip Skokan Reviewed-By: René --- .github/workflows/stress-test.yml | 116 ++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 .github/workflows/stress-test.yml diff --git a/.github/workflows/stress-test.yml b/.github/workflows/stress-test.yml new file mode 100644 index 00000000000000..e3846cf4d387ac --- /dev/null +++ b/.github/workflows/stress-test.yml @@ -0,0 +1,116 @@ +name: Stress Test (rebase tested branch on main for cache reuse) + +on: + workflow_dispatch: + inputs: + test_path: + description: Test path or glob (e.g. test/parallel/test-debugger-break.js or "test/parallel/test-debugger-*") + required: true + type: string + os: + description: Runner to use + required: true + default: macos-15 + type: choice + options: + - macos-15 + - ubuntu-24.04 + - ubuntu-24.04-arm + repeat: + description: Number of times to repeat each test (--repeat) + required: true + default: '1000' + type: string + test_jobs: + description: Number of jobs to run in parallel (-j). + required: true + default: '1' + type: string + test_args: + description: Extra args for tools/test.py, space-separated (e.g. "-t 10 --worker") + required: false + default: '' + type: string + +env: + PYTHON_VERSION: '3.14' + XCODE_VERSION: '16.4' + CLANG_VERSION: '19' + RUSTC_VERSION: '1.82' + +permissions: + contents: read + +jobs: + stress-test: + runs-on: ${{ inputs.os }} + env: + # Linux builds with clang (matching test-linux.yml), macOS with gcc/g++. + CC: ${{ startsWith(inputs.os, 'ubuntu') && 'sccache clang-19' || 'sccache gcc' }} + CXX: ${{ startsWith(inputs.os, 'ubuntu') && 'sccache clang++-19' || 'sccache g++' }} + # Enable sccache - this is okay for a manually-dispatched workflow that is typically used to + # test lib-only or test-only deflaking changes. This should be able to pick up cache from + # test-linux.yml and test-macos.yml running on the main branch. + SCCACHE_GHA_ENABLED: 'true' + SCCACHE_IDLE_TIMEOUT: '0' + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: false + path: node + - name: Install Clang ${{ env.CLANG_VERSION }} + if: runner.os == 'Linux' + uses: ./node/.github/actions/install-clang + with: + clang-version: ${{ env.CLANG_VERSION }} + - name: Set up Xcode ${{ env.XCODE_VERSION }} + if: runner.os == 'macOS' + run: sudo xcode-select -s /Applications/Xcode_${{ env.XCODE_VERSION }}.app + - name: Set up Python ${{ env.PYTHON_VERSION }} + uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 + with: + python-version: ${{ env.PYTHON_VERSION }} + allow-prereleases: true + - name: Install Rust ${{ env.RUSTC_VERSION }} + run: | + rustup override set "$RUSTC_VERSION" + rustup --version + - name: Set up sccache + uses: Mozilla-Actions/sccache-action@9e7fa8a12102821edf02ca5dbea1acd0f89a2696 # v0.0.10 + with: + version: v0.16.0 + - name: Environment Information + run: npx envinfo@7.21.0 + # This is needed due to https://github.com/nodejs/build/issues/3878 + - name: Cleanup + if: runner.os == 'macOS' + run: | + echo "::group::Free space before cleanup" + df -h + echo "::endgroup::" + echo "::group::Cleaned Files" + + sudo rm -rf /Users/runner/Library/Android/sdk + + echo "::endgroup::" + echo "::group::Free space after cleanup" + df -h + echo "::endgroup::" + - name: Build + run: make -C node build-ci -j"$(getconf _NPROCESSORS_ONLN)" V=1 CONFIG_FLAGS="--error-on-warn --v8-enable-temporal-support" + - name: Stress run ${{ inputs.test_path }} x${{ inputs.repeat }} + shell: bash + env: + REPEAT: ${{ inputs.repeat }} + JOBS: ${{ inputs.test_jobs }} + TEST_ARGS: ${{ inputs.test_args }} + TEST_PATH: ${{ inputs.test_path }} + run: | + cd node + read -ra EXTRA_ARGS <<< "$TEST_ARGS" + python3 tools/test.py \ + --repeat "$REPEAT" \ + -j "$JOBS" \ + -p actions \ + "${EXTRA_ARGS[@]}" \ + "$TEST_PATH" From 7ba71f7f0d6c4316804e936a2123b75eea922585 Mon Sep 17 00:00:00 2001 From: Stewart X Addison Date: Mon, 29 Jun 2026 09:26:45 +0100 Subject: [PATCH 5/8] doc: add sxa GPG key (ed25519) Signed-off-by: Stewart X Addison PR-URL: https://github.com/nodejs/node/pull/64193 Reviewed-By: Antoine du Hamel Reviewed-By: Richard Lau --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index f93cfa6d492b47..dc175eadf5bb6a 100644 --- a/README.md +++ b/README.md @@ -785,6 +785,8 @@ Primary GPG keys for Node.js Releasers (some Releasers sign with subkeys): `C82FA3AE1CBEDC6BE46B9360C43CEC45C17AB93C` * **Ruy Adorno** <> `108F52B48DB57BB0CC439B2997B01419BD92F80A` +* **Stewart X Addison** <> + `655F3B5C1FB3FA8D1A0CA6BDE4A7D232B936D2FD` * **Ulises Gascón** <> `A363A499291CBBC940DD62E41F10027AF002F8B0` @@ -802,6 +804,7 @@ gpg --keyserver hkps://keys.openpgp.org --recv-keys 8FCCA13FEF1D0C2E91008E09770F gpg --keyserver hkps://keys.openpgp.org --recv-keys 890C08DB8579162FEE0DF9DB8BEAB4DFCF555EF4 # Rafael Gonzaga gpg --keyserver hkps://keys.openpgp.org --recv-keys C82FA3AE1CBEDC6BE46B9360C43CEC45C17AB93C # Richard Lau gpg --keyserver hkps://keys.openpgp.org --recv-keys 108F52B48DB57BB0CC439B2997B01419BD92F80A # Ruy Adorno +gpg --keyserver hkps://keys.openpgp.org --recv-keys 655F3B5C1FB3FA8D1A0CA6BDE4A7D232B936D2FD # Stewart X Addison gpg --keyserver hkps://keys.openpgp.org --recv-keys A363A499291CBBC940DD62E41F10027AF002F8B0 # Ulises Gascón ``` From f1e01e76e76de524bf21a28d8c8f6a43856754a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Wed, 1 Jul 2026 12:20:50 +0100 Subject: [PATCH 6/8] build: suppress clang errors building libffi on Windows Signed-off-by: Renegade334 PR-URL: https://github.com/nodejs/node/pull/64222 Refs: https://github.com/nodejs/node/pull/64040 Reviewed-By: Chengzhong Wu Reviewed-By: Stefan Stojanovic Reviewed-By: Richard Lau --- deps/libffi/libffi.gyp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/deps/libffi/libffi.gyp b/deps/libffi/libffi.gyp index f3122a4e2a0041..2d43dcdb733c97 100644 --- a/deps/libffi/libffi.gyp +++ b/deps/libffi/libffi.gyp @@ -204,6 +204,13 @@ }, ], 'conditions': [ + ['OS == "win" and clang == 1', { + 'msvs_settings': { + 'VCCLCompilerTool': { + 'AdditionalOptions': ['-Wno-incompatible-pointer-types'], + }, + }, + }], ['OS == "win" and (target_arch == "x64" or target_arch == "x86_64")', { 'actions': [ { From ed33235bdaff6013786a432d1ff1d78edff9f627 Mon Sep 17 00:00:00 2001 From: Daijiro Wachi Date: Wed, 1 Jul 2026 20:49:20 +0900 Subject: [PATCH 7/8] doc: fix Fast FFI argument count in ffi.md The text said functions with more than 7 arguments fall back to the generic call path, contradicting the preceding sentence that Fast FFI calls support up to 8 arguments. The real limit is architecture- and type-dependent: the hard cap is 8 public arguments (src/ffi/fast.cc), but register pressure lowers the effective count to 7 integer/pointer arguments on AArch64 and 6 on x86-64, while floating-point arguments can use up to 8 on both. Describe this instead of the previous, inaccurate single threshold. Signed-off-by: Daijiro Wachi PR-URL: https://github.com/nodejs/node/pull/63960 Reviewed-By: Colin Ihrig Reviewed-By: Paolo Insogna --- doc/api/ffi.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/doc/api/ffi.md b/doc/api/ffi.md index 4deef4accad375..172af6bbc6edee 100644 --- a/doc/api/ffi.md +++ b/doc/api/ffi.md @@ -125,8 +125,13 @@ raw pointer `bigint` values. For pointer-like parameters, `null`, `undefined`, strings, `Buffer`, typed array, `DataView`, and `ArrayBuffer` values are converted on the JavaScript side before calling the optimized native wrapper. -Optimized Fast FFI calls support at most 8 function arguments. Functions with -more than 7 arguments use the generic FFI call path instead. +Optimized Fast FFI calls support at most 8 function arguments, but the exact +limit depends on the architecture and on the argument types, because each +argument must fit in the registers used by the platform trampoline. Integer +and pointer arguments are limited to 7 on AArch64 and to 6 on x86-64, while +floating-point arguments can use up to 8 on both. Functions that exceed these +limits, including any function with more than 8 arguments, use the generic FFI +call path instead. ## Signature objects From 42fd49a4af6d62d8faf5a633af8f3cae5466b70e Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Wed, 1 Jul 2026 08:43:10 -0500 Subject: [PATCH 8/8] build: enable Maglev for riscv64 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit V8's Maglev compiler has supported riscv64 since V8 14.0 (with full source files in deps/v8/src/maglev/riscv/), but Node.js never wired it up: - configure.py excluded riscv64 from maglev_enabled_architectures - tools/v8_gypfiles/v8.gyp lacked GN-scraper conditions for riscv64 Maglev sources in both the v8_internal_headers and v8_base_without_compiler blocks This adds riscv64 to both, following the same pattern used for s390x in #60863 and matching V8's own BUILD.gn which already lists riscv64 alongside arm, arm64, x64, s390x, and ppc64 as Maglev-enabled architectures. Refs: https://github.com/nodejs/build/issues/4099 Signed-off-by: Jamie Magee PR-URL: https://github.com/nodejs/node/pull/62605 Reviewed-By: René Reviewed-By: Stewart X Addison Reviewed-By: Richard Lau --- configure.py | 2 +- tools/v8_gypfiles/v8.gyp | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/configure.py b/configure.py index a5b57064d1fa4a..2dc6d72d159cc4 100755 --- a/configure.py +++ b/configure.py @@ -55,7 +55,7 @@ valid_mips_float_abi = ('soft', 'hard') valid_intl_modes = ('none', 'small-icu', 'full-icu', 'system-icu') icu_versions = json.loads((tools_path / 'icu' / 'icu_versions.json').read_text(encoding='utf-8')) -maglev_enabled_architectures = ('x64', 'arm', 'arm64', 'ppc64', 's390x') +maglev_enabled_architectures = ('x64', 'arm', 'arm64', 'ppc64', 's390x', 'riscv64') # builtins may be removed later if they have been disabled by options shareable_builtins = {'undici/undici': 'deps/undici/undici.js', diff --git a/tools/v8_gypfiles/v8.gyp b/tools/v8_gypfiles/v8.gyp index 38732b4b34cf68..193b8aebdd28f1 100644 --- a/tools/v8_gypfiles/v8.gyp +++ b/tools/v8_gypfiles/v8.gyp @@ -671,6 +671,11 @@ '