WebSocket client: support ws+unix:// and wss+unix://#29203
WebSocket client: support ws+unix:// and wss+unix://#29203Jarred-Sumner merged 5 commits intomainfrom
Conversation
The WebSocket client can now connect over Unix domain sockets using ws+unix:///path/to/sock[:/request/path] (plaintext) or wss+unix:// (TLS). The URL format matches the npm 'ws' package: the pathname is split on the first ':' into a socket path and an HTTP request path, and the Host header defaults to 'localhost'. The upgrade client reuses the existing uSockets unix-connect path (us_socket_context_connect_unix) on the same per-SSL socket context, so no new context is required. wss+unix:// selects the TLS socket context and runs the normal TLS handshake over the domain socket.
|
Updated 12:01 AM PT - Apr 12th, 2026
❌ @robobun, your commit 95458e5 has 1 failures in 🧪 To try this PR locally: bunx bun-pr 29203That installs a local version of the PR into your bun-29203 --bun |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds ws+unix/wss+unix WebSocket support: extends C-ABI connect signatures to accept a unix socket path, updates URL parsing and connection flow to split socket vs HTTP resource, disables proxying for unix sockets, implements Unix-socket connect in Zig client, and adds comprehensive unix-socket tests. Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/http/websocket_client/WebSocketUpgradeClient.zig`:
- Around line 331-337: The code sets TLS SNI/verification from host_slice
(URL/default) which ignores an explicit Host header override and breaks
wss+unix:// verification; change the logic that assigns client.hostname (in
WebSocketUpgradeClient.zig around host_slice) to use the effective host: prefer
the request's Host header (the override) if present, otherwise fall back to the
URL hostname, then only set client.hostname when that effective host is
non-empty and not an IP (same isIPAddress check); ensure you reference and
derive the effective host (hostname orelse url.hostname) before the existing
client.hostname assignment so TLS verification uses the override when supplied.
In `@test/js/web/websocket/websocket-unix.test.ts`:
- Around line 207-210: Remove the fragile stderr assertion in the test: delete
the line using expect(stderr).not.toContain("error") and rely on the existing
checks of stdout.trim() === "pong:from-child" and exitCode === 0 (the variables
proc, stdout, stderr, exitCode are in the surrounding code); only add stderr
assertions if you need to assert a specific expected stderr message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 235d7cf5-d8bf-41e6-ab5f-ca9735d3feeb
📒 Files selected for processing (4)
src/bun.js/bindings/headers.hsrc/bun.js/bindings/webcore/WebSocket.cppsrc/http/websocket_client/WebSocketUpgradeClient.zigtest/js/web/websocket/websocket-unix.test.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/js/web/websocket/websocket-unix.test.ts (1)
207-210:⚠️ Potential issue | 🟡 MinorRemove the fragile generic stderr check on Line 209.
expect(stderr).not.toContain("error")is noisy/flaky in CI (e.g., sanitizer/runtime diagnostics).stdout.trim()andexitCodealready validate the round-trip outcome.Proposed fix
- const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); expect(stdout.trim()).toBe("pong:from-child"); - expect(stderr).not.toContain("error"); expect(exitCode).toBe(0);As per coding guidelines, only assert stderr when checking for a specific expected error message.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/js/web/websocket/websocket-unix.test.ts` around lines 207 - 210, Remove the fragile generic stderr assertion that checks for the substring "error" and rely on the existing definitive assertions (stdout.trim() equals "pong:from-child" and exitCode equals 0); specifically remove the line referencing stderr.not.toContain("error") that uses the stderr variable, and only add a stderr assertion if you have a precise expected stderr string to validate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/js/web/websocket/websocket-unix.test.ts`:
- Around line 207-210: Remove the fragile generic stderr assertion that checks
for the substring "error" and rely on the existing definitive assertions
(stdout.trim() equals "pong:from-child" and exitCode equals 0); specifically
remove the line referencing stderr.not.toContain("error") that uses the stderr
variable, and only add a stderr assertion if you have a precise expected stderr
string to validate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a6d7b4fc-2c35-4e48-8eed-7865f81956d4
📒 Files selected for processing (1)
test/js/web/websocket/websocket-unix.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The comment claimed a user-supplied Host header drives SNI, but the implementation (like the TCP path) only uses the URL host. Correct the comment to describe what actually happens and point at the workarounds.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/http/websocket_client/WebSocketUpgradeClient.zig`:
- Around line 338-340: Replace the silent OOM fallback when duplicating
host_slice so allocation failures crash instead of setting an empty hostname:
change the expression assigning client.hostname (the call to
bun.default_allocator.dupeZ(u8, host_slice.slice()) catch "") to handle errors
via bun.handleOom() for error.OutOfMemory and propagate any other errors instead
of swallowing them; ensure the surrounding function (in
WebSocketUpgradeClient.zig) has the appropriate error union to allow propagating
non-OOM errors and that strings.isIPAddress(host_slice.slice()) logic remains
unchanged.
- Around line 318-327: The unix-path can set client.tcp too late causing
handleConnectError() to detach the wrong sentinel; change the lifetime handling
in the Socket.connectUnixAnon branch to mirror the Socket.connectPtr() pattern
used elsewhere: assign client.tcp (or otherwise take ownership of the returned
socket) immediately after the connect succeeds, or if you must keep the current
ordering ensure you call client.tcp.detach() (or the equivalent detach method)
before calling client.deref() when client.state == .failed so the real unix
socket is detached rather than an old sentinel; update the
Socket.connectUnixAnon handling to match handleConnectError()/connectPtr
lifetime semantics to avoid tearing down a client whose tcp was never detached.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4d492598-713e-40e7-a6ff-119c9f77ccf6
📒 Files selected for processing (1)
src/http/websocket_client/WebSocketUpgradeClient.zig
ws+unix:///sock:api/v1 previously produced 'GET api/v1 HTTP/1.1', which is not valid origin-form per RFC 7230 section 5.3.1. Prepend a '/' when the user-supplied request path after the colon does not already start with one.
There was a problem hiding this comment.
This is a well-iterated feature PR — the two issues I raised earlier (misleading SNI comment, missing leading-slash normalization on the request path) were both fixed. No bugs remain open. Because the change touches core WebSocket networking code across C++, Zig, and the ABI boundary, I'll leave final sign-off to a human maintainer.
Extended reasoning...
Overview
Adds and URL scheme support to the WebSocket client. Changes span four files: (ABI signature update for both connect functions), (URL parsing, socket-path/request-path splitting, proxy bypass, Host defaulting), (new branch mirroring the TCP path), and a new test file covering 7 scenarios.
Security risks
No security concerns introduced. Unix domain socket connections are inherently local; the PR correctly disables proxy routing for them. TLS verification on uses the URL host for SNI (defaulting to 'localhost'), mirroring the TCP path — intentional and documented in the comment.
Level of scrutiny
Medium-high. The C++ ↔ Zig ABI is extended (both signatures gain a trailing parameter), the URL-parsing logic handles a new scheme family, and the Zig connect path is new code. All of these areas have been reviewed iteratively and the two bugs I found were fixed.
Other factors
Two bugs were found and fixed during review: a misleading comment about Host-header-driven SNI (commit 012e687) and missing leading-slash normalization on the colon-separated request path (commit 95458e5). The fragile stderr assertion was removed in commit 21821bb. All inline thread discussions are resolved. Test coverage is comprehensive (echo, path/query parsing, binary, connect failure, SyntaxError, TLS, subprocess).
2 participants
What does this PR do?
Adds Unix domain socket support to the WebSocket client via the
ws+unix://andwss+unix://URL schemes.Hostheader defaults tolocalhost, matching Node'shttp.request({ socketPath })andws.wss+unix://selects the TLS socket context and runs the normal handshake over the domain socket.How did you verify your code works?
New test file
test/js/web/websocket/websocket-unix.test.tscovers:Bun.serve({ unix }):path+ query string parsing andHostheader valueSyntaxErroron missing socket pathwss+unix://againstBun.serve({ unix, tls })Existing
websocket-client.test.tsandwebsocket-custom-headers.test.tscontinue to pass.Implementation
WebSocket.cpp: recognisews+unix:/wss+unix:, split pathname on the first:into socket path + request path, default host tolocalhost, skip proxy, pass the socket path through to Zig.WebSocketUpgradeClient.zig: newunix_socket_patharg; when set, dial viaSocket.connectUnixAnonon the existing per-SSLus_socket_contextinstead of the TCPconnectPtrpath. All downstream state (upgrade, adopt, deflate, custom SSL ctx) is unchanged.headers.h: add the parameter to bothBun__WebSocketHTTP{,S}Client__connectexterns.Fixes #4423