Skip to content
Open
Show file tree
Hide file tree
Changes from 5 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
64 changes: 64 additions & 0 deletions src/http_jsc/websocket_client/CppWebSocket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,28 @@ bun_opaque::opaque_ffi! {
pub struct CppWebSocket;
}

/// A single handshake response header borrowed from the parse buffer, matching
/// the C++ `WebCore::WebSocket::HandshakeRawHeader` layout. Only valid for the
/// duration of the [`CppWebSocket::did_receive_handshake_response`] call.
#[repr(C)]
pub(crate) struct HandshakeRawHeader {
name_ptr: *const u8,
name_len: usize,
value_ptr: *const u8,
value_len: usize,
}

impl HandshakeRawHeader {
pub(crate) fn new(name: &[u8], value: &[u8]) -> Self {
Self {
name_ptr: name.as_ptr(),
name_len: name.len(),
value_ptr: value.as_ptr(),
value_len: value.len(),
}
}
}

// FFI surface for `WebCore::WebSocket` (src/jsc/bindings/webcore/WebSocket.cpp).
// Kept private to this module — the safe wrappers below are the only callers.
//
Expand Down Expand Up @@ -61,6 +83,16 @@ unsafe extern "C" {
safe fn WebSocket__incrementPendingActivity(websocket_context: &CppWebSocket);
safe fn WebSocket__decrementPendingActivity(websocket_context: &CppWebSocket);
fn WebSocket__setProtocol(websocket_context: &CppWebSocket, protocol: *mut BunString);
fn WebSocket__didReceiveHandshakeResponse(
websocket_context: &CppWebSocket,
status_code: u16,
status_message: *const u8,
status_message_len: usize,
headers: *const HandshakeRawHeader,
headers_len: usize,
body: *const u8,
body_len: usize,
);
}

// PORT NOTE: receivers are `&self` (not `&mut self`) because `CppWebSocket` is
Expand Down Expand Up @@ -173,6 +205,38 @@ impl CppWebSocket {
};
event_loop.exit();
}

/// Forward the parsed HTTP handshake response to C++ so the `ws` shim can
/// emit `upgrade` / `unexpected-response`. The C++ side skips all work when
/// no `handshake` listener is registered, so the common browser-style path
/// stays zero-cost. All slices must outlive the call (they borrow the parse
/// buffer, which the caller keeps alive).
pub(crate) fn did_receive_handshake_response(
&self,
status_code: u16,
status_message: &[u8],
headers: &[HandshakeRawHeader],
body: &[u8],
) {
// SAFETY: VirtualMachine::get() returns the live current-thread VM;
// event_loop() yields its raw event-loop pointer (live for VM lifetime).
let event_loop = VirtualMachine::get().event_loop_mut();
event_loop.enter();
// SAFETY: self is a valid C++ WebCore::WebSocket; all slices outlive the call.
unsafe {
WebSocket__didReceiveHandshakeResponse(
self,
status_code,
status_message.as_ptr(),
status_message.len(),
headers.as_ptr(),
headers.len(),
body.as_ptr(),
body.len(),
)
};
event_loop.exit();
}
}

