From e116db2989cd2b71dfe0608b5f7e33722f792a1b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 14 May 2026 09:07:17 +0000 Subject: [PATCH 1/3] fetch: strip IPv6 brackets before TLS cert verification The URL parser preserves the surrounding brackets on IPv6 hostnames ("[::1]"). When that form reached TLS certificate verification, two paths broke: - Native fast path: strings::is_ip_address("[::1]") returns false, so check_x509_server_identity took the DNS-name branch and never compared against the cert's IP SAN entries. - User-supplied tls.checkServerIdentity callback: the hostname was handed to the user verbatim; if the user then forwarded it to node:tls.checkServerIdentity, net.isIP("[::1]") === 0 meant the same DNS-name fallthrough and ERR_TLS_CERT_ALTNAME_INVALID. Node.js strips brackets in urlToHttpOptions before either path runs. Mirror that at the single chokepoint get_tls_hostname in src/http/lib.rs, so every fetch TLS consumer (native cert check, SNI's is_ip_address check, and the user-supplied checkServerIdentity callback) sees the bare address. Test gated on isIPv6() (matches valkey-tls-verify.test.ts) and placed in its own file so the subprocess env doesn't race with other concurrent tests in fetch.tls.test.ts. Asserts stdout/stderr before exitCode per CLAUDE.md. Fixes #30668 --- src/http/lib.rs | 23 +++++- test/js/web/fetch/fetch.tls.ipv6.test.ts | 94 ++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 test/js/web/fetch/fetch.tls.ipv6.test.ts diff --git a/src/http/lib.rs b/src/http/lib.rs index 6f28a8cfb2a..dcb82dba149 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -980,10 +980,15 @@ pub(crate) fn abort_tracker() -> &'static mut ArrayHashMap /// Priority: tls_props.server_name > client.hostname > client.url.hostname /// The Host header value (client.hostname) may contain a port suffix which /// must be stripped because it is not part of the DNS name in certificates. +/// IPv6 literals have their surrounding brackets stripped (e.g. "[::1]" -> "::1") +/// so downstream consumers (boringssl::check_x509_server_identity, SNI's +/// is_ip_address check, and any user-supplied checkServerIdentity callback) see +/// the bare address. This mirrors Node.js, which strips brackets in +/// urlToHttpOptions before the hostname reaches tls.checkServerIdentity. fn get_tls_hostname<'c>(client: &'c HTTPClient<'_>, allow_proxy_url: bool) -> &'c [u8] { if allow_proxy_url { if let Some(proxy) = &client.http_proxy { - return proxy.hostname; + return strip_ipv6_brackets(proxy.hostname); } } // Prefer the explicit TLS server_name (e.g. from Node.js servername option) @@ -996,15 +1001,25 @@ fn get_tls_hostname<'c>(client: &'c HTTPClient<'_>, allow_proxy_url: bool) -> &' // `client.tls_props`) without a `(ptr,len)` round-trip. let sn_slice = unsafe { bun_core::ffi::cstr(sn) }.to_bytes(); if !sn_slice.is_empty() { - return sn_slice; + return strip_ipv6_brackets(sn_slice); } } } // client.hostname comes from the Host header and may include ":port" if let Some(host) = &client.hostname { - return strip_port_from_host(host); + return strip_ipv6_brackets(strip_port_from_host(host)); + } + strip_ipv6_brackets(client.url.hostname) +} + +/// Strips surrounding `[` `]` from an IPv6 literal, e.g. "[::1]" -> "::1". +/// Leaves non-bracketed hostnames unchanged. +fn strip_ipv6_brackets(host: &[u8]) -> &[u8] { + if host.len() >= 2 && host[0] == b'[' && host[host.len() - 1] == b']' { + &host[1..host.len() - 1] + } else { + host } - client.url.hostname } // ── support types ─────────────────────────────────────────────────────── diff --git a/test/js/web/fetch/fetch.tls.ipv6.test.ts b/test/js/web/fetch/fetch.tls.ipv6.test.ts new file mode 100644 index 00000000000..ece9805aebd --- /dev/null +++ b/test/js/web/fetch/fetch.tls.ipv6.test.ts @@ -0,0 +1,94 @@ +// Regression test for https://github.com/oven-sh/bun/issues/30668 +// +// The URL parser keeps the surrounding `[`/`]` on IPv6 hostnames. That bracketed +// form must NOT leak into TLS certificate verification: +// - strings::is_ip_address("[::1]") is false, so the native fast path would +// take the DNS-name branch and skip the IP-SAN match on the cert. +// - node:tls.checkServerIdentity uses net.isIP("[::1]") === 0 and likewise +// falls through to CN matching and emits ERR_TLS_CERT_ALTNAME_INVALID. +// +// Node.js strips brackets in urlToHttpOptions before either verification path +// runs. get_tls_hostname in src/http/lib.rs now mirrors that contract so every +// TLS consumer (native cert check, SNI's is_ip_address check, and the +// user-supplied checkServerIdentity callback) sees the bare address `::1`. +// +// This test lives in its own file rather than fetch.tls.test.ts because the +// subprocess approach is sensitive to environment setup (HTTP_PROXY, +// NO_PROXY) and we want a clean top-level env rather than sharing one with +// other concurrent tests in that file. + +import { expect, it } from "bun:test"; +import { bunEnv, bunExe, isIPv6, tls as validTls } from "harness"; + +// Skipped on Buildkite Linux — those AWS instances don't have IPv6 set up +// (see `isIPv6` in harness.ts). Matches the gating pattern already used by +// the directly analogous valkey-tls-verify.test.ts:177. +it.skipIf(!isIPv6())("fetch with IPv6 literal hostname verifies the certificate", async () => { + using server = Bun.serve({ + port: 0, + tls: validTls, + fetch() { + return new Response("Hello World"); + }, + }); + const port = server.port; + + // Drop HTTP_PROXY/HTTPS_PROXY so the container-level egress proxy doesn't + // intercept a request to [::1] (NO_PROXY doesn't match the bracketed form + // in some fetch paths). + const { HTTP_PROXY, HTTPS_PROXY, http_proxy, https_proxy, ...cleanEnv } = bunEnv; + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const tls = require("node:tls"); + const cert = ${JSON.stringify(validTls.cert)}; + const url = "https://[::1]:${port}/"; + // Native fast path — no user-supplied checkServerIdentity. Native + // check_x509_server_identity must see "::1" (not "[::1]") so that + // strings::is_ip_address() picks the IP-SAN branch. + { + const res = await fetch(url, { + keepalive: false, + tls: { ca: cert, rejectUnauthorized: true }, + }); + console.log("native:", await res.text()); + } + // User-supplied checkServerIdentity path — the hostname handed to + // the callback must be bracket-stripped so that + // node:tls.checkServerIdentity (net.isIP) accepts it, matching + // Node.js's urlToHttpOptions behavior. + { + let observed; + const res = await fetch(url, { + keepalive: false, + tls: { + ca: cert, + rejectUnauthorized: true, + checkServerIdentity(hostname, cert) { + observed = hostname; + return tls.checkServerIdentity(hostname, cert); + }, + }, + }); + console.log("callback hostname:", observed); + console.log("js:", await res.text()); + } + `, + ], + env: cleanEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + proc.stdout.text(), + proc.stderr.text(), + proc.exited, + ]); + // Asserting stdout/stderr before exitCode produces a more useful failure + // message when the subprocess crashes unexpectedly. + expect(stdout).toBe("native: Hello World\ncallback hostname: ::1\njs: Hello World\n"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); +}); From 756439ca8c8b47440b7d40f6c19ef356b9cd4d59 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 14 May 2026 09:10:40 +0000 Subject: [PATCH 2/3] [autofix.ci] apply automated fixes --- test/js/web/fetch/fetch.tls.ipv6.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/js/web/fetch/fetch.tls.ipv6.test.ts b/test/js/web/fetch/fetch.tls.ipv6.test.ts index ece9805aebd..cd35a15eb4e 100644 --- a/test/js/web/fetch/fetch.tls.ipv6.test.ts +++ b/test/js/web/fetch/fetch.tls.ipv6.test.ts @@ -81,11 +81,7 @@ it.skipIf(!isIPv6())("fetch with IPv6 literal hostname verifies the certificate" stdout: "pipe", stderr: "pipe", }); - const [stdout, stderr, exitCode] = await Promise.all([ - proc.stdout.text(), - proc.stderr.text(), - proc.exited, - ]); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); // Asserting stdout/stderr before exitCode produces a more useful failure // message when the subprocess crashes unexpectedly. expect(stdout).toBe("native: Hello World\ncallback hostname: ::1\njs: Hello World\n"); From 840002966efa3a62f1755fce8f1368067541203b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 14 May 2026 09:37:24 +0000 Subject: [PATCH 3/3] ci: retrigger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI on f5378838 hit an unrelated flake on the debian-13 x64-asan lane (test/js/web/fetch/fetch-tcp-keepalive.test.ts: "fetch sockets have TCP keepalive enabled" + "node:http globalAgent (keepAlive: true) enables SO_KEEPALIVE", both checking SO_KEEPALIVE via /proc/net/tcp). Same tests failed on unrelated PR #54218 at the same sha snapshot, confirming flake rather than regression — this PR doesn't touch TCP keepalive code.