diff --git a/.changeset/fix-proxy-hijacking-hmr-websocket.md b/.changeset/fix-proxy-hijacking-hmr-websocket.md new file mode 100644 index 0000000000..dd4f2278fe --- /dev/null +++ b/.changeset/fix-proxy-hijacking-hmr-websocket.md @@ -0,0 +1,5 @@ +--- +"webpack-dev-server": patch +--- + +Prevent user-defined `ws: true` proxies from intercepting the dev server's own HMR WebSocket upgrade. diff --git a/lib/Server.js b/lib/Server.js index 377ebbf36d..9cfd070ba9 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -1756,13 +1756,31 @@ class Server { /** @type {RequestHandler[]} */ (this.webSocketProxies); + const hmrPath = + this.options.webSocketServer && + /** @type {WebSocketServerConfiguration} */ + (this.options.webSocketServer).options && + /** @type {NonNullable} */ + ( + /** @type {WebSocketServerConfiguration} */ + (this.options.webSocketServer).options + ).path; + for (const webSocketProxy of webSocketProxies) { - /** @type {S} */ - (this.server).on( - "upgrade", + const proxyUpgrade = /** @type {RequestHandler & { upgrade: NonNullable }} */ - (webSocketProxy).upgrade, - ); + (webSocketProxy).upgrade; + + /** @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 (pathname === hmrPath) { + return; + } + } + proxyUpgrade(req, socket, head); + }); } } diff --git a/test/e2e/__snapshots__/port.test.js.snap.webpack5 b/test/e2e/__snapshots__/port.test.js.snap.webpack5 index 317910202e..5afe975db3 100644 --- a/test/e2e/__snapshots__/port.test.js.snap.webpack5 +++ b/test/e2e/__snapshots__/port.test.js.snap.webpack5 @@ -10,7 +10,7 @@ exports[`port > should work using "0" port 2`] = ` [] `; -exports[`port > should work using "8159" port 1`] = ` +exports[`port > should work using "8160" port 1`] = ` [ "[webpack-dev-server] Server started: Hot Module Replacement enabled, Live Reloading enabled, Progress disabled, Overlay enabled.", "[HMR] Waiting for update signal from WDS...", @@ -18,11 +18,11 @@ exports[`port > should work using "8159" port 1`] = ` ] `; -exports[`port > should work using "8159" port 2`] = ` +exports[`port > should work using "8160" port 2`] = ` [] `; -exports[`port > should work using "8159" port 3`] = ` +exports[`port > should work using "8160" port 3`] = ` [ "[webpack-dev-server] Server started: Hot Module Replacement enabled, Live Reloading enabled, Progress disabled, Overlay enabled.", "[HMR] Waiting for update signal from WDS...", @@ -30,7 +30,7 @@ exports[`port > should work using "8159" port 3`] = ` ] `; -exports[`port > should work using "8159" port 4`] = ` +exports[`port > should work using "8160" port 4`] = ` [] `; diff --git a/test/ports-map.js b/test/ports-map.js index e980495bc9..f9f5d9dcfe 100644 --- a/test/ports-map.js +++ b/test/ports-map.js @@ -32,7 +32,7 @@ const listOfTests = { "on-listening-option": 1, "open-option": 1, "port-option": 1, - "proxy-option": 4, + "proxy-option": 5, server: 1, "setup-exit-signals-option": 1, "static-directory-option": 1, diff --git a/test/server/__snapshots__/proxy-option.test.js.snap.webpack5 b/test/server/__snapshots__/proxy-option.test.js.snap.webpack5 index 144a568902..577aaa55a7 100644 --- a/test/server/__snapshots__/proxy-option.test.js.snap.webpack5 +++ b/test/server/__snapshots__/proxy-option.test.js.snap.webpack5 @@ -1,3 +1,7 @@ +exports[`proxy option > should not log proxy errors for the dev-server HMR upgrade > does not surface any [HPM] error when the HMR client connects 1`] = ` +"" +`; + exports[`proxy option > should work and respect \`logger\` option > target > respects a proxy option when a request path is matched 1`] = ` "[HPM] Error occurred while proxying request %s to %s [%s] (%s)" `; diff --git a/test/server/proxy-option.test.js b/test/server/proxy-option.test.js index e5c1e6b5d7..1740185d78 100644 --- a/test/server/proxy-option.test.js +++ b/test/server/proxy-option.test.js @@ -1,3 +1,4 @@ +import http from "node:http"; import path from "node:path"; import { after, before, beforeEach, describe, it } from "node:test"; import { fileURLToPath } from "node:url"; @@ -13,7 +14,7 @@ import portsMap from "../ports-map.js"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); -const [port1, port2, port3, port4] = portsMap["proxy-option"]; +const [port1, port2, port3, port4, port5] = portsMap["proxy-option"]; const staticDirectory = path.resolve(__dirname, "../fixtures/proxy-config"); @@ -609,6 +610,203 @@ describe("proxy option", () => { } }); + describe("should not silently proxy dev-server HMR websocket to a permissive backend", () => { + let server; + let backend; + let backendWss; + let backendUpgradeCount; + + const BACKEND_MESSAGE_TYPE = "backend-message"; + + before(async () => { + backendUpgradeCount = 0; + + backend = http.createServer(); + backendWss = new WebSocketServer({ server: backend }); + backendWss.on("connection", (connection) => { + backendUpgradeCount += 1; + connection.send(JSON.stringify({ type: BACKEND_MESSAGE_TYPE })); + }); + await new Promise((resolve) => { + backend.listen(port5, resolve); + }); + + const compiler = webpack(config); + + server = new Server( + { + hot: true, + allowedHosts: "all", + webSocketServer: "ws", + proxy: [ + { + context: "/", + target: `http://localhost:${port5}`, + ws: true, + }, + ], + port: port3, + }, + compiler, + ); + + await server.start(); + }); + + after(async () => { + for (const client of backendWss.clients) { + client.terminate(); + } + backendWss.close(); + // Force-drop any lingering proxy-opened sockets so backend.close() does + // not hang when the fix is missing and the proxy is mid-upgrade. + backend.closeAllConnections(); + await server.stop(); + await new Promise((resolve) => { + backend.close(resolve); + }); + }); + + it("delivers the HMR control messages and never reaches the proxy target", async () => { + const messages = []; + + const ws = new WebSocket(`ws://localhost:${port3}/ws`); + + await new Promise((resolve, reject) => { + const timer = setTimeout(() => { + reject( + new Error( + `Timed out waiting for HMR message. Got: ${JSON.stringify(messages)}`, + ), + ); + }, 3000); + + ws.on("message", (raw) => { + const parsed = JSON.parse(raw.toString()); + messages.push(parsed); + if (parsed.type === "hot") { + clearTimeout(timer); + resolve(); + } + }); + + ws.on("error", (err) => { + clearTimeout(timer); + reject(err); + }); + }); + + ws.close(); + + // Let the proxy finish its async forwarding so the assertion below sees + // the upgrade attempt deterministically (without this, the proxy may + // reach the backend only after the test exits). + await new Promise((resolve) => { + setTimeout(resolve, 300); + }); + + expect(messages.some((m) => m.type === "hot")).toBe(true); + expect(messages.some((m) => m.type === BACKEND_MESSAGE_TYPE)).toBe(false); + expect(backendUpgradeCount).toBe(0); + }); + }); + + describe("should not log proxy errors for the dev-server HMR upgrade", () => { + let server; + let backend; + let stderrSpy; + + before(async () => { + stderrSpy = spyOn(process.stderr, "write").mockImplementation(() => true); + + backend = http.createServer(); + backend.on("upgrade", (req, socket) => { + socket.destroy(); + }); + await new Promise((resolve) => { + backend.listen(port5, resolve); + }); + + const compiler = webpack(config); + + server = new Server( + { + hot: true, + allowedHosts: "all", + webSocketServer: "ws", + proxy: [ + { + context: "/", + target: `http://localhost:${port5}`, + ws: true, + }, + ], + port: port3, + }, + compiler, + ); + + await server.start(); + }); + + after(async () => { + stderrSpy.mockRestore(); + backend.closeAllConnections(); + await server.stop(); + await new Promise((resolve) => { + backend.close(resolve); + }); + }); + + it("does not surface any [HPM] error when the HMR client connects", async (t) => { + const messages = []; + + const ws = new WebSocket(`ws://localhost:${port3}/ws`); + + await new Promise((resolve, reject) => { + const timer = setTimeout(() => { + reject( + new Error( + `Timed out waiting for HMR message. Got: ${JSON.stringify(messages)}`, + ), + ); + }, 3000); + + ws.on("message", (raw) => { + const parsed = JSON.parse(raw.toString()); + messages.push(parsed); + if (parsed.type === "hot") { + clearTimeout(timer); + resolve(); + } + }); + + ws.on("error", (err) => { + clearTimeout(timer); + reject(err); + }); + }); + + ws.close(); + + // Give the proxy a moment to (incorrectly) log any error. + await new Promise((resolve) => { + setTimeout(resolve, 200); + }); + + const hpmLines = stderrSpy.mock.calls + .map((c) => c[0]) + .join("") + .split("\n") + .filter((line) => line.includes("[HPM]")) + .map((line) => line.replaceAll(/localhost:\d+/g, "localhost:")) + .join("\n"); + + t.assert.snapshot(hpmLines); + expect(messages.some((m) => m.type === "hot")).toBe(true); + }); + }); + describe("should supports http methods", () => { let server; let req;