WebSocket: send credentials for a password-only URL, decode userinfo to bytes

Closed
RO
1mo ago
+349-18

Problem

  • new WebSocket("wss://:SECRET@host") sends no Authorization header. npm ws, curl and fetch() send :SECRET.
  • ws://user:p%E9ss@host sends user:. ws://us%ffer:pass@host sends no header. A proxy URL does the same. No user reported these.
  • URL::user() and URL::password() (src/jsc/bindings/webcore/WebSocket.cpp:233, :606) decode the userinfo as UTF-8 text and return a null String for other bytes. The header also needs a user.

Fix

  • One helper, basicAuthorization(const URL&), builds both headers from the percent-decoded bytes of encodedUser() and encodedPassword(). A user or a password is enough for a header.
  • Bytes are a choice, for parity with fetch() (#42692) and curl. Valid UTF-8 userinfo is unchanged.
  • Verified: 21 new tests in websocket-upgrade.test.ts, websocket-proxy.test.ts and ws-proxy.test.ts, all fail on main. Also ran test/js/web/websocket/ and test/js/first_party/ws/.
  • Self-reviewed: 20 concerns raised, 16 addressed, 4 in part (Notes).

Background

  • The userinfo is the user:password@ part of a URL. encodedUser() returns it still percent-encoded.
  • Bun sends it as Basic credentials (#26278). Node's WebSocket (after a 401) and npm ws send it as written.
  • Considered one Rust decoder shared with fetch(): it changes two FFI arguments and fetch() behaviour.

Downsides

  • A literal %e9 in a password still fails: Bun sends the byte, Node the text. Write %25.
  • A proxy URL credential that main dropped replaces a Proxy-Authorization in proxy.headers, as user:pass does.
  • Not changed: fetch() and HTTP_PROXY send no header for a % that is not an escape (src/http/AsyncHTTP.rs:112). URLPattern (#34203) and RedisClient (#41795) still call URL::user().
Notes

What each client sends

The Basic credentials that reach a loopback server, decoded. Bun columns are release builds of main 7adb8ee1dc with and without this change. curl is 8.14.1, Node is v26.3.0 with ws 8.18.3.

userinfoWebSocket, mainWebSocket, this PRfetch()curlNode WebSocket after a 401, npm ws at once
:passno header:pass:pass:pass:pass
user:p%E9ssuser:user:p\xE9ssuser:p\xE9ssuser:p\xE9ssuser:p%E9ss
user:pa%ffssuser:user:pa\xFFssuser:pa\xFFssuser:pa\xFFssuser:pa%ffss
us%ffer:passno headerus\xFFer:passus\xFFer:passus\xFFer:passus%ffer:pass
user:pa%25ssuser:pa%ssuser:pa%ssuser:pa%ssuser:pa%ssuser:pa%25ss
user:pa%zzssuser:pa%zzssuser:pa%zzssno headeruser:pa%zzssuser:pa%zzss

For a proxy URL, the WebSocket and fetch() columns are the same on Proxy-Authorization.

Who this is for

  • :SECRET is the shape of an API key that is sent as the password with no user (curl --user :SECRET). A library that takes only a URL string has no other way to pass it. Through Bun's ws, main sends no header where npm ws on Node sends :SECRET.
  • The rows with an escape that is not UTF-8 have no report in the tracker. The two requests behind this feature (#8320, #24388) use ASCII user:password, which does not change. Such a URL is written for curl or fetch(), for example for a server that reads Basic credentials as ISO-8859-1. There %E9 is the only way to write é in a URL.
  • On a released Bun, a caller who owns the constructor can pass headers: { Authorization }, or proxy: { url, headers: { "Proxy-Authorization": ... } }, and get the same bytes.

Why bytes

Three answers are in use for an escape that is not UTF-8:

  • The bytes: curl (for http:// and ws://) and Bun's fetch().
  • The text as written: npm ws, and Node's WebSocket after a 401.
  • An error: http.request(new URL(...)) in Node and in Bun, and https-proxy-agent, throw URIError.

This change takes the first. One proxy URL then gives fetch() and WebSocket the same Proxy-Authorization. Each URL that connects on main sends the same bytes as before. The text answer would undo the decoding that #26278 added and that test/regression/issue/24388.test.ts pins. An error would make URLs that connect today fail.

The gate and the decoder go together. With hasCredentials() and the old text decode, us%ffer:pass would send :pass.

Not changed here

  • fetch() and each reader of HTTP_PROXY (fetch(), bun install, bun pm, bun upgrade, bun create, the S3 client). basic_authorization (src/http/AsyncHTTP.rs:112) decodes with PercentEncoding::decode_alloc, which fails on a % that two hex digits do not follow. The function then returns no header, with a debug log only. HTTP_PROXY=http://svc:Winter2024%@127.0.0.1:PORT bun pm view left-pad sends no Proxy-Authorization. The answer for that % is open: the literal byte (curl, WebSocket) or an error (node:http). A change there moves the proxy credentials of the package manager, so it is not part of a WebSocket change.
  • Six calls of URL::user() and URL::password() remain. src/jsc/bindings/webcore/URLPattern.cpp:93, :101, :368, :369 need the text that is still encoded (#34203). src/jsc/bindings/BunString.cpp:685, :690 feed the RedisClient and need bytes (#41795). On main, new URLPattern({}).exec("https://user:pa%ffss@h.example/").password.input is "", and redis://us%ffer:pass@host sends HELLO 3 AUTH default pass. When both are in, Bun has no caller of the two accessors, and Bun's build of WebKit can leave them out.
  • new Bun.SQL("postgres://user:p%E9ss@host/db") throws URIError. It fails before it connects, and it decodes in JS, not through URL::user().
  • %00 in the userinfo still sends a NUL byte. curl refuses that URL.
  • URL::fileSystemPath() has the same null return for a path. oven-sh/WebKit#789 covers it.
  • Bun's ws ignores the auth option. new WebSocket(url, { auth: "user:pass" }) sends no header, and npm ws on Node sends user:pass. That is in src/js/thirdparty/ws.js, not in the code that builds the credentials.

The decoder

appendPercentDecoded is the WHATWG percent-decode rule. It is written out because URLParser::percentDecode is private in WTF. #41795 (decode_whatwg) and #39603, #41667 (decode_lenient_alloc) add the same rule in Rust on their branches. None is on main.

A design review before the code weighed six options, one of them no change. The helper at the two call sites won over a decoder in the Rust upgrade client, an exported Rust function, a byte accessor in WebKit, and a null test with a second decode.

Measurements

Release builds of main 7adb8ee1dc and of this change on it, linux-x64, same toolchain. Tools: gdb, llvm-objdump, nm, size, and a ptrace single-stepper. Not installed: perf, valgrind, strace, bloaty.

  • Credential block per new WebSocket(url): no userinfo 26 instructions (main: 121), proxied 28 (main: 105), user:pass 498 (main: 1,980). gdb stepi between the block bounds, 3 identical runs.
  • Allocator calls per construct: connect 13 without userinfo (main: 13), 17 with user:pass (main: 23), 19 with a 201-byte userinfo (main: 24). setupProxy 2 and 3 (main: 2 and 9). gdb breakpoint counters on mi_*.
  • Text: connect 7,314 B (main: 7,702), setupProxy 2,279 B (main: 3,069), basicAuthorization + appendPercentDecoded 1,267 B. Release .text is 448 B smaller (nm -S and size).
  • Whole connect with no userinfo, outside the allocator: 13,597 to 13,604 instructions (main: 13,696).
  • Wire matrix, 14 userinfo rows x 6 clients: 10 cells changed. All are WebSocket target or proxy cells of the :pass rows and the rows that are not UTF-8, and each now equals the fetch() and curl cells. No cell that was right on main changed.
  • New tests: 21 cases. 21 fail on main 6c1eb067e3, 0 fail with bun bd test.
  • Calls to URL::user() and URL::password() on the WebSocket credential path: 0 (main: 6 call instructions on 4 source lines).
  • Merge of WebSocket.cpp with open #44122, #44123, #37638, #41698, #41674: 0 conflict hunks (git merge-file). #41712 has 2 against main alone, and this change adds none.

Tests

  • test/js/web/websocket/websocket-upgrade.test.ts: a 24-row table for new WebSocket(url), then 4 rows through each form of the constructor, a URL in a 16-bit string, wss://, and the ws package with and without finishRequest. An Authorization header from the caller still wins. Three rows each pin one clause of the escape test (user:pa%fzss, user:pa%zfss, cafe12:beef34).
  • test/js/web/websocket/websocket-proxy.test.ts: a 12-row table on the CONNECT request, then a URL object, { url, headers }, a wss:// target and an https:// proxy. The URL credential wins over proxy.headers. A bypassed proxy sends nothing and the target still gets its own credentials.
  • test/js/first_party/ws/ws-proxy.test.ts: the proxy option and the three agent forms of Bun's ws.
  • The five tests of test/regression/issue/24388.test.ts pass unchanged.
  • test/js/web/websocket/ and test/js/first_party/ws/ on the release build with this change, 443 tests: 2 fail, should connect over https and should send and receive messages in websocket.test.js. They need the public internet, and the same 2 fail on main.
  • The same two directories on a debug build of this branch, 516 tests: 482 pass, 23 fail. 7 are in websocket.test.js: the public internet, and tests that run out of time on a debug build. 16 are subprocess tests in ws-upgrade-events.test.ts, websocket-syscall-fault.test.ts and websocket-proxy-close-reentrancy.test.ts. They run out of the 5 s default on my machine, and those files pass with --timeout 120000.
  • The ws rows for the target URL are in websocket-upgrade.test.ts. ws.test.ts has a TLS test (ciphers) that runs out of time on a debug build when the whole file runs.

Order with other changes

#41712 reads http_proxy and https_proxy for WebSocket through the same setupProxy, so its proxy URL gets these credentials too. This change adds no merge conflict to it.

Self-review

20 concerns raised. 16 addressed: 2 in code and tests (the rows that pin the clauses of the escape test, the name of the rule in the comment) and 14 in this text. 4 addressed in part: they ask for this text, which is here, and for tracking issues for the items under "Not changed here". No such issue is open yet. Two comments on this PR state the first two items, each with a script. One part not taken: a sentence in the docs about the decoding. The docs do not describe the URL credentials of WebSocket today, so that is a change of its own.