Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/http/AsyncHTTP.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,15 @@ impl<'a> AsyncHTTP<'a> {
.cast::<AsyncHTTP<'static>>()
}

/// 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`]).
Comment thread
robobun marked this conversation as resolved.
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]
Expand Down
17 changes: 16 additions & 1 deletion src/http/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1507,7 +1507,7 @@ fn write_to_socket_with_buffer_fallback<const IS_SSL: bool>(
// 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.
Expand All @@ -1516,6 +1516,21 @@ fn write_to_socket_with_buffer_fallback<const IS_SSL: bool>(
// 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.
Comment thread
robobun marked this conversation as resolved.
if error_no == -71 {
return crate::Error::Sys(bun_errno::SystemErrno::EPROTO);
}
if error_no < 0 {
return crate::Error::Sys(bun_errno::SystemErrno::ECONNRESET);
}
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
crate::Error::Cert(match error_no {
0 => CertError::OK, // X509_V_OK
2 => CertError::UNABLE_TO_GET_ISSUER_CERT,
Expand Down
12 changes: 12 additions & 0 deletions src/runtime/webcore/fetch/FetchTasklet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment thread
robobun marked this conversation as resolved.
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.
Comment thread
robobun marked this conversation as resolved.
http::Error::Sys(bun_errno::SystemErrno::ECONNRESET) if used_tls => BunString::static_(
"Client network socket disconnected before secure TLS connection was established",
),
Comment thread
robobun marked this conversation as resolved.
http::Error::FailedToOpenSocket => {
BunString::static_("Was there a typo in the url or port?")
}
Expand Down
67 changes: 67 additions & 0 deletions test/cli/install/bun-install-stalled-tls.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Comment thread
robobun marked this conversation as resolved.

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
Expand Down Expand Up @@ -74,3 +75,69 @@ test("bun install times out when the registry accepts TCP but never completes th
await new Promise<void>(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<net.Socket>();
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<void>();
// 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;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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<void>(resolve => server.close(() => resolve()));
}
});
83 changes: 82 additions & 1 deletion test/js/web/fetch/fetch.tls.test.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -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<net.Socket>();
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<void>();
// 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 {
Expand Down
Loading