diff --git a/src/http/AsyncHTTP.rs b/src/http/AsyncHTTP.rs index 970c61bbe591..db0b6980c546 100644 --- a/src/http/AsyncHTTP.rs +++ b/src/http/AsyncHTTP.rs @@ -273,6 +273,15 @@ impl<'a> AsyncHTTP<'a> { .cast::>() } + /// Whether any hop of this request performs a TLS handshake: the target + /// URL directly, or the proxy connection when one is configured. + /// + /// **Not thread safe while request is in-flight** (same caveat as + /// [`HTTPClient::is_https`]). + pub fn used_tls(&self) -> bool { + self.url.is_https() || self.client.is_https() + } + /// Accessor for the global concurrent-request cap. Returned as a static /// so callers can `.load()` / `.store()` directly. #[inline] diff --git a/src/http/lib.rs b/src/http/lib.rs index 57bfb2fccb51..a2562e9d4d4b 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -1507,7 +1507,7 @@ fn write_to_socket_with_buffer_fallback( // and ProxyTunnel.rs. // ──────────────────────────────────────────────────────────────────────── -/// Maps an X509 verify code +/// Maps the `error_no` of the uSockets handshake verify error /// onto a `crate::Error` whose name is the upper-snake error tag /// (e.g. `CERT_HAS_EXPIRED`). JS-side `error.code` matches on this exact /// string, so do NOT substitute `X509_verify_cert_error_string` output here. @@ -1516,6 +1516,21 @@ fn write_to_socket_with_buffer_fallback( // this file doesn't grow a dep on a header-generated const set. pub(crate) fn get_cert_error_from_no(error_no: i32) -> crate::Error { use crate::error::CertError; + // uSockets synthesizes negative `error_no` sentinels for handshake + // failures that are not certificate problems (`X509_V_ERR_*` codes are + // all non-negative): -71/"EPROTO" for a fatal TLS protocol error + // (`ssl_dispatch_parked_reason`) and -46/"ECONNRESET" for a connection + // reset/closed before the handshake completed + // (`ssl_trigger_handshake_econnreset`), both in + // packages/bun-usockets/src/crypto/openssl.c. Report them with Node's + // codes for the same cases instead of + // UNKNOWN_CERTIFICATE_VERIFICATION_ERROR. + if error_no == -71 { + return crate::Error::Sys(bun_errno::SystemErrno::EPROTO); + } + if error_no < 0 { + return crate::Error::Sys(bun_errno::SystemErrno::ECONNRESET); + } crate::Error::Cert(match error_no { 0 => CertError::OK, // X509_V_OK 2 => CertError::UNABLE_TO_GET_ISSUER_CERT, diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 9c472c096320..041e79421258 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -1356,10 +1356,22 @@ impl FetchTasklet { BunString::static_(fail.name()) }; + // `Error::Sys(ECONNRESET)` has two producers: the mid-TLS-handshake + // reset sentinel (`get_cert_error_from_no`) and the plain-HTTP + // sendfile body path surfacing the raw errno (`SendFile::write`, + // which is `url.is_http()`-only). Only emit the TLS-specific message + // when some hop of this request actually performs a TLS handshake. + let used_tls = self.http.as_ref().is_some_and(|http_| http_.used_tls()); + let message = match fail { http::Error::ConnectionClosed => BunString::static_( "The socket connection was closed unexpectedly. For more information, pass `verbose: true` in the second argument to fetch()", ), + // Connection reset/closed before the TLS handshake completed; + // message matches Node's ConnResetException for the same case. + http::Error::Sys(bun_errno::SystemErrno::ECONNRESET) if used_tls => BunString::static_( + "Client network socket disconnected before secure TLS connection was established", + ), http::Error::FailedToOpenSocket => { BunString::static_("Was there a typo in the url or port?") } diff --git a/test/cli/install/bun-install-stalled-tls.test.ts b/test/cli/install/bun-install-stalled-tls.test.ts index c5bcf51f0e95..ec6a7fc0a40f 100644 --- a/test/cli/install/bun-install-stalled-tls.test.ts +++ b/test/cli/install/bun-install-stalled-tls.test.ts @@ -13,6 +13,7 @@ import { expect, test } from "bun:test"; import { bunEnv, bunExe, tempDir } from "harness"; import * as net from "node:net"; +import { join } from "node:path"; test("bun install times out when the registry accepts TCP but never completes the TLS handshake", async () => { // Raw TCP listener: accepts the connection, reads (and discards) the @@ -74,3 +75,69 @@ test("bun install times out when the registry accepts TCP but never completes th await new Promise(resolve => server.close(() => resolve())); } }, 60_000); + +// https://github.com/oven-sh/bun/issues/31949 +// +// A registry connection that dies during the TLS handshake involves no +// certificate at all, so `bun install` must report it as a connection error +// (ECONNRESET, the code Node and npm surface), never as +// UNKNOWN_CERTIFICATE_VERIFICATION_ERROR, which sends users hunting through +// CA stores for a network problem. A FIN (socket.destroy) reaches the SSL +// close path and its mid-handshake sentinel; a peer RST raw-closes the +// socket before the SSL layer sees it and reports ConnectionClosed instead. +test("bun install reports a connection error when the registry closes the connection during the TLS handshake", async () => { + // Raw TCP listener: accepts the connection, reads the ClientHello, and + // closes the socket without ever writing a TLS byte back. + const sockets = new Set(); + const server = net.createServer(socket => { + sockets.add(socket); + socket.on("close", () => sockets.delete(socket)); + socket.on("error", () => {}); + socket.once("data", () => socket.destroy()); + }); + const { promise: listening, resolve: onListening, reject: onListenError } = Promise.withResolvers(); + // Left attached after listen succeeds: rejecting a settled promise is a + // no-op, and it keeps a later server-level "error" from crashing the test. + server.once("error", onListenError); + server.listen(0, "127.0.0.1", onListening); + await listening; + const port = (server.address() as net.AddressInfo).port; + + try { + using dir = tempDir("install-handshake-reset", { + "package.json": JSON.stringify({ + name: "reset-repro", + version: "1.0.0", + dependencies: { "left-pad": "1.3.0" }, + }), + "bunfig.toml": `[install]\nregistry = "https://127.0.0.1:${port}/"\n`, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + env: { + ...bunEnv, + // Keep the manifest lookup off any shared cache so the request always + // hits the failing registry. + BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache"), + // One failed attempt is enough to observe the error. + BUN_CONFIG_HTTP_RETRY_COUNT: "0", + }, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + const combined = stdout + stderr; + expect(combined).not.toContain("UNKNOWN_CERTIFICATE_VERIFICATION_ERROR"); + // ECONNRESET from the handshake sentinel; ConnectionClosed when the + // platform's event loop raw-closes the socket before the SSL layer runs. + expect(combined).toMatch(/error: (ECONNRESET|ConnectionClosed) downloading package manifest left-pad/); + expect(exitCode).not.toBe(0); + } finally { + for (const s of sockets) s.destroy(); + await new Promise(resolve => server.close(() => resolve())); + } +}); diff --git a/test/js/web/fetch/fetch.tls.test.ts b/test/js/web/fetch/fetch.tls.test.ts index bc479f1b5a03..3719bc189de2 100644 --- a/test/js/web/fetch/fetch.tls.test.ts +++ b/test/js/web/fetch/fetch.tls.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "bun:test"; -import { bunEnv, bunExe, isASAN, tmpdirSync } from "harness"; +import { bunEnv, bunExe, isASAN, isWindows, tmpdirSync } from "harness"; +import net from "node:net"; import { join } from "node:path"; import tls from "node:tls"; @@ -390,6 +391,86 @@ describe.concurrent("fetch-tls", () => { }); }); + // A connection reset/closed mid-TLS-handshake involves no certificate at + // all, so it must surface as ECONNRESET (like Node), not as a certificate + // verification error. https://github.com/oven-sh/bun/issues/31949 + // + // The two variants take different paths: a FIN reaches the SSL close path + // and its mid-handshake sentinel, while a peer RST raw-closes the socket + // (see the POLL_TYPE_SOCKET error arm in packages/bun-usockets/src/loop.c) + // and surfaces as the generic connection-closed failure. Both carry the + // ECONNRESET code; only the FIN path has Node's handshake-specific message. + for (const [closeMode, closeSocket, viaHandshakeSentinel] of [ + ["resets (RST)", (socket: net.Socket) => socket.resetAndDestroy(), false], + ["closes (FIN)", (socket: net.Socket) => socket.destroy(), true], + ] as const) { + it(`fetch reports ECONNRESET when the server ${closeMode} the connection during the TLS handshake`, async () => { + // Raw TCP listener: accepts the connection, reads the ClientHello, and + // kills the socket without ever writing a TLS byte back. + const sockets = new Set(); + const server = net.createServer(socket => { + sockets.add(socket); + socket.on("close", () => sockets.delete(socket)); + socket.on("error", () => {}); + socket.once("data", () => closeSocket(socket)); + }); + const { promise: listening, resolve: onListening, reject: onListenError } = Promise.withResolvers(); + // Left attached after listen succeeds: rejecting a settled promise is a + // no-op, and it keeps a later server-level "error" from crashing the test. + server.once("error", onListenError); + server.listen(0, "127.0.0.1", onListening); + try { + await listening; + const port = (server.address() as net.AddressInfo).port; + + let err: any; + try { + await fetch(`https://127.0.0.1:${port}/`, { keepalive: false }); + expect.unreachable(); + } catch (e) { + err = e; + } + + expect(err).toBeInstanceOf(Error); + // The load-bearing invariant: a mid-handshake reset is a connection + // error, never a certificate error. + expect(err.code).toBe("ECONNRESET"); + if (viaHandshakeSentinel && !isWindows) { + // Node's exact message for this case. On Windows CI the error + // carries a different (still ECONNRESET-coded) message, so the + // exact-text assertion is POSIX-only. + expect(err.message).toBe("Client network socket disconnected before secure TLS connection was established"); + } + } finally { + for (const s of sockets) s.destroy(); + server.close(); + } + }); + } + + // A fatal TLS protocol error (the peer answers the ClientHello with + // non-TLS bytes) is the other non-certificate handshake failure: uSockets + // reports it as the -71/"EPROTO" sentinel (ssl_dispatch_parked_reason), + // and it must surface as EPROTO like Node, not as a certificate error. + it("fetch reports EPROTO when the server speaks plain HTTP during the TLS handshake", async () => { + using server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch: () => new Response("ok"), + }); + + let err: any; + try { + await fetch(`https://127.0.0.1:${server.port}/`, { keepalive: false }); + expect.unreachable(); + } catch (e) { + err = e; + } + + expect(err).toBeInstanceOf(Error); + expect(err.code).toBe("EPROTO"); + }); + it("fetch with checkServerIdentity failing should throw", async () => { await createServer(CERT_LOCALHOST_IP, async port => { try {