Skip to content

Commit cb01605

Browse files
pi0pi0
committed
fix(ws): preserve wss:// protocol and fix error handling in proxyUpgrade
- Preserve TLS protocol from addr string in `_buildTargetURL` so wss:// and https:// targets use HTTPS upstream instead of silently falling back to plain HTTP - Consume upstream response body (`res.resume()`) when client socket is already destroyed to prevent unhandled stream errors (both proxyUpgrade and server ws-incoming) - Remove pre-upgrade error listener after successful upgrade in server ws-incoming stream pass to avoid spurious error events and pointless proxyReq.destroy()
1 parent efa9711 commit cb01605

3 files changed

Lines changed: 201 additions & 13 deletions

File tree

‎src/middleware/ws-incoming.ts‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -98,15 +98,20 @@ export const stream = defineProxyMiddleware<Socket>(
9898
// if upgrade event isn't going to happen, close the socket
9999
// guard against writing to an already-destroyed socket
100100
// (https://github.com/http-party/node-http-proxy/pull/1433)
101-
if (!(res as any).upgrade && !socket.destroyed && socket.writable) {
102-
socket.write(
103-
createHttpHeader(
104-
"HTTP/" + res.httpVersion + " " + res.statusCode + " " + res.statusMessage,
105-
res.headers,
106-
),
107-
);
108-
res.on("error", onOutgoingError);
109-
res.pipe(socket);
101+
if (!(res as any).upgrade) {
102+
if (!socket.destroyed && socket.writable) {
103+
socket.write(
104+
createHttpHeader(
105+
"HTTP/" + res.httpVersion + " " + res.statusCode + " " + res.statusMessage,
106+
res.headers,
107+
),
108+
);
109+
res.on("error", onOutgoingError);
110+
res.pipe(socket);
111+
} else {
112+
// Socket already gone — consume response to avoid unhandled stream errors
113+
res.resume();
114+
}
110115
}
111116
});
112117

@@ -118,6 +123,10 @@ export const stream = defineProxyMiddleware<Socket>(
118123
server.emit("close", proxyRes, proxySocket, proxyHead);
119124
});
120125

126+
// Remove the pre-upgrade error handler — it calls proxyReq.destroy()
127+
// which is pointless after upgrade and emits a spurious error event.
128+
socket.removeListener("error", onSocketError);
129+
121130
// The pipe below will end proxySocket if socket closes cleanly, but not
122131
// if it errors (eg, vanishes from the net and starts returning
123132
// EHOSTUNREACH). We need to do that explicitly.

