Skip to content
Merged
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
6 changes: 3 additions & 3 deletions src/runtime/socket/UpgradedDuplex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,18 +245,18 @@ impl UpgradedDuplex {
let buffer = match bun_jsc::array_buffer::BinaryType::Buffer.to_js(data, &global) {
Ok(b) => b,
Err(err) => {
(self.handlers.on_error)(self.handlers.ctx, global.take_exception(err));
(self.handlers.on_error)(self.handlers.ctx, global.take_error(err));
return;
}
};
buffer.ensure_still_alive();

if let Err(err) = write_or_end.call(&global, duplex, &[buffer]) {
(self.handlers.on_error)(self.handlers.ctx, global.take_exception(err));
(self.handlers.on_error)(self.handlers.ctx, global.take_error(err));
}
} else {
if let Err(err) = write_or_end.call(&global, duplex, &[JSValue::NULL]) {
(self.handlers.on_error)(self.handlers.ctx, global.take_exception(err));
(self.handlers.on_error)(self.handlers.ctx, global.take_error(err));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/runtime/socket/socket_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ extern "C" fn select_alpn_callback(
// The selection's ToString threw (a Symbol or a throwing
// toString): hand it to `error` like the callback's own
// throw, then refuse the protocol.
let err_value = global.take_exception(err);
let err_value = global.take_error(err);
crate::dispatch::fold(
handlers.call_error_handler(this_value, &[this_value, err_value]),
);
Expand Down Expand Up @@ -2171,7 +2171,7 @@ impl<const SSL: bool> NewSocket<SSL> {
let output_value = match handlers.binary_type.get().to_js(data, &global) {
Ok(v) => v,
Err(err) => {
return this.handle_error(global.take_exception(err));
return this.handle_error(global.take_error(err));
Comment thread
claude[bot] marked this conversation as resolved.
}
};

Expand Down
58 changes: 58 additions & 0 deletions test/js/bun/net/socket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1547,6 +1547,64 @@ describe.concurrent("TLS server: write() to the accepted socket from inside its
}
});

it("alpnCallback: a selection whose ToString throws surfaces the thrown Error, not an engine-internal cell", async () => {
// The thrown value must reach the `error` handler as a plain Error, not the
// JSC::Exception wrapper cell: `Object.prototype.toString.call` on the cell
// aborts the process and `instanceof Error` on it is false. The crash fires
// on the first property load of the value, so probe it in a subprocess.
using dir = tempDir("alpn-tostring-throw", {
"fixture.cjs": `
const tlsMod = require("node:tls");
const server = Bun.listen({
hostname: "127.0.0.1",
port: 0,
tls: { key: ${JSON.stringify(tls.key)}, cert: ${JSON.stringify(tls.cert)} },
socket: {
alpnCallback() {
// Not a boolean, and ToString on it throws a TypeError.
return Symbol("alpn-choice");
},
data() {},
error(_socket, v) {
const tag = Object.prototype.toString.call(v);
console.log("error:" + (v instanceof Error) + ":" + tag + ":" + (v && v.name));
},
close() {},
},
});
const client = tlsMod.connect({
port: server.port,
host: "127.0.0.1",
ca: ${JSON.stringify(tls.cert)},
servername: "localhost",
ALPNProtocols: ["x/1"],
});
// The server refuses the connection with a fatal no_application_protocol
// alert after its error handler ran; either client event ends the test.
client.on("error", () => server.stop(true));
client.on("secureConnect", () => {
client.end();
server.stop(true);
});
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "fixture.cjs"],
env: { ...bunEnv, ASAN_OPTIONS: "symbolize=0:abort_on_error=1:allow_user_segv_handler=1" },
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect({ lines: stdout.trim().split(/\r?\n/), stderr, exitCode }).toEqual({
lines: ["error:true:[object Error]:TypeError"],
stderr: "",
exitCode: 0,
});
});

// Bun.connect() on a Windows named pipe takes a dedicated early branch in
// Listener.connectInner that heap-allocates a standalone Handlers block. That
// block's `.mode` must be `.client` so Handlers.markInactive() destroys it on
Expand Down
77 changes: 77 additions & 0 deletions test/js/node/tls/node-tls-duplex-write-throw-error-value.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// When a TLS connection is wrapped around a user-supplied Duplex transport and
// that Duplex's `write()` throws synchronously, the thrown value must reach the
// TLSSocket's `'error'` listener as a plain Error, not the engine-internal
// JSC::Exception wrapper cell. The wrapper cell has no prototype:
// `Object.prototype.toString.call` on it aborts the process, property loads on
// it trip a debug assertion inside `synthesizePrototype`, and the user's own
// error message is lost.

import { expect, test } from "bun:test";
import { bunEnv, bunExe, tls as certs, tempDir } from "harness";

test("tls over Duplex: throwing transport write surfaces the thrown Error, not an engine-internal cell", async () => {
using dir = tempDir("tls-duplex-throw", {
"fixture.cjs": `
const tls = require("node:tls");
const { Duplex } = require("node:stream");
const key = ${JSON.stringify(certs.key)};
const cert = ${JSON.stringify(certs.cert)};

let armed = false;
let fired = 0;
class Wire extends Duplex {
_write(chunk, enc, cb) {
if (armed && this.hook && fired++ === 0) throw new Error("boom-in-transport-_write");
this.peer.push(Buffer.from(chunk));
cb();
}
_read() {}
}
const c = new Wire();
const s = new Wire();
c.peer = s;
s.peer = c;
c.hook = true;

process.on("uncaughtException", e => console.log("uncaught:" + (e && e.message)));

const srv = new tls.TLSSocket(s, { isServer: true, key, cert });
srv.on("error", () => {});
srv.resume();

const cli = tls.connect({ socket: c, rejectUnauthorized: false });
cli.on("error", v => {
// The crash under test fires on the first property load / toString of
// the value: in debug builds even node:events' own \`err.code\` read
// trips a JSC assertion before this listener runs at all.
const tag = Object.prototype.toString.call(v);
console.log("error:" + (v instanceof Error) + ":" + tag + ":" + (v && v.message));
});
cli.on("secureConnect", () => {
armed = true;
cli.write(Buffer.alloc(64 * 1024, 0x41));
setImmediate(() => setImmediate(() => process.exit(0)));
});
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "fixture.cjs"],
env: { ...bunEnv, ASAN_OPTIONS: "symbolize=0:abort_on_error=1:allow_user_segv_handler=1" },
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

// The server side sees the truncated handshake bytes and may report its own
// protocol error as an uncaughtException; the assertion below only cares
// that the client's `'error'` listener received the user's thrown Error.
const lines = stdout.trim().split(/\r?\n/);
expect({ lines, stderr, exitCode }).toEqual({
lines: expect.arrayContaining(["error:true:[object Error]:boom-in-transport-_write"]),
stderr: "",
exitCode: 0,
});
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Loading