impl CppWebSocket {
Expand Down
77 changes: 64 additions & 13 deletions src/http_jsc/websocket_client/WebSocketUpgradeClient.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use bun_picohttp as picohttp;
use bun_ptr::ThisPtr;
use bun_uws::{self as uws, SocketHandler, SocketKind, SslCtx};

use super::cpp_websocket::CppWebSocket;
use super::cpp_websocket::{CppWebSocket, HandshakeRawHeader};
use super::websocket_deflate as WebSocketDeflate;
use super::websocket_proxy::WebSocketProxy;
use super::websocket_proxy_tunnel::WebSocketProxyTunnel;
Expand Down Expand Up @@ -1572,17 +1572,54 @@ impl<const SSL: bool> HTTPClient<SSL> {
return;
}

// Ownership transfer: `overflow` is HANDED OFF across FFI —
// Forward the parsed 101 handshake response to the C++ WebSocket so the
// `ws` shim can emit `upgrade` (before `open`). The C++ side is a no-op
// unless a `handshake` listener is registered, so the browser-style
// `new WebSocket()` path pays nothing. This dispatches into JS *before*
// `did_connect` (while the socket is still CONNECTING, matching node's
// `ws`) so a `handshake`/`upgrade` handler that synchronously closes the
// socket is handled by the existing `!tcp.is_closed() && has_ws`
// re-check below: `cancel()` takes `outgoing_websocket`, so `has_ws`
// becomes false and `did_connect` is skipped.
//
// The header name/value slices borrow the parse buffer (`body` in
// `handle_data`), which is still alive here — `clear_data()` (which
// frees it) runs further below. `remain_buf` is passed as the response
// body; for a 101 it's the first WebSocket frame, so the shim drops it.
//
// SAFETY: short-lived read of `outgoing_websocket`.
if let Some(ws) = unsafe { (*this).outgoing_websocket } {
let mut raw_headers: Vec<HandshakeRawHeader> =
Vec::with_capacity(response.headers.list.len());
for header in response.headers.list {
raw_headers.push(HandshakeRawHeader::new(header.name(), header.value()));
}
let status_code = u16::try_from(response.status_code).unwrap_or(0);
CppWebSocket::opaque_ref(ws).did_receive_handshake_response(
status_code,
response.status,
&raw_headers,
remain_buf,
);
}
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.

// Owned copy of the bytes that trailed the 101 header block. It is
// HANDED OFF across FFI in the success arms below —
// `WebSocket__didConnect` → `Bun__WebSocketClient__init`/`_initWithTunnel`
// adopts the raw `(ptr, len)` into an `InitialDataHandler` queued as a
// microtask, which reclaims it via `Box::<[u8]>::from_raw` when the
// microtask runs. Allocate as `Box<[u8]>` and `heap::alloc` it so the
// alloc/free pair through the SAME Rust global allocator (mimalloc).
// Do NOT keep a `Vec`/`Box` binding past the FFI call — it would drop
// at scope exit and leave the queued microtask with a dangling pointer
// (UAF on read in `handle_data`, then double-free on drop).
// microtask runs. Allocate as `Box<[u8]>` so the alloc/free pair goes
// through the SAME Rust global allocator (mimalloc).
//
// Keep it as an owned `Box` (NOT leaked yet) until a success arm is
// reached. A `handshake`/`upgrade` handler can synchronously close the
// socket (see the note above), which makes the `!tcp.is_closed() &&
// has_ws` re-check below fail and route into an else-arm that never
// calls `did_connect`; leaking the buffer up-front would then orphan it
// (no consumer to reclaim it). By only `heap::into_raw`-ing it at the
// moment of handoff, the else-arms drop the `Box` normally instead.
let overflow_len = remain_buf.len();
let overflow_ptr: *mut u8 = if overflow_len > 0 {
let mut overflow_box: Option<Box<[u8]>> = if overflow_len > 0 {
let mut v: Vec<u8> = Vec::new();
if v.try_reserve_exact(overflow_len).is_err() {
// Spec .zig:1020 — OOM here terminates with `invalid_response`
Expand All @@ -1592,11 +1629,18 @@ impl<const SSL: bool> HTTPClient<SSL> {
return;
}
v.extend_from_slice(remain_buf);
// Leak across the FFI boundary; `InitialDataHandler` reconstructs
// the `Box<[u8]>` and drops it after delivery.
bun_core::heap::into_raw(v.into_boxed_slice()).cast::<u8>()
Some(v.into_boxed_slice())
} else {
core::ptr::null_mut()
None
};
// Leak the owned buffer across the FFI boundary right before handoff;
// `InitialDataHandler` reconstructs the `Box<[u8]>` and drops it after
// delivery. Returns a null thin pointer when there is no overflow.
let take_overflow_ptr = |b: &mut Option<Box<[u8]>>| -> *mut u8 {
match b.take() {
Some(boxed) => bun_core::heap::into_raw(boxed).cast::<u8>(),
None => core::ptr::null_mut(),
}
};

// Check if we're using a proxy tunnel (wss:// through HTTP proxy)
Expand Down Expand Up @@ -1624,7 +1668,10 @@ impl<const SSL: bool> HTTPClient<SSL> {
// SAFETY: short-lived `&mut` for the field take.
let ws = unsafe { (*this).outgoing_websocket.take().unwrap() };

// Create the WebSocket client with the tunnel
// Create the WebSocket client with the tunnel. Hand off the
// overflow buffer (leak it across FFI) only now that we're
// committed to delivering it.
let overflow_ptr = take_overflow_ptr(&mut overflow_box);
// SAFETY: live C++ back-reference.
unsafe {
(*ws).did_connect_with_tunnel(
Expand Down Expand Up @@ -1683,6 +1730,10 @@ impl<const SSL: bool> HTTPClient<SSL> {
// SAFETY: short-lived `&mut` for the field detach; ends before the FFI call below.
unsafe { (*this).tcp.detach() };
if let uws::InternalSocket::Connected(native_socket) = socket.socket {
// Hand off the overflow buffer (leak it across FFI) only now
// that `did_connect` will consume it. If the socket is not
// Connected (else-arm), `overflow_box` is dropped instead.
let overflow_ptr = take_overflow_ptr(&mut overflow_box);
// SAFETY: live C++ back-reference.
unsafe {
(*ws).did_connect(
Expand Down
3 changes: 3 additions & 0 deletions src/js/builtins/BunBuiltinNames.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ using namespace JSC;
macro(pullAlgorithm) \
macro(pulling) \
macro(queue) \
macro(rawHeaders) \
macro(read) \
macro(readIntoRequests) \
macro(readRequests) \
Expand Down Expand Up @@ -193,6 +194,8 @@ using namespace JSC;
macro(started) \
macro(state) \
macro(status) \
macro(statusCode) \
macro(statusMessage) \
macro(statusText) \
macro(storedError) \
macro(strategy) \
Expand Down
67 changes: 66 additions & 1 deletion src/js/thirdparty/ws.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,42 @@
console.warn("[bun] Warning:", message);
}

// ws emits `upgrade` / `unexpected-response` with an `http.IncomingMessage` for
// the handshake response. We bypass node:http, so build a minimal
// IncomingMessage-shaped Readable from the status + rawHeaders the native
// WebSocket parsed. Lazily pull in node:stream the first time it's needed.
let lazyReadable;
function makeHandshakeResponse(statusCode, statusMessage, rawHeaders, body) {
lazyReadable ??= require("node:stream").Readable;
const res = new lazyReadable({ read() {} });
// Match node's http.IncomingMessage.headers: a plain object that inherits
// from Object.prototype (so `res.headers.hasOwnProperty(...)` works). Use
// Object.hasOwn for the dup-check so a header literally named "constructor"
// is not confused with Object.prototype.constructor.
const headers = (res.headers = {});
res.rawHeaders = rawHeaders;
for (let i = 0; i < rawHeaders.length; i += 2) {
const lower = rawHeaders[i].toLowerCase();
const value = rawHeaders[i + 1];
const seen = Object.hasOwn(headers, lower);
if (lower === "set-cookie") {
if (!seen) headers[lower] = [value];
else headers[lower].push(value);
} else {
headers[lower] = !seen ? value : headers[lower] + ", " + value;
}
Comment thread
robobun marked this conversation as resolved.
}
res.statusCode = statusCode;
res.statusMessage = statusMessage;
res.httpVersion = "1.1";
res.httpVersionMajor = 1;
res.httpVersionMinor = 1;
res.socket = res.connection = null;
if (body && body.length) res.push(body);
res.push(null);
return res;
}

// TODO: add private method on WebSocket to avoid these allocations
function normalizeData(data, opts) {
const isBinary = opts?.binary;
Expand Down Expand Up @@ -124,6 +160,7 @@
#paused = false;
#fragments = false;
#binaryType = "nodebuffer";
#handshakeListenerRegistered = false;
// Bitset to track whether event handlers are set.
#eventId = 0;

Expand Down Expand Up @@ -266,14 +303,42 @@
}
let ws = (this.#ws = new WebSocket(url, wsOptions));
ws.binaryType = "nodebuffer";
// The native 'handshake' listener is registered lazily (from #onOrOnce
// when the user subscribes to 'upgrade') so callers that only listen to
// 'open'/'message'/'close' never exercise the native handshake-dispatch
// path.
Comment thread
robobun marked this conversation as resolved.
Outdated

return ws;
}

#ensureHandshakeListener() {
if (this.#handshakeListenerRegistered) return;
this.#handshakeListenerRegistered = true;
this.#ws.addEventListener("handshake", event => this.#onHandshake(event.data), onceObject);
}

#onHandshake(data) {
const { statusCode, statusMessage, rawHeaders } = data;
// The native client only forwards the successful 101 handshake here; it
// fails the connection on any other status before reaching this point. On a
// 101, bytes after the header block are the first WebSocket frame (not an
// HTTP body) and the native client forwards them to the protocol reader on
// connect, so don't include a body in the IncomingMessage.
const res = makeHandshakeResponse(statusCode, statusMessage, rawHeaders, null);
// ws emits `upgrade` with `(response)`, right before `open`.
this.emit("upgrade", res);
}

#onOrOnce(event, listener, once) {
if (event === "unexpected-response" || event === "upgrade" || event === "redirect") {
if (event === "unexpected-response" || event === "redirect") {
emitWarning(event, "ws.WebSocket '" + event + "' event is not implemented in bun");
}
if (event === "upgrade") {
// Lazily wire the native handshake listener; `upgrade` is emitted from
// #onHandshake.

Check warning on line 338 in src/js/thirdparty/ws.js

View check run for this annotation

Claude / Claude Code Review

addListener/prependListener('upgrade', ...) bypasses #ensureHandshakeListener

🟡 Nit (pre-existing pattern): `#ensureHandshakeListener()` is only reached from `#onOrOnce`, which only intercepts `on()`/`once()` — but Bun's `EventEmitter` defines `addListener`/`prependListener`/`prependOnceListener` as their own prototype methods (events.ts:241/266/326), so `ws.addListener('upgrade', cb)` registers the EE listener but never wires the native `handshake` listener and `upgrade` silently never fires (whereas it does under Node + npm ws). This is the same shim limitation that alr
Comment thread
robobun marked this conversation as resolved.
this.#ensureHandshakeListener();
return once ? super.once(event, listener) : super.on(event, listener);
}
const mask = 1 << eventIds[event];
const hasPersistentListener = mask && (this.#eventId & mask) === mask;
// Add a native listener if:
Expand Down
21 changes: 11 additions & 10 deletions src/jsc/bindings/webcore/EventNames.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,17 @@

namespace WebCore {

#define DOM_EVENT_NAMES_FOR_EACH(macro) \
macro(error) \
macro(abort) \
macro(close) \
macro(open) \
macro(rename) \
macro(message) \
macro(change) \
macro(messageerror) \
macro(resourcetimingbufferfull)
#define DOM_EVENT_NAMES_FOR_EACH(macro) \
macro(error) \
macro(abort) \
macro(close) \
macro(open) \
macro(rename) \
macro(message) \
macro(change) \
macro(messageerror) \
macro(handshake) \
macro(resourcetimingbufferfull)

struct EventNames {
WTF_MAKE_NONCOPYABLE(EventNames);
Expand Down
Loading
Loading