From 4865f2e62eff9cf59f602e753d9f84159a3139af Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne Date: Tue, 22 Oct 2024 00:49:25 +0200 Subject: [PATCH] fix(eio-client): prevent infinite loop with Node.js built-in WebSocket Related: https://github.com/socketio/socket.io/issues/5194 --- .github/workflows/ci.yml | 4 ++++ .../engine.io-client/lib/transports/websocket.ts | 1 + packages/engine.io-client/package.json | 1 + packages/engine.io-client/test/connection.js | 13 +++++++++++++ packages/engine.io-client/test/support/env.js | 8 ++++++++ packages/engine.io-client/test/transport.js | 15 ++++++++++++--- 6 files changed, 39 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2428514cd9..6acb3241c4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,3 +60,7 @@ jobs: - name: Run tests with fetch instead of XHR (engine.io-client) run: npm run test:node-fetch --workspace=engine.io-client if: ${{ matrix.node-version == '18' }} + + - name: Run tests with Node.js native WebSocket (engine.io-client) + run: npm run test:node-builtin-ws --workspace=engine.io-client + if: ${{ matrix.node-version == '22' }} diff --git a/packages/engine.io-client/lib/transports/websocket.ts b/packages/engine.io-client/lib/transports/websocket.ts index 25e2d4609b..b0099a7b4b 100644 --- a/packages/engine.io-client/lib/transports/websocket.ts +++ b/packages/engine.io-client/lib/transports/websocket.ts @@ -123,6 +123,7 @@ export abstract class BaseWS extends Transport { override doClose() { if (typeof this.ws !== "undefined") { + this.ws.onerror = () => {}; this.ws.close(); this.ws = null; } diff --git a/packages/engine.io-client/package.json b/packages/engine.io-client/package.json index 0a76e8c6a6..2ccaf9890d 100644 --- a/packages/engine.io-client/package.json +++ b/packages/engine.io-client/package.json @@ -63,6 +63,7 @@ "test": "npm run format:check && npm run compile && if test \"$BROWSERS\" = \"1\" ; then npm run test:browser; else npm run test:node; fi", "test:node": "mocha --bail --require test/support/hooks.js test/index.js test/webtransport.mjs", "test:node-fetch": "USE_FETCH=1 npm run test:node", + "test:node-builtin-ws": "USE_BUILTIN_WS=1 npm run test:node", "test:browser": "zuul test/index.js", "build": "rimraf ./dist && rollup -c support/rollup.config.umd.js && rollup -c support/rollup.config.esm.js", "bundle-size": "node support/bundle-size.js", diff --git a/packages/engine.io-client/test/connection.js b/packages/engine.io-client/test/connection.js index 5cb5d4e2fc..02f9be6e81 100644 --- a/packages/engine.io-client/test/connection.js +++ b/packages/engine.io-client/test/connection.js @@ -17,6 +17,19 @@ describe("connection", function () { }); }); + it("should connect to localhost (ws)", (done) => { + const socket = new Socket({ + transports: ["websocket"], + }); + socket.on("open", () => { + socket.on("message", (data) => { + expect(data).to.equal("hi"); + socket.close(); + done(); + }); + }); + }); + it("should receive multibyte utf-8 strings with polling", (done) => { const socket = new Socket(); socket.on("open", () => { diff --git a/packages/engine.io-client/test/support/env.js b/packages/engine.io-client/test/support/env.js index c9ee4ea56f..f96f26260b 100644 --- a/packages/engine.io-client/test/support/env.js +++ b/packages/engine.io-client/test/support/env.js @@ -35,3 +35,11 @@ if (exports.useFetch) { const { transports, Fetch } = require("../.."); transports.polling = Fetch; } + +exports.useBuiltinWs = process.env.USE_BUILTIN_WS !== undefined; + +if (exports.useBuiltinWs) { + console.warn("testing with built-in WebSocket object"); + const { transports, WebSocket } = require("../.."); + transports.websocket = WebSocket; +} diff --git a/packages/engine.io-client/test/transport.js b/packages/engine.io-client/test/transport.js index 5e5ad19bfa..47016fc3ab 100644 --- a/packages/engine.io-client/test/transport.js +++ b/packages/engine.io-client/test/transport.js @@ -210,7 +210,10 @@ describe("Transport", () => { // these are server only if (!env.browser) { describe("options", () => { - it("should accept an `agent` option for WebSockets", (done) => { + it("should accept an `agent` option for WebSockets", function (done) { + if (env.useBuiltinWs) { + return this.skip(); + } const polling = new eio.transports.websocket({ path: "/engine.io", hostname: "localhost", @@ -269,7 +272,10 @@ describe("Transport", () => { }); describe("perMessageDeflate", () => { - it("should set threshold", (done) => { + it("should set threshold", function (done) { + if (env.useBuiltinWs) { + return this.skip(); + } const socket = new eio.Socket({ transports: ["websocket"], perMessageDeflate: { threshold: 0 }, @@ -289,7 +295,10 @@ describe("Transport", () => { }); }); - it("should not compress when the byte size is below threshold", (done) => { + it("should not compress when the byte size is below threshold", function (done) { + if (env.useBuiltinWs) { + return this.skip(); + } const socket = new eio.Socket({ transports: ["websocket"] }); socket.on("open", () => { const ws = socket.transport.ws;