diff --git a/lib/Server.js b/lib/Server.js index 19631099ab..28f1687fab 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -1867,8 +1867,16 @@ class Server { /** @type {S} */ (this.server).on("upgrade", (req, socket, head) => { - if (hmrPath && req.url) { - const { pathname } = new URL(req.url, "http://0.0.0.0"); + if (hmrPath && typeof req.url === "string") { + // Match the configured HMR path exactly the same way the underlying + // WebSocket server (`ws`) does in `WebSocketServer#shouldHandle`: a + // raw, case-sensitive comparison of the request target with the query + // string stripped. Any normalization here would classify URL variants + // (`//ws`, `/WS`, …) as the HMR socket even though `ws` refuses them. + // https://github.com/websockets/ws/blob/8.18.3/lib/websocket-server.js#L214 + const queryIndex = req.url.indexOf("?"); + const pathname = + queryIndex !== -1 ? req.url.slice(0, queryIndex) : req.url; if (pathname === hmrPath) { return; } diff --git a/test/server/proxy-option.test.js b/test/server/proxy-option.test.js index c82e56d13e..c83b6cfd9b 100644 --- a/test/server/proxy-option.test.js +++ b/test/server/proxy-option.test.js @@ -859,6 +859,188 @@ describe("proxy option", () => { }); }); + describe("HMR upgrade dispatching to user proxies", () => { + let server; + let backend; + let backendUpgradeCount; + + // Start a backend WebSocket server (the user proxy target) and a dev-server + // proxying everything to it, with the given dev-server options merged in. + const setup = async (devServerOptions) => { + backendUpgradeCount = 0; + + backend = http.createServer(); + new WebSocketServer({ server: backend }).on("connection", () => { + backendUpgradeCount += 1; + }); + + await new Promise((resolve) => { + backend.listen(port5, resolve); + }); + + server = new Server( + { + hot: true, + allowedHosts: "all", + proxy: [ + { + context: "/", + target: `http://localhost:${port5}`, + ws: true, + }, + ], + port: port3, + ...devServerOptions, + }, + webpack(config), + ); + + await server.start(); + }; + + const teardown = async () => { + backend.closeAllConnections(); + await server.stop(); + await new Promise((resolve) => { + backend.close(resolve); + }); + }; + + // Open a WebSocket to `path` and report whether the dev-server completed the + // handshake (`opened`) and whether the upgrade was forwarded to the backend + // proxy (`forwarded`). + const probe = async (path) => { + const before = backendUpgradeCount; + + let opened = false; + const ws = new WebSocket(`ws://localhost:${port3}${path}`); + ws.on("open", () => { + opened = true; + }); + ws.on("error", () => {}); + + await new Promise((resolve) => { + setTimeout(resolve, 400); + }); + + try { + ws.close(); + } catch { + // ignore close errors on already-failed sockets + } + + await new Promise((resolve) => { + setTimeout(resolve, 200); + }); + + return { opened, forwarded: backendUpgradeCount > before }; + }; + + // Behavior shared by every WebSocket server implementation: the HMR socket + // is served locally and never forwarded, while any path the HMR server does + // not own falls through to the user proxy. SockJS serves its transport under + // `////websocket`, not the bare `/ws`. + const serverTypes = [ + { type: "ws", hmrPath: "/ws", nonHmrPath: "/not-hmr" }, + { + type: "sockjs", + hmrPath: "/ws/000/abcd1234/websocket", + nonHmrPath: "/not-hmr", + }, + ]; + + for (const { type, hmrPath, nonHmrPath } of serverTypes) { + describe(`with webSocketServerType: ${type}`, () => { + beforeAll(() => setup({ webSocketServer: type })); + + afterAll(teardown); + + it("serves the HMR upgrade locally and does not forward it to the proxy", async () => { + const { opened, forwarded } = await probe(hmrPath); + + expect(opened).toBe(true); + expect(forwarded).toBe(false); + }); + + it("forwards a non-HMR upgrade to the user proxy", async () => { + const { forwarded } = await probe(nonHmrPath); + + expect(forwarded).toBe(true); + }); + }); + } + + // `ws`-specific: the dispatch compares the path exactly the same way + // `WebSocketServer#shouldHandle` does, so only the configured path (query + // stripped) is the HMR socket; every other variant is forwarded. + describe("with the `ws` server, path matching is exact", () => { + beforeAll(() => setup({ webSocketServer: "ws" })); + + afterAll(teardown); + + it.each([ + ["exact path", "/ws"], + ["path with query string", "/ws?token=1"], + ])("treats %s (%s) as the HMR upgrade path", async (_label, path) => { + const { forwarded } = await probe(path); + + expect(forwarded).toBe(false); + }); + + it.each([ + ["leading double slash", "//ws"], + ["trailing slash", "/ws/"], + ["uppercase", "/WS"], + ["mixed case", "/wS"], + ["percent-encoded path", "/%77%73"], + ])("forwards %s (%s) to the user proxy", async (_label, path) => { + const { forwarded } = await probe(path); + + expect(forwarded).toBe(true); + }); + }); + + // The HMR path is read from the configured `webSocketServer` options, not a + // hardcoded `/ws`. + describe("with a custom `ws` path", () => { + beforeAll(() => + setup({ + webSocketServer: { type: "ws", options: { path: "/custom-hmr" } }, + }), + ); + + afterAll(teardown); + + it("treats the configured path (/custom-hmr) as the HMR upgrade path", async () => { + const { forwarded } = await probe("/custom-hmr"); + + expect(forwarded).toBe(false); + }); + + it("forwards the default path (/ws) once it is no longer the HMR path", async () => { + const { forwarded } = await probe("/ws"); + + expect(forwarded).toBe(true); + }); + }); + + // With no HMR server there is no socket to protect, so the filter never + // engages and even `/ws` is forwarded to the user proxy. + describe("without a webSocketServer", () => { + beforeAll(() => + setup({ hot: false, liveReload: false, webSocketServer: false }), + ); + + afterAll(teardown); + + it("forwards /ws to the user proxy because there is no HMR socket to protect", async () => { + const { forwarded } = await probe("/ws"); + + expect(forwarded).toBe(true); + }); + }); + }); + describe("should supports http methods", () => { let server; let req;