sql: close the socket at once when postgres or mysql fail a TLS connection whose peer stopped responding

src/sql_jsc/mysql/JSMySQLConnection.rs

@@ -638,7 +638,8 @@ impl JSMySQLConnection {
             }
             S::Connected | S::Disconnected | S::Failed => {
                 let queries = this.get_queries_array();
-                this.connection_mut().clean_queue_and_close(None, queries);
+                this.connection_mut()
+                    .clean_queue_and_close(None, queries, uws::CloseCode::Normal);
             }
         }
         Ok(JSValue::UNDEFINED)
@@ -707,7 +708,8 @@ impl JSMySQLConnection {
             // `_ref` has not yet dropped, so `*p` is still live; `ParentRef`
             // yields a fresh `&Self` per access (R-2: every callee is `&self`).
             let queries = p.get_queries_array();
-            p.connection_mut().clean_queue_and_close(Some(value), queries);
+            p.connection_mut()
+                .clean_queue_and_close(Some(value), queries, uws::CloseCode::Failure);
             p.update_reference_type();
         }
         self.stop_timers();

src/sql_jsc/mysql/MySQLConnection.rs

@@ -291,15 +291,17 @@ impl MySQLConnection {
         }
     }
 
-    pub(crate) fn close(&mut self) {
-        self.socket.close(uws::CloseKind::Normal);
+    pub(crate) fn close(&mut self, code: uws::CloseCode) {
+        self.socket.close(code);
         self.write_buffer = OffsetByteList::default();
     }
 
+    /// `fail_with_js_value` passes `Failure`, the one code a TLS socket never defers (see `CloseCode`).
     pub(crate) fn clean_queue_and_close(
         &mut self,
         js_reason: Option<JSValue>,
         js_queries_array: JSValue,
+        code: uws::CloseCode,
     ) {
         // cleanup requests
         self.queue.clean(
@@ -311,7 +313,7 @@ impl MySQLConnection {
             },
         );
 
-        self.close();
+        self.close(code);
     }
 
     pub(crate) fn cleanup(&mut self) {

src/sql_jsc/postgres/PostgresSQLConnection.rs

@@ -731,7 +731,7 @@ impl PostgresSQLConnection {
                 &[js_error, queries],
             );
         }
-        self.ref_and_close(Some(value));
+        self.ref_and_close(Some(value), uws::CloseCode::Failure);
         // SAFETY: `self` is a live Box-allocated connection; this releases one ref.
         unsafe { Self::deref(self.as_ctx_ptr()) };
         self.update_has_pending_activity();
@@ -1510,14 +1510,15 @@ impl PostgresSQLConnection {
         }
     }
 
-    fn ref_and_close(&self, js_reason: Option<JSValue>) {
+    /// `fail_with_js_value` passes `Failure`, the one code a TLS socket never defers (see `CloseCode`).
+    fn ref_and_close(&self, js_reason: Option<JSValue>, code: uws::CloseCode) {
         // refAndClose is always called when we wanna to disconnect or when we are closed
 
         if !self.socket.get().is_closed() {
             // event loop need to be alive to close the socket
             self.poll_ref.with_mut(|r| r.ref_(self.vm_ctx()));
             // will unref on socket close
-            self.socket.get().close(uws::CloseKind::Normal);
+            self.socket.get().close(code);
         }
 
         // cleanup requests
@@ -1529,7 +1530,7 @@ impl PostgresSQLConnection {
         self.unregister_auto_flusher();
         if self.status.get() == Status::Connected {
             self.status.set(Status::Disconnected);
-            self.ref_and_close(None);
+            self.ref_and_close(None, uws::CloseCode::Normal);
         }
     }
 

test/js/sql/sql-close-pending-connection.test.ts

@@ -18,6 +18,8 @@
 
 import { SQL } from "bun";
 import { expect, test } from "bun:test";
+import { bunEnv, bunExe, tls } from "harness";
+import path from "node:path";
 import { neverAnsweringServer } from "./wire-frames";
 
 const drivers = [
@@ -101,3 +103,50 @@ test("pool scans tolerate unassigned connection slots during pool start", async
     server.close();
   }
 });
+
+// When the client itself gives up on a connection (connection timeout, protocol
+// violation, forced close while still connecting, ...) it has to close the
+// socket without waiting for the peer. Over TLS the graceful close both drivers
+// used to issue from fail() sends a close_notify and keeps the socket open until
+// the peer answers it, which a peer that has stopped responding never does: the
+// failure was reported, but the socket stayed open and, with it, the process
+// stayed alive. The fixture runs both drivers against mocks that complete the
+// TLS handshake and then stop responding, and has to exit by itself once each
+// client has reported the failure and each mock has seen its connection close.
+//
+// Test timeout: the fixture is a second debug build doing four TLS handshakes,
+// and without the fix it only ends when it runs into the spawn timeout.
+test("a TLS connection the client gives up on is closed at once even though the peer stopped responding", async () => {
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), path.join(import.meta.dir, "sql-tls-peer-stopped-responding.fixture.ts")],
+    env: { ...bunEnv, MOCK_TLS_KEY: tls.key, MOCK_TLS_CERT: tls.cert },
+    stdout: "pipe",
+    stderr: "pipe",
+    // Without the fix the fixture never exits; this turns that into a failure
+    // that shows which lines never got printed.
+    timeout: 20_000,
+  });
+  const [stdout, stderr] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect({
+    stdout: stdout.trim().split(/\r?\n/).sort(),
+    exitCode: proc.exitCode,
+    signalCode: proc.signalCode,
+    stderr,
+  }).toEqual({
+    stdout: [
+      "mysql close: rejected with ERR_MYSQL_CONNECTION_CLOSED",
+      "mysql close: the mock saw the connection close",
+      "mysql unexpected: rejected with ERR_MYSQL_UNEXPECTED_PACKET",
+      "mysql unexpected: the mock saw the connection close",
+      "postgres close: rejected with ERR_POSTGRES_CONNECTION_CLOSED",
+      "postgres close: the mock saw the connection close",
+      "postgres unexpected: rejected with ERR_POSTGRES_UNEXPECTED_MESSAGE",
+      "postgres unexpected: the mock saw the connection close",
+    ],
+    exitCode: 0,
+    signalCode: null,
+    // not asserted; included so that whatever the fixture died of shows up
+    // in the diff
+    stderr: expect.any(String),
+  });
+}, 30_000);

