Visitar URL original
fix: skip HMR websocket path when forwarding upgrades to user-defined… · webpack/webpack-dev-server@93e8996 · GitHub
Skip to content

Commit 93e8996

Browse files
ajhyndmanbjohansebas
ajhyndman
and
bjohansebas
authored
fix: skip HMR websocket path when forwarding upgrades to user-defined proxies (#4316)
* fix: never proxy hot-reload websocket requests Both create-react-app and vue-cli-service have introduced workarounds for this, but it seems to me that this is always desirable behaviour for webpack-dev-server. vuejs/vue-cli@72ba750 react/create-react-app#7444 * test: update snapshot and proxy option tests for port 8162 * fixup! --------- Co-authored-by: Sebastian Beltran <bjohansebas@gmail.com>
1 parent fd40130 commit 93e8996

6 files changed

Lines changed: 233 additions & 15 deletions

File tree

‎lib/Server.js‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1850,13 +1850,31 @@ class Server {
18501850
/** @type {RequestHandler[]} */
18511851
(this.webSocketProxies);
18521852

1853+
const hmrPath =
1854+
this.options.webSocketServer &&
1855+
/** @type {WebSocketServerConfiguration} */
1856+
(this.options.webSocketServer).options &&
1857+
/** @type {NonNullable<WebSocketServerConfiguration["options"]>} */
1858+
(
1859+
/** @type {WebSocketServerConfiguration} */
1860+
(this.options.webSocketServer).options
1861+
).path;
1862+
18531863
for (const webSocketProxy of webSocketProxies) {
1854-
/** @type {S} */
1855-
(this.server).on(
1856-
"upgrade",
1864+
const proxyUpgrade =
18571865
/** @type {RequestHandler & { upgrade: NonNullable<RequestHandler["upgrade"]> }} */
1858-
(webSocketProxy).upgrade,
1859-
);
1866+
(webSocketProxy).upgrade;
1867+
1868+
/** @type {S} */
1869+
(this.server).on("upgrade", (req, socket, head) => {
1870+
if (hmrPath && req.url) {
1871+
const { pathname } = new URL(req.url, "http://0.0.0.0");
1872+
if (pathname === hmrPath) {
1873+
return;
1874+
}
1875+
}
1876+
proxyUpgrade(req, socket, head);
1877+
});
18601878
}
18611879
}
18621880

‎test/e2e/__snapshots__/api.test.js.snap.webpack5‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ exports[`API Server.checkHostHeader should allow URLs with scheme for checking o
2929
"[webpack-dev-server] Server started: Hot Module Replacement enabled, Live Reloading enabled, Progress disabled, Overlay enabled.",
3030
"[HMR] Waiting for update signal from WDS...",
3131
"Hey.",
32-
"WebSocket connection to 'ws://test.host:8158/ws' failed: Error in connection establishment: net::ERR_NAME_NOT_RESOLVED",
32+
"WebSocket connection to 'ws://test.host:8159/ws' failed: Error in connection establishment: net::ERR_NAME_NOT_RESOLVED",
3333
"[webpack-dev-server] JSHandle@object",
3434
"[webpack-dev-server] Disconnected!",
3535
"[webpack-dev-server] Trying to reconnect...",
@@ -40,7 +40,7 @@ exports[`API Server.checkHostHeader should allow URLs with scheme for checking o
4040

4141
exports[`API Server.checkHostHeader should allow URLs with scheme for checking origin when the "option.client.webSocketURL" is object: response status 1`] = `200`;
4242

43-
exports[`API Server.checkHostHeader should allow URLs with scheme for checking origin when the "option.client.webSocketURL" is object: web socket URL 1`] = `"ws://test.host:8158/ws"`;
43+
exports[`API Server.checkHostHeader should allow URLs with scheme for checking origin when the "option.client.webSocketURL" is object: web socket URL 1`] = `"ws://test.host:8159/ws"`;
4444

4545
exports[`API Server.getFreePort should retry finding the port for up to defaultPortRetry times (number): console messages 1`] = `
4646
[

‎test/e2e/__snapshots__/client-reconnect.test.js.snap.webpack5‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ exports[`client.reconnect option specified as number should try to reconnect 2 t
2020
"Hey.",
2121
"[webpack-dev-server] Disconnected!",
2222
"[webpack-dev-server] Trying to reconnect...",
23-
"WebSocket connection to 'ws://localhost:8163/ws' failed: Error in connection establishment: net::ERR_CONNECTION_REFUSED",
23+
"WebSocket connection to 'ws://localhost:8164/ws' failed: Error in connection establishment: net::ERR_CONNECTION_REFUSED",
2424
"[webpack-dev-server] JSHandle@object",
2525
"[webpack-dev-server] Trying to reconnect...",
26-
"WebSocket connection to 'ws://localhost:8163/ws' failed: Error in connection establishment: net::ERR_CONNECTION_REFUSED",
26+
"WebSocket connection to 'ws://localhost:8164/ws' failed: Error in connection establishment: net::ERR_CONNECTION_REFUSED",
2727
"[webpack-dev-server] JSHandle@object",
2828
]
2929
`;

‎test/e2e/__snapshots__/port.test.js.snap.webpack5‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,25 +20,25 @@ exports[`port should work using "0" port : console messages 1`] = `
2020

2121
exports[`port should work using "0" port : page errors 1`] = `[]`;
2222

23-
exports[`port should work using "8161" port : console messages 1`] = `
23+
exports[`port should work using "8162" port : console messages 1`] = `
2424
[
2525
"[webpack-dev-server] Server started: Hot Module Replacement enabled, Live Reloading enabled, Progress disabled, Overlay enabled.",
2626
"[HMR] Waiting for update signal from WDS...",
2727
"Hey.",
2828
]
2929
`;
3030

31-
exports[`port should work using "8161" port : console messages 2`] = `
31+
exports[`port should work using "8162" port : console messages 2`] = `
3232
[
3333
"[webpack-dev-server] Server started: Hot Module Replacement enabled, Live Reloading enabled, Progress disabled, Overlay enabled.",
3434
"[HMR] Waiting for update signal from WDS...",
3535
"Hey.",
3636
]
3737
`;
3838

39-
exports[`port should work using "8161" port : page errors 1`] = `[]`;
39+
exports[`port should work using "8162" port : page errors 1`] = `[]`;
4040

41-
exports[`port should work using "8161" port : page errors 2`] = `[]`;
41+
exports[`port should work using "8162" port : page errors 2`] = `[]`;
4242

4343
exports[`port should work using "auto" port : console messages 1`] = `
4444
[

‎test/ports-map.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ const listOfTests = {
3535
"on-listening-option": 1,
3636
"open-option": 1,
3737
"port-option": 1,
38-
"proxy-option": 4,
38+
"proxy-option": 5,
3939
server: 1,
4040
"setup-exit-signals-option": 1,
4141
"static-directory-option": 1,

‎test/server/proxy-option.test.js‎

Lines changed: 201 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"use strict";
22

3+
const http = require("node:http");
34
const path = require("node:path");
45
const util = require("node:util");
56
const express = require("express");
@@ -8,7 +9,8 @@ const webpack = require("webpack");
89
const WebSocket = require("ws");
910
const Server = require("../../lib/Server");
1011
const config = require("../fixtures/proxy-config/webpack.config");
11-
const [port1, port2, port3, port4] = require("../ports-map")["proxy-option"];
12+
const [port1, port2, port3, port4, port5] =
13+
require("../ports-map")["proxy-option"];
1214

1315
const WebSocketServer = WebSocket.Server;
1416
const staticDirectory = path.resolve(__dirname, "../fixtures/proxy-config");
@@ -659,6 +661,204 @@ describe("proxy option", () => {
659661
}
660662
});
661663

664+
describe("should not silently proxy dev-server HMR websocket to a permissive backend", () => {
665+
let server;
666+
let backend;
667+
let backendWss;
668+
let backendUpgradeCount;
669+
670+
const BACKEND_MESSAGE_TYPE = "backend-message";
671+
672+
beforeAll(async () => {
673+
backendUpgradeCount = 0;
674+
675+
backend = http.createServer();
676+
backendWss = new WebSocketServer({ server: backend });
677+
backendWss.on("connection", (connection) => {
678+
backendUpgradeCount += 1;
679+
connection.send(JSON.stringify({ type: BACKEND_MESSAGE_TYPE }));
680+
});
681+
682+
await new Promise((resolve) => {
683+
backend.listen(port5, resolve);
684+
});
685+
686+
const compiler = webpack(config);
687+
688+
server = new Server(
689+
{
690+
hot: true,
691+
allowedHosts: "all",
692+
webSocketServer: "ws",
693+
proxy: [
694+
{
695+
context: "/",
696+
target: `http://localhost:${port5}`,
697+
ws: true,
698+
},
699+
],
700+
port: port3,
701+
},
702+
compiler,
703+
);
704+
705+
await server.start();
706+
});
707+
708+
afterAll(async () => {
709+
for (const client of backendWss.clients) {
710+
client.terminate();
711+
}
712+
backendWss.close();
713+
// Force-drop any lingering proxy-opened sockets so backend.close() does
714+
// not hang when the fix is missing and the proxy is mid-upgrade.
715+
backend.closeAllConnections();
716+
await server.stop();
717+
await new Promise((resolve) => {
718+
backend.close(resolve);
719+
});
720+
});
721+
722+
it("delivers the HMR control messages and never reaches the proxy target", async () => {
723+
const messages = [];
724+
725+
const ws = new WebSocket(`ws://localhost:${port3}/ws`);
726+
727+
await new Promise((resolve, reject) => {
728+
const timer = setTimeout(() => {
729+
reject(
730+
new Error(
731+
`Timed out waiting for HMR message. Got: ${JSON.stringify(messages)}`,
732+
),
733+
);
734+
}, 3000);
735+
736+
ws.on("message", (raw) => {
737+
const parsed = JSON.parse(raw.toString());
738+
messages.push(parsed);
739+
if (parsed.type === "hot") {
740+
clearTimeout(timer);
741+
resolve();
742+
}
743+
});
744+
745+
ws.on("error", (err) => {
746+
clearTimeout(timer);
747+
reject(err);
748+
});
749+
});
750+
751+
ws.close();
752+
753+
// Let the proxy finish its async forwarding so the assertion below sees
754+
// the upgrade attempt deterministically.
755+
await new Promise((resolve) => {
756+
setTimeout(resolve, 300);
757+
});
758+
759+
expect(messages.some((m) => m.type === "hot")).toBe(true);
760+
expect(messages.some((m) => m.type === BACKEND_MESSAGE_TYPE)).toBe(false);
761+
expect(backendUpgradeCount).toBe(0);
762+
});
763+
});
764+
765+
describe("should not log proxy errors for the dev-server HMR upgrade", () => {
766+
let server;
767+
let backend;
768+
let stderrSpy;
769+
770+
beforeAll(async () => {
771+
stderrSpy = jest
772+
.spyOn(process.stderr, "write")
773+
.mockImplementation(() => true);
774+
775+
backend = http.createServer();
776+
backend.on("upgrade", (req, socket) => {
777+
socket.destroy();
778+
});
779+
await new Promise((resolve) => {
780+
backend.listen(port5, resolve);
781+
});
782+
783+
const compiler = webpack(config);
784+
785+
server = new Server(
786+
{
787+
hot: true,
788+
allowedHosts: "all",
789+
webSocketServer: "ws",
790+
proxy: [
791+
{
792+
context: "/",
793+
target: `http://localhost:${port5}`,
794+
ws: true,
795+
},
796+
],
797+
port: port3,
798+
},
799+
compiler,
800+
);
801+
802+
await server.start();
803+
});
804+
805+
afterAll(async () => {
806+
stderrSpy.mockRestore();
807+
backend.closeAllConnections();
808+
await server.stop();
809+
await new Promise((resolve) => {
810+
backend.close(resolve);
811+
});
812+
});
813+
814+
it("does not surface any [HPM] error when the HMR client connects", async () => {
815+
const messages = [];
816+
817+
const ws = new WebSocket(`ws://localhost:${port3}/ws`);
818+
819+
await new Promise((resolve, reject) => {
820+
const timer = setTimeout(() => {
821+
reject(
822+
new Error(
823+
`Timed out waiting for HMR message. Got: ${JSON.stringify(messages)}`,
824+
),
825+
);
826+
}, 3000);
827+
828+
ws.on("message", (raw) => {
829+
const parsed = JSON.parse(raw.toString());
830+
messages.push(parsed);
831+
if (parsed.type === "hot") {
832+
clearTimeout(timer);
833+
resolve();
834+
}
835+
});
836+
837+
ws.on("error", (err) => {
838+
clearTimeout(timer);
839+
reject(err);
840+
});
841+
});
842+
843+
ws.close();
844+
845+
await new Promise((resolve) => {
846+
setTimeout(resolve, 200);
847+
});
848+
849+
const hpmLines = stderrSpy.mock.calls
850+
.map((c) => c[0])
851+
.join("")
852+
.split("\n")
853+
.filter((line) => line.includes("[HPM]"))
854+
.map((line) => line.replaceAll(/localhost:\d+/g, "localhost:<port>"))
855+
.join("\n");
856+
857+
expect(hpmLines).toBe("");
858+
expect(messages.some((m) => m.type === "hot")).toBe(true);
859+
});
860+
});
861+
662862
describe("should supports http methods", () => {
663863
let server;
664864
let req;

0 commit comments

Comments
 (0)