‎src/ws.ts‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,12 @@ export function proxyUpgrade(
9696
): Promise<Socket> {
9797
const resolvedAddr = parseAddr(addr);
9898

99+
// Detect SSL from addr string protocol (wss:// or https://)
100+
let useSSL = false;
101+
if (typeof addr === "string" && !addr.startsWith("unix:")) {
102+
useSSL = isSSL.test(new URL(addr).protocol);
103+
}
104+
99105
// Validate WS upgrade request
100106
if (req.method !== "GET" || req.headers.upgrade?.toLowerCase() !== "websocket") {
101107
socket.destroy();
@@ -114,7 +120,7 @@ export function proxyUpgrade(
114120
}
115121

116122
// Build target URL for setupOutgoing
117-
const target = _buildTargetURL(resolvedAddr);
123+
const target = _buildTargetURL(resolvedAddr, useSSL);
118124
const requestOptions: ProxyUpgradeOptions & { target: URL } = {
119125
...opts,
120126
target,
@@ -159,6 +165,9 @@ export function proxyUpgrade(
159165
);
160166
res.on("error", onOutgoingError);
161167
res.pipe(sock);
168+
} else {
169+
// Socket already gone — consume response to avoid unhandled stream errors
170+
res.resume();
162171
}
163172
if (!settled) {
164173
settled = true;
@@ -210,13 +219,14 @@ export function proxyUpgrade(
210219

211220
// --- Internal ---
212221

213-
function _buildTargetURL(addr: ProxyAddr): URL {
222+
function _buildTargetURL(addr: ProxyAddr, useSSL = false): URL {
223+
const protocol = useSSL ? "https" : "http";
214224
if (addr.socketPath) {
215-
const url = new URL("http://unix");
225+
const url = new URL(`${protocol}://unix`);
216226
(url as any).socketPath = addr.socketPath;
217227
return url;
218228
}
219-
return new URL(`http://${addr.host || "localhost"}${addr.port ? `:${addr.port}` : ""}`);
229+
return new URL(`${protocol}://${addr.host || "localhost"}${addr.port ? `:${addr.port}` : ""}`);
220230
}
221231

222232
function _createHttpHeader(

‎test/ws.test.ts‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
import { createServer, type Server, type IncomingMessage } from "node:http";
2+
import { createServer as createHTTPSServer } from "node:https";
3+
import { readFileSync } from "node:fs";
4+
import { join } from "node:path";
25
import { Duplex } from "node:stream";
36
import { connect, type AddressInfo } from "node:net";
47
import { afterAll, beforeAll, describe, expect, it } from "vitest";
@@ -683,4 +686,170 @@ describe("proxyUpgrade", () => {
683686
proxy.close();
684687
});
685688
});
689+
690+
describe("wss:// (TLS upstream)", () => {
691+
const __dirname = new URL(".", import.meta.url).pathname;
692+
const sslOpts = {
693+
key: readFileSync(join(__dirname, "fixtures", "agent2-key.pem")),
694+
cert: readFileSync(join(__dirname, "fixtures", "agent2-cert.pem")),
695+
};
696+
697+
it("should use HTTPS request for wss:// addr", async () => {
698+
// Create an HTTPS server with WebSocket support
699+
const httpsServer = createHTTPSServer(sslOpts);
700+
const targetWs = new ws.WebSocketServer({ server: httpsServer });
701+
702+
targetWs.on("connection", (socket) => {
703+
socket.on("message", (msg) => {
704+
socket.send("secure-echo:" + msg.toString("utf8"));
705+
});
706+
});
707+
708+
await new Promise<void>((resolve) => {
709+
httpsServer.listen(0, "127.0.0.1", resolve);
710+
});
711+
const targetPort = (httpsServer.address() as AddressInfo).port;
712+
713+
const proxy = createProxyServer(`wss://127.0.0.1:${targetPort}`, {
714+
secure: false,
715+
});
716+
const proxyPort = await listenServer(proxy);
717+
718+
const { promise, resolve } = Promise.withResolvers<void>();
719+
const client = new ws.WebSocket("ws://127.0.0.1:" + proxyPort);
720+
721+
client.on("open", () => {
722+
client.send("tls-test");
723+
});
724+
725+
client.on("message", (msg) => {
726+
expect(msg.toString("utf8")).toBe("secure-echo:tls-test");
727+
client.close();
728+
targetWs.close();
729+
httpsServer.close();
730+
proxy.close(() => resolve());
731+
});
732+
733+
client.on("error", (err) => {
734+
targetWs.close();
735+
httpsServer.close();
736+
proxy.close();
737+
throw err;
738+
});
739+
740+
await promise;
741+
});
742+
743+
it("should use HTTPS request for https:// addr", async () => {
744+
const httpsServer = createHTTPSServer(sslOpts);
745+
const targetWs = new ws.WebSocketServer({ server: httpsServer });
746+
747+
targetWs.on("connection", (socket) => {
748+
socket.on("message", (msg) => {
749+
socket.send("https-echo:" + msg.toString("utf8"));
750+
});
751+
});
752+
753+
await new Promise<void>((resolve) => {
754+
httpsServer.listen(0, "127.0.0.1", resolve);
755+
});
756+
const targetPort = (httpsServer.address() as AddressInfo).port;
757+
758+
const proxy = createProxyServer(`https://127.0.0.1:${targetPort}`, {
759+
secure: false,
760+
});
761+
const proxyPort = await listenServer(proxy);
762+
763+
const { promise, resolve } = Promise.withResolvers<void>();
764+
const client = new ws.WebSocket("ws://127.0.0.1:" + proxyPort);
765+
766+
client.on("open", () => {
767+
client.send("https-test");
768+
});
769+
770+
client.on("message", (msg) => {
771+
expect(msg.toString("utf8")).toBe("https-echo:https-test");
772+
client.close();
773+
targetWs.close();
774+
httpsServer.close();
775+
proxy.close(() => resolve());
776+
});
777+
778+
client.on("error", (err) => {
779+
targetWs.close();
780+
httpsServer.close();
781+
proxy.close();
782+
throw err;
783+
});
784+
785+
await promise;
786+
});
787+
788+
it("should use plain HTTP request for ws:// addr (no TLS)", async () => {
789+
// Verify ws:// still uses plain HTTP (not HTTPS)
790+
const proxy = createProxyServer({ host: "127.0.0.1", port: wsPort });
791+
const proxyPort = await listenServer(proxy);
792+
793+
const { promise, resolve } = Promise.withResolvers<void>();
794+
const client = new ws.WebSocket("ws://127.0.0.1:" + proxyPort);
795+
796+
client.on("open", () => {
797+
client.send("plain-test");
798+
});
799+
800+
client.on("message", (msg) => {
801+
expect(msg.toString("utf8")).toBe("echo:plain-test");
802+
client.close();
803+
proxy.close(() => resolve());
804+
});
805+
806+
await promise;
807+
});
808+
});
809+
810+
describe("non-upgrade response with destroyed socket", () => {
811+
it("should consume response body when socket is already destroyed", async () => {
812+
// Regression: when the client socket is destroyed before the upstream
813+
// non-upgrade response arrives, the response stream must be consumed
814+
// (res.resume()) to avoid unhandled stream errors.
815+
const { promise: targetReqReceived, resolve: onTargetReq } = Promise.withResolvers<void>();
816+
const { promise: canRespond, resolve: allowResponse } = Promise.withResolvers<void>();
817+
818+
const targetServer = createServer(async (_req, res) => {
819+
onTargetReq();
820+
await canRespond;
821+
// Send a larger response body to make unconsumed stream errors more likely
822+
res.writeHead(503);
823+
res.end("Service Unavailable — " + "x".repeat(1024));
824+
});
825+
const targetPort = await listenServer(targetServer);
826+
827+
const server = createServer();
828+
const { promise, resolve } = Promise.withResolvers<void>();
829+
830+
server.on("upgrade", (req, socket, head) => {
831+
// Destroy socket before upstream responds
832+
targetReqReceived.then(() => {
833+
socket.destroy();
834+
setTimeout(allowResponse, 10);
835+
});
836+
837+
proxyUpgrade({ host: "127.0.0.1", port: targetPort }, req, socket, head).catch(() => {
838+
// Give time for any potential unhandled stream errors to surface
839+
setTimeout(resolve, 50);
840+
});
841+
});
842+
843+
const port = await listenServer(server);
844+
845+
const sock = connect(port, "127.0.0.1", () => {
846+
sock.write(wsUpgradeRequest(port));
847+
});
848+
sock.on("error", () => {});
849+
850+
await promise;
851+
targetServer.close();
852+
server.close();
853+
});
854+
});
686855
});

0 commit comments

Comments
 (0)