diff --git a/src/js/node/_http_server.ts b/src/js/node/_http_server.ts index c7c54c5582c..963d38e623c 100644 --- a/src/js/node/_http_server.ts +++ b/src/js/node/_http_server.ts @@ -601,7 +601,13 @@ Server.prototype[kRealListen] = function (tls, port, host, socketPath, reusePort socket[kRequest] = http_req; const is_upgrade = http_req.headers.upgrade; - if (!is_upgrade) { + // An Upgrade header only steers us off the request/response path when + // someone is actually listening for 'upgrade'. Otherwise Node surfaces + // the message as a normal 'request' event, and we need the response to + // be bound to the real socket (for `res.socket`, `writeContinue`, + // `writeEarlyHints`, the 'socket' event, timeout wiring, …). + const handleAsUpgrade = is_upgrade && server.listenerCount("upgrade") > 0; + if (!handleAsUpgrade) { if (canUseInternalAssignSocket) { // ~10% performance improvement in JavaScriptCore due to avoiding .once("close", ...) and removing a listener assignSocketInternal(http_res, socket); @@ -620,16 +626,24 @@ Server.prototype[kRealListen] = function (tls, port, host, socketPath, reusePort http_res.writeHead(503); http_res.end(); socket.destroy(); - } else if (is_upgrade) { - server.emit("upgrade", http_req, socket, kEmptyBuffer); - if (!socket._httpMessage) { - if (canUseInternalAssignSocket) { - // ~10% performance improvement in JavaScriptCore due to avoiding .once("close", ...) and removing a listener - assignSocketInternal(http_res, socket); - } else { - http_res.assignSocket(socket); - } - } + } else if (handleAsUpgrade) { + // Hand the connection off to the 'upgrade' handler, matching the + // CONNECT path: enable bidirectional streaming on the socket, tell + // uWS to stop HTTP-parsing inbound bytes (so they surface through + // `ondata`), and stay alive until the socket closes. Without this, + // `socket.write()` was a silent no-op because `handle.ondrain` was + // undefined and the response lifecycle tore the socket down before + // the peer could read the 101 handshake. (ws/rsbuild/http-proxy.) + socket[kEnableStreaming](true); + socketHandle.markAsRawMode(); + const { promise: upgradePromise, resolve: upgradeResolve } = $newPromiseCapability(Promise); + socket.once("close", upgradeResolve); + // Forward pipelined bytes that arrived in the same TCP segment as the + // Upgrade headers — Node passes them as the 3rd arg so `ws.handleUpgrade` + // can feed them into the new WebSocket stream as `head`. + const head = connectHead ? connectHead : kEmptyBuffer; + server.emit("upgrade", http_req, socket, head); + return upgradePromise; } else if (http_req.headers.expect !== undefined) { if (http_req.headers.expect === "100-continue") { if (server.listenerCount("checkContinue") > 0) { diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp b/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp index 0ed176ffae7..854d5b2e671 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp @@ -6,6 +6,7 @@ #include "helpers.h" #include #include +#include extern "C" EncodedJSValue us_socket_buffered_js_write(void* socket, bool is_ssl, bool ended, us_socket_stream_buffer_t* streamBuffer, JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue data, JSC::EncodedJSValue encoding); extern "C" uint64_t uws_res_get_remote_address_info(void* res, const char** dest, int* port, bool* is_ipv6); @@ -28,6 +29,7 @@ JSC_DECLARE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterBytesWritten); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketClose); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketWrite); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketEnd); +JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketMarkAsRawMode); JSC_DECLARE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterResponse); JSC_DECLARE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterRemoteAddress); JSC_DECLARE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterLocalAddress); @@ -56,6 +58,7 @@ static const JSC::HashTableValue JSNodeHTTPServerSocketPrototypeTableValues[] = { "close"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketClose, 0 } }, { "write"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketWrite, 2 } }, { "end"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketEnd, 0 } }, + { "markAsRawMode"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketMarkAsRawMode, 0 } }, { "secureEstablished"_s, static_cast(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterIsSecureEstablished, noOpSetter } }, }; @@ -113,6 +116,33 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketEnd, (JSC::JSGlobalObject return JSValue::encode(JSC::jsUndefined()); } +// Mark this socket's HTTP response context as a raw/tunnel connection (like CONNECT). +// After this runs, uWS will stop parsing inbound bytes as HTTP and will surface +// them to the JS-side `ondata` hook through `onSocketData`, matching the CONNECT +// path — required for `server.on("upgrade")` handlers that take over the socket +// (e.g. `ws.handleUpgrade` in rsbuild/vite/webpack-dev-server). +JSC_DEFINE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketMarkAsRawMode, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) +{ + auto* thisObject = dynamicDowncast(callFrame->thisValue()); + if (!thisObject) [[unlikely]] { + return JSValue::encode(JSC::jsUndefined()); + } + if (thisObject->isClosed() || thisObject->upgraded) { + return JSValue::encode(JSC::jsUndefined()); + } + + // The layout of HttpResponseData is independent of SSL for the `isConnectRequest` + // field offset, but we pick the right template instantiation anyway. + if (thisObject->is_ssl) { + auto* data = reinterpret_cast*>(us_socket_ext(thisObject->socket)); + if (data) data->isConnectRequest = true; + } else { + auto* data = reinterpret_cast*>(us_socket_ext(thisObject->socket)); + if (data) data->isConnectRequest = true; + } + return JSValue::encode(JSC::jsUndefined()); +} + // Implementation of custom getters JSC_DEFINE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterIsSecureEstablished, (JSC::JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, JSC::PropertyName)) { diff --git a/test/js/node/http/node-http-with-ws.test.ts b/test/js/node/http/node-http-with-ws.test.ts index a3ef8cac6a2..de920139061 100644 --- a/test/js/node/http/node-http-with-ws.test.ts +++ b/test/js/node/http/node-http-with-ws.test.ts @@ -1,10 +1,15 @@ import { expect, test } from "bun:test"; import { bunEnv, bunExe, tls as options } from "harness"; import https from "https"; -import type { AddressInfo } from "node:net"; +import { createHash } from "node:crypto"; +import http from "node:http"; +import { connect as netConnect, type AddressInfo } from "node:net"; import tls from "tls"; import { WebSocketServer } from "ws"; +// RFC 6455 GUID appended to Sec-WebSocket-Key before hashing. +const WS_GUID = "258EAFA5-E914-47DA-95CA-C5AB0DC85B11"; + test.concurrent("WebSocket upgrade should unref poll_ref from response", async () => { // Regression test for bug where poll_ref was not unref'd on WebSocket upgrade // The bug: NodeHTTPResponse.poll_ref stayed active after upgrade @@ -103,3 +108,216 @@ test.concurrent("should not crash when closing sockets after upgrade", async () await promise; expect().pass(); }); + +// Regression test for the rsbuild HMR failure reported in #30661 (duplicate of +// #9882, #18945, #14522, #26924): `server.on("upgrade", (req, socket) => +// socket.write(response))` was a silent no-op because the upgrade socket was +// not handed off to userland. The 101 handshake never left the server, the +// browser's WebSocket attempt timed out, and rsbuild surfaced that as an HMR +// error. Any `ws`-based stack (vite, webpack-dev-server, http-proxy, socket.io) +// hits the same path. +test.concurrent("server.on('upgrade') hands the raw socket off to userland", async () => { + await using server = http.createServer((_req, res) => { + res.writeHead(200); + res.end("not upgrade"); + }); + + const serverReceived: Buffer[] = []; + const { promise: serverGotData, resolve: resolveServerData } = Promise.withResolvers(); + const { promise: serverClosed, resolve: resolveServerClosed } = Promise.withResolvers(); + + server.on("upgrade", (req, socket) => { + const key = req.headers["sec-websocket-key"] as string; + const accept = createHash("sha1") + .update(key + WS_GUID) + .digest("base64"); + socket.write( + "HTTP/1.1 101 Switching Protocols\r\n" + + "Upgrade: websocket\r\n" + + "Connection: Upgrade\r\n" + + `Sec-WebSocket-Accept: ${accept}\r\n\r\n`, + ); + socket.on("data", chunk => { + serverReceived.push(chunk); + resolveServerData(); + }); + socket.on("close", () => resolveServerClosed()); + }); + + await new Promise(r => server.listen(0, "127.0.0.1", r)); + const { port } = server.address() as AddressInfo; + + const client = netConnect(port, "127.0.0.1"); + const handshakeBuf: Buffer[] = []; + const { promise: handshakeDone, resolve: resolveHandshake } = Promise.withResolvers(); + client.on("data", chunk => { + handshakeBuf.push(chunk); + const joined = Buffer.concat(handshakeBuf).toString("utf8"); + if (joined.includes("\r\n\r\n")) resolveHandshake(joined); + }); + client.write( + "GET /hmr HTTP/1.1\r\n" + + `Host: 127.0.0.1:${port}\r\n` + + "Upgrade: websocket\r\n" + + "Connection: Upgrade\r\n" + + "Sec-WebSocket-Version: 13\r\n" + + "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n\r\n", + ); + + const handshake = await handshakeDone; + expect(handshake).toStartWith("HTTP/1.1 101 Switching Protocols\r\n"); + expect(handshake).toContain("Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo="); + // Only one HTTP status line — the response lifecycle must not sneak a 200 OK + // onto the wire after the handshake. + expect(handshake.split("HTTP/1.1").length - 1).toBe(1); + + // Post-upgrade bytes from the client must reach the server's `data` listener + // (uWS would otherwise keep parsing the socket as HTTP and drop the payload). + client.write(Buffer.from([0x81, 0x83, 0x00, 0x00, 0x00, 0x00, 0x66, 0x6f, 0x6f])); + await serverGotData; + expect(Buffer.concat(serverReceived)).toEqual(Buffer.from([0x81, 0x83, 0x00, 0x00, 0x00, 0x00, 0x66, 0x6f, 0x6f])); + + client.end(); + await serverClosed; +}); + +// Bytes the client sends in the same TCP segment as the Upgrade request +// headers (e.g. the first WebSocket frame piggy-backed onto the handshake +// write) must reach the upgrade listener as the 3rd `head` argument, matching +// Node — `ws.handleUpgrade` feeds `head` into the new WebSocket stream so +// these bytes aren't lost. +test.concurrent("server.on('upgrade') passes pipelined head bytes to the listener", async () => { + await using server = http.createServer(); + + const { promise: upgradeFired, resolve: resolveUpgrade } = Promise.withResolvers(); + server.on("upgrade", (_req, socket, head) => { + resolveUpgrade(Buffer.from(head)); + socket.destroy(); + }); + + await new Promise(r => server.listen(0, "127.0.0.1", r)); + const { port } = server.address() as AddressInfo; + + // Pipeline a WebSocket frame right after the header terminator so the + // request parser hands the trailing bytes to the upgrade event. + const pipelined = Buffer.from([0x81, 0x83, 0xaa, 0xbb, 0xcc, 0xdd, 0x66, 0x6f, 0x6f]); + const client = netConnect(port, "127.0.0.1"); + client.write( + Buffer.concat([ + Buffer.from( + "GET /hmr HTTP/1.1\r\n" + + `Host: 127.0.0.1:${port}\r\n` + + "Upgrade: websocket\r\n" + + "Connection: Upgrade\r\n" + + "Sec-WebSocket-Version: 13\r\n" + + "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n\r\n", + ), + pipelined, + ]), + ); + + const head = await upgradeFired; + expect(head).toEqual(pipelined); +}); + +// Node surfaces an Upgrade request as a regular `'request'` event when the +// server has no `'upgrade'` listener. In that fall-through path `res.socket` +// must be bound to the real `NodeHTTPServerSocket` — otherwise it gets a +// lazy `FakeSocket` whose `remoteAddress` is wrong, the `'socket'` event +// never fires on the response, and `writeContinue()` / `writeEarlyHints()` +// silently drop bytes. +test.concurrent("Upgrade with no 'upgrade' listener falls through to 'request' with the real socket", async () => { + const { promise: requestFired, resolve: resolveRequest } = Promise.withResolvers<{ + resSocketSameAsReqSocket: boolean; + hasRemoteAddress: boolean; + }>(); + + await using server = http.createServer((req, res) => { + resolveRequest({ + resSocketSameAsReqSocket: res.socket === req.socket, + hasRemoteAddress: typeof res.socket?.remoteAddress === "string", + }); + res.writeHead(200); + res.end("ok"); + }); + // Deliberately no `server.on("upgrade", ...)`. + + await new Promise(r => server.listen(0, "127.0.0.1", r)); + const { port } = server.address() as AddressInfo; + + const client = netConnect(port, "127.0.0.1"); + client.write( + "GET / HTTP/1.1\r\n" + + `Host: 127.0.0.1:${port}\r\n` + + "Upgrade: h2c\r\n" + + "Connection: Upgrade, HTTP2-Settings\r\n" + + "HTTP2-Settings: AAMAAABkAAQAAP__\r\n\r\n", + ); + client.on("data", () => {}); + + const result = await requestFired; + expect(result).toEqual({ resSocketSameAsReqSocket: true, hasRemoteAddress: true }); + client.end(); +}); + +test.concurrent("server.on('upgrade') works over TLS (https)", async () => { + await using server = https.createServer(options, (_req, res) => { + res.writeHead(200); + res.end("not upgrade"); + }); + + const serverReceived: Buffer[] = []; + const { promise: serverGotData, resolve: resolveServerData } = Promise.withResolvers(); + const { promise: serverClosed, resolve: resolveServerClosed } = Promise.withResolvers(); + + server.on("upgrade", (req, socket) => { + const key = req.headers["sec-websocket-key"] as string; + const accept = createHash("sha1") + .update(key + WS_GUID) + .digest("base64"); + socket.write( + "HTTP/1.1 101 Switching Protocols\r\n" + + "Upgrade: websocket\r\n" + + "Connection: Upgrade\r\n" + + `Sec-WebSocket-Accept: ${accept}\r\n\r\n`, + ); + socket.on("data", chunk => { + serverReceived.push(chunk); + resolveServerData(); + }); + socket.on("close", () => resolveServerClosed()); + }); + + await new Promise(r => server.listen(0, "127.0.0.1", r)); + const { port } = server.address() as AddressInfo; + + const client = tls.connect({ port, host: "127.0.0.1", ca: options.cert, rejectUnauthorized: false }); + const handshakeBuf: Buffer[] = []; + const { promise: handshakeDone, resolve: resolveHandshake } = Promise.withResolvers(); + client.on("data", chunk => { + handshakeBuf.push(chunk); + const joined = Buffer.concat(handshakeBuf).toString("utf8"); + if (joined.includes("\r\n\r\n")) resolveHandshake(joined); + }); + await new Promise(r => client.on("secureConnect", r)); + client.write( + "GET /hmr HTTP/1.1\r\n" + + `Host: 127.0.0.1:${port}\r\n` + + "Upgrade: websocket\r\n" + + "Connection: Upgrade\r\n" + + "Sec-WebSocket-Version: 13\r\n" + + "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n\r\n", + ); + + const handshake = await handshakeDone; + expect(handshake).toStartWith("HTTP/1.1 101 Switching Protocols\r\n"); + expect(handshake).toContain("Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo="); + expect(handshake.split("HTTP/1.1").length - 1).toBe(1); + + client.write(Buffer.from([0x81, 0x83, 0x00, 0x00, 0x00, 0x00, 0x66, 0x6f, 0x6f])); + await serverGotData; + expect(Buffer.concat(serverReceived)).toEqual(Buffer.from([0x81, 0x83, 0x00, 0x00, 0x00, 0x00, 0x66, 0x6f, 0x6f])); + + client.end(); + await serverClosed; +});