ws: upgrade through the request's own native response, refuse a handshake that has none

Closed
RO
1mo ago
+677-216

Problem

  • One WebSocket Upgrade request ends the server process: TypeError: undefined is not an object (evaluating 'socket') or TypeError: upgrade requires a Request object from completeUpgrade (src/js/thirdparty/ws.js:1549). It hits http2 allowHTTP1, server.emit("connection"), and http.Server after close().
  • A GET and a rejected handshake in one write throw Cannot writeHead headers after they are sent to the client from abortHandshake (ws.js:951). Both are uncaught since #35347.

Fix

  • ws upgrades through the request's own native response (upgradeNodeHTTPResponse) and its own server. A handshake after close() gets 101, as in Node.js.
  • A request that node:http parsed in JS has none: only that client gets 500. abortHandshake() no longer answers through another request's response.
  • NodeHTTPResponse::can_upgrade() runs before anything is written. A stopped server counts a WebSocket that opens late.
  • Verified: test/js/first_party/ws/ws.test.ts (59 new tests, 40 fail on main). Self-reviewed: 4 concerns raised, 3 addressed (no tracking issue opened).

Background

  • The built-in ws server has no WebSocket framing. It upgrades through the native response (NodeHTTPResponse) of the Upgrade request, into the Bun.serve instance behind http.Server.
  • node:http serves connections that this instance did not accept from JS, without a native response.
  • Considered null checks at both throw sites: they keep the lookup through socket.server, which close() clears.

Downsides

  • WebSockets still fail on JS-served connections: 500 where Node.js answers 101.
  • Removed the NodeHTTPResponse arm of server.upgrade(). ws was its only caller.
  • JS2Native functions 185 to 186, bundled JS +1103 bytes. Release binary size not measured (the release build did not finish).
Notes

1.4.2 closed that one client on the JS-served connections, because a throw out of a socket 'data' listener was not yet an uncaught exception.

Connections and what they get (raw clients, ws.test.ts, { server } and noServer with handleUpgrade() in a later task or in a 'request' listener):

connectionmainthis PRNode.js + npm ws
http.Server#listen(), https.Server#listen()101101101
after http.Server#close(), connection still openTypeError, exit 1 (all releases since 1.3.14)101, 'close' waits for the WebSocket101
http2 allowHTTP1TypeError, exit 1500101
server.emit("connection", socket)TypeError, exit 1500101
the same on a server that also listensupgrade requires a Request object, exit 1500101
socket of a 'connect' event handed to an inner serverTypeError, exit 1500101
socket of an 'upgrade' event handed to an inner serverTypeError, or a 101 built from the outer request's key500101
GET + rejected handshake in one write (JS-served)ERR_HTTP_HEADERS_SENT, exit 1200, 400200, 400

The 500 is 500 Internal Server Error with the body WebSocket is not supported on this connection, written to the socket like every other refusal of the npm package, or through the ServerResponse when handleUpgrade() runs in that request's 'request' listener.

Which native response. node:http keeps the native response of a request on the IncomingMessage (kHeaderSource, now exported from internal/http. It already existed for req.rawHeaders and is never cleared). The two places where node:http parses a request in JS (internal/http1_server_fallback, node:_http_common) set the slot to null. Such a request is refused before handleProtocols and 'headers' run. On any other request object the slot is undefined (a plain object, or an IncomingMessage that the program constructed), so the response that the socket holds is taken, as before. The socket of a 'connect' or 'upgrade' event still holds the native response of that event. A later request on it is parsed in JS and must not borrow it.

Which response carries a refusal. abortHandshake() answers through socket._httpMessage when that response belongs to the request, or to a wrapper that shares its native response (a Proxy, an object that inherits from it). In every other case the reply goes to the socket and ends it, as the npm package does. For a GET that is still in flight and a rejected handshake behind it, Node.js v26.3.0 with ws 8.18.3 sends the 400 alone and closes the connection. A test pins the same behavior here.

Defects found on the way, fixed here.

  • server.upgrade(res, { headers }) wrote 101 Switching Protocols before it found that the response cannot upgrade. A tunnel into a listening inner server with a subprotocol read 101 and then 500. The arm that did this is removed. The new entry writes nothing before can_upgrade().
  • uWS reports the connection closed inside HttpResponse::upgrade(), before the WebSocket opens. With no listener and no request in flight (a handshake completed in a later task after close()), the server counted itself drained in that window and cleared its WebSocket handler's server pointer. The WebSocket was never counted, 'close' fired under it and the loop ref was dropped. upgrade() now holds a pending request across the uWS call.
  • can_upgrade() asks the connection before it reads the server: the response can outlive its server, an open connection cannot.

The subprotocol. handleProtocols can return any value. It still goes through Headers (FetchHeaders::put): trimmed, converted with toString(), and a value with CR, LF, NUL or a code point above 0xFF throws Header 'Sec-WebSocket-Protocol' has invalid value, as before.

Removed. The NodeHTTPResponse arm of server.upgrade() in src/runtime/server/server_body.rs (117 lines), its three tests that reached it through Symbol.for("::bunternal::"), and the sec_websocket_extensions parameter of NodeHTTPResponse::upgrade(). The re-entrancy those tests covered (user code that ends the response during the conversion) is now tested through handleProtocols.

Other pull requests.

  • #28152 (a socket argument wrapped in a Proxy that hides symbol keys, upgrade requires a Request object): covered, because the upgrade follows the request and not the socket. Its case is a test here.
  • #43441: the req parameter of abortHandshake() is the same idea as its ws.js hunk. The dispatcher's pipelining gate that it changes is untouched. Its comment at _http_server.ts:930 is updated, because the reason it gave is no longer true.
  • #43176 edits completeUpgrade() too. Whichever lands second needs a small rebase.

Costs for a handshake that always worked. Bundled JS by file: ws.js +888 bytes, _http_common.js +133, internal/http.js +58, http1_server_fallback.js +42, _http_incoming.js -18. One property read on the request and one branch in nativeRequest(). No options object and no headers object per handshake any more. Natively: one can_upgrade() call in place of two inline checks, and two counter updates around the uWS upgrade. Nothing changes for a request that is not a handshake: _http_server.ts has one comment line changed, and the IncomingMessage has the same own properties.

Not covered. A handshake that arrives while an earlier response on the same native connection is still in flight is dispatched as a plain request (the gate of #43441). A real WebSocket on a JS-served connection needs framing over a plain Duplex in the built-in ws, or adoption of the socket by the native server.

Commits after the first push. @cirospaciari added the undefined state of the slot (a constructed IncomingMessage upgrades as it did before), the wrapper rule of abortHandshake(), HeadersRef for the temporary headers, and shorter comments. ws.test.ts on that head: 134 pass, 0 fail.

Suites run with the debug build. test/js/first_party/ws/ (4 files), test/js/node/http/node-http-with-ws.test.ts, node-http.test.ts, node-http-connect.test.ts, test/regression/issue/{3613,32734,26358,29684,012040,03844}, and the vendored test-http-upgrade-server*.js, test-http2-https-fallback.js, test-http2-allow-http1.js, test-http-server-close-idle.js, test-http-connect.js. The spawn-based tests in these files time out on this machine when it is loaded, with and without the change. Each one passes when it runs alone. Not run: macOS, Windows.