test/js/sql/sql-tls-peer-stopped-responding.fixture.ts

@@ -0,0 +1,127 @@
+// Fixture for the "TLS peer stopped responding" test in
+// sql-close-pending-connection.test.ts. Every driver x trigger combination
+// below runs concurrently against its own mock. Each prints one line when the
+// client reports the failure and one more when the mock's TCP socket sees the
+// connection go away; the process then has to exit by itself, since a
+// connection the client has given up on must not keep it alive.
+//
+// Each mock completes the STARTTLS upgrade, takes the client's startup message
+// and then stops responding for good (see startTlsServerSide), so the client's
+// close_notify is never answered. The client then gives the connection up
+// either because the pool is force-closed while the connection is still
+// waiting for the startup reply ("close") or because what the mock answered
+// was a protocol violation ("unexpected"). Both triggers happen strictly after
+// the mock has stopped responding, and both go through the driver's fail(),
+// the same path a connection or idle timeout takes; a timeout itself is not
+// used as a trigger because on a slow debug build it can fire before the TLS
+// handshake is even done, at which point there is nothing to wait for.
+//
+// The pools in the "unexpected" scenarios are deliberately not closed: on
+// mysql a close() issued after the failure closes the socket a second time,
+// which happens to complete a graceful TLS close that is still waiting for
+// the peer, and would hide exactly what this fixture observes.
+
+import { SQL } from "bun";
+import type net from "node:net";
+import {
+  listeningServer,
+  MYSQL_CLIENT_SSL,
+  MYSQL_DEFAULT_CAPABILITIES,
+  mysqlHandshakeV10,
+  mysqlRawPacket,
+  pgReadyForQuery,
+  pgSSLRequest,
+  pgSSLResponse,
+  startTlsServerSide,
+} from "./wire-frames";
+
+type Driver = "postgres" | "mysql";
+type Trigger = "close" | "unexpected";
+
+// The harness certificate for 127.0.0.1, handed over by the test: importing
+// "harness" here would cost this debug-build subprocess several seconds.
+const { MOCK_TLS_KEY: key, MOCK_TLS_CERT: cert } = process.env;
+if (!key || !cert) throw new Error("MOCK_TLS_KEY and MOCK_TLS_CERT must be set by the test that spawns this fixture");
+const tlsCredentials = { key, cert };
+
+// Answers to the startup message that the client rejects: a ReadyForQuery before
+// any Authentication message, resp. an auth reply whose header byte no auth
+// packet uses.
+const unexpectedReply: Record<Driver, Buffer> = {
+  postgres: pgReadyForQuery(),
+  mysql: mysqlRawPacket(3, Buffer.from([0x42])),
+};
+
+// Length of the plaintext the client sends before TLS once `buffered` holds all
+// of it: postgres sends the 8-byte SSLRequest, mysql one packet (its SSLRequest)
+// in reply to our greeting. undefined while more bytes are needed.
+function preludeLength(driver: Driver, buffered: Buffer): number | undefined {
+  if (driver === "postgres") {
+    return buffered.length >= pgSSLRequest().length ? pgSSLRequest().length : undefined;
+  }
+  if (buffered.length < 4) return undefined;
+  const length = 4 + (buffered[0] | (buffered[1] << 8) | (buffered[2] << 16));
+  return buffered.length >= length ? length : undefined;
+}
+
+/** Resolves once the mock has taken the client's startup message and stopped responding. */
+function mockThatStopsResponding(driver: Driver, trigger: Trigger, raw: net.Socket): Promise<void> {
+  const stoppedResponding = Promise.withResolvers<void>();
+  // Once the client closes for real, the RST it sends surfaces here.
+  raw.on("error", () => {});
+  if (driver === "mysql") {
+    raw.write(mysqlHandshakeV10({ capabilities: MYSQL_DEFAULT_CAPABILITIES | MYSQL_CLIENT_SSL }));
+  }
+  let buffered = Buffer.alloc(0);
+  const onPlaintext = (chunk: Buffer) => {
+    buffered = Buffer.concat([buffered, chunk]);
+    const length = preludeLength(driver, buffered);
+    if (length === undefined) return;
+    raw.removeListener("data", onPlaintext);
+    if (driver === "postgres") raw.write(pgSSLResponse("S"));
+    const peer = startTlsServerSide(raw, buffered.subarray(length), tlsCredentials);
+    // The first decrypted bytes are the StartupMessage / HandshakeResponse.
+    peer.secure.once("data", () => {
+      peer.stopReading();
+      if (trigger === "unexpected") peer.secure.write(unexpectedReply[driver]);
+      stoppedResponding.resolve();
+    });
+  };
+  raw.on("data", onPlaintext);
+  return stoppedResponding.promise;
+}
+
+async function scenario(driver: Driver, trigger: Trigger) {
+  const mockStoppedResponding = Promise.withResolvers<void>();
+  const mockSawClose = Promise.withResolvers<void>();
+  const { port, server } = await listeningServer(raw => {
+    raw.on("close", () => mockSawClose.resolve());
+    mockThatStopsResponding(driver, trigger, raw).then(mockStoppedResponding.resolve);
+  });
+  const sql = new SQL({
+    url: `${driver}://user:password@127.0.0.1:${port}/db`,
+    max: 1,
+    tls: { ca: tlsCredentials.cert },
+  });
+  const query = sql`select 1`.then(
+    () => "resolved",
+    (err: any) => `rejected with ${err?.code ?? err}`,
+  );
+  if (trigger === "close") {
+    await mockStoppedResponding.promise;
+    // The string form closes at once even though a query is waiting; a numeric
+    // 0 currently waits for it (https://github.com/oven-sh/bun/issues/32038).
+    await sql.close({ timeout: "0" });
+  }
+  console.log(`${driver} ${trigger}: ${await query}`);
+  await mockSawClose.promise;
+  console.log(`${driver} ${trigger}: the mock saw the connection close`);
+  server.close();
+}
+
+await Promise.all([
+  scenario("postgres", "close"),
+  scenario("postgres", "unexpected"),
+  scenario("mysql", "close"),
+  scenario("mysql", "unexpected"),
+]);

test/js/sql/wire-frames.ts

@@ -5,6 +5,8 @@
 // Buffer.alloc / writeInt32BE sequences.
 
 import net from "node:net";
+import { Duplex } from "node:stream";
+import tls from "node:tls";
 
 // ---------------------------------------------------------------------------
 // Server helpers shared by every fault-injection test.
@@ -43,6 +45,46 @@ export async function neverAnsweringServer(): Promise<{ port: number; server: ne
   return { port, server, accepted: first.promise };
 }
 
+/**
+ * Server side of a STARTTLS-style upgrade (Postgres once the SSLRequest has been
+ * answered with 'S', MySQL once the client's SSLRequest packet has arrived) for
+ * a mock that is going to stop responding later on. The TLS engine runs over a
+ * Duplex that this helper feeds with the bytes read from `raw`, so after
+ * `stopReading()` everything the client sends is dropped unread while the TCP
+ * connection stays open: in particular the client's close_notify is never
+ * answered, which is what a peer that has hung looks like. (A TLS socket
+ * wrapping the connection itself would answer it natively, and pause() does
+ * not prevent that.) `leftover` is whatever arrived behind the plaintext
+ * prelude, i.e. the start of the client's ClientHello.
+ */
+export function startTlsServerSide(
+  raw: net.Socket,
+  leftover: Buffer,
+  credentials: { key: string; cert: string },
+): { secure: tls.TLSSocket; stopReading(): void } {
+  let reading = true;
+  const bridge = new Duplex({
+    read() {},
+    // Errors on `raw` are the caller's to observe; the bridge itself never fails.
+    write(chunk, _encoding, callback) {
+      raw.write(chunk);
+      callback();
+    },
+  });
+  raw.on("data", chunk => {
+    if (reading) bridge.push(chunk);
+  });
+  if (leftover.length) bridge.push(leftover);
+  const secure = new tls.TLSSocket(bridge, { isServer: true, ...credentials });
+  secure.on("error", () => {});
+  return {
+    secure,
+    stopReading() {
+      reading = false;
+    },
+  };
+}
+
 // ---------------------------------------------------------------------------
 // PostgreSQL frontend/backend protocol — https://www.postgresql.org/docs/current/protocol-message-formats.html
 // ---------------------------------------------------------------------------