From 4c521e7af13b60e5a7f0139fc022f71af9469e66 Mon Sep 17 00:00:00 2001 From: Jaroslav Minarik Date: Wed, 19 Jul 2023 18:05:06 +1000 Subject: [PATCH 1/3] Clean up namespaces after client is rejected in middleware --- lib/socket.ts | 6 ++-- package-lock.json | 6 ++-- test/namespaces.ts | 69 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 6 deletions(-) diff --git a/lib/socket.ts b/lib/socket.ts index 89b5eeaf31..4825c67c1e 100644 --- a/lib/socket.ts +++ b/lib/socket.ts @@ -758,9 +758,6 @@ export class Socket< } this._cleanup(); - this.nsp._remove(this); - this.client._remove(this); - this.connected = false; this.emitReserved("disconnect", reason, description); return; } @@ -772,6 +769,9 @@ export class Socket< */ _cleanup() { this.leaveAll(); + this.nsp._remove(this); + this.client._remove(this); + this.connected = false; this.join = noop; } diff --git a/package-lock.json b/package-lock.json index 7095cb3082..026d06ceae 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "socket.io", - "version": "4.7.0", + "version": "4.7.1", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "socket.io", - "version": "4.7.0", + "version": "4.7.1", "license": "MIT", "dependencies": { "accepts": "~1.3.4", @@ -34,7 +34,7 @@ "uWebSockets.js": "github:uNetworking/uWebSockets.js#v20.30.0" }, "engines": { - "node": ">=10.0.0" + "node": ">=10.2.0" } }, "node_modules/@ampproject/remapping": { diff --git a/test/namespaces.ts b/test/namespaces.ts index dd82f4bc3e..8150690b91 100644 --- a/test/namespaces.ts +++ b/test/namespaces.ts @@ -653,6 +653,75 @@ describe("namespaces", () => { io.of(/^\/dynamic-\d+$/); }); + it("should NOT clean up namespace when cleanupEmptyChildNamespaces is OFF and client is rejected in middleware", (done) => { + const io = new Server(0, { cleanupEmptyChildNamespaces: false }); + io.of(/^\/dynamic-\d+$/).use((socket, next) => { + next(new Error("You shall not pass!")); + }); + const c1 = createClient(io, "/dynamic-101"); + + c1.on("connect", () => { + done(new Error("Should not connect")); + }); + + c1.on("connect_error", () => { + setTimeout(() => { + expect(io._nsps.has("/dynamic-101")).to.be(true); + success(done, io); + }, 100); + }); + }); + + it("should clean up namespace when cleanupEmptyChildNamespaces is ON and client is rejected in middleware", (done) => { + const io = new Server(0, { cleanupEmptyChildNamespaces: true }); + io.of(/^\/dynamic-\d+$/).use((socket, next) => { + next(new Error("You shall not pass!")); + }); + const c1 = createClient(io, "/dynamic-101"); + + c1.on("connect", () => { + done(new Error("Should not connect")); + }); + + c1.on("connect_error", () => { + setTimeout(() => { + expect(io._nsps.has("/dynamic-101")).to.be(false); + success(done, io); + }, 100); + }); + }); + + it("should NOT clean up namespace when cleanupEmptyChildNamespaces is ON and client is rejected in middleware but there are other clients connected", (done) => { + const io = new Server(0, { cleanupEmptyChildNamespaces: true }); + let clientIdxToReject = 0; + io.of(/^\/dynamic-\d+$/).use((socket, next) => { + if (clientIdxToReject) { + next(new Error("You shall not pass!")); + } else { + next(); + } + clientIdxToReject++; + }); + const c1 = createClient(io, "/dynamic-101"); + + c1.on("connect", () => { + const c2 = createClient(io, "/dynamic-101"); + c2.on("connect", () => { + done(new Error("Client 2 should not connect")); + }); + c2.on("connect_error", () => { + setTimeout(() => { + expect(io._nsps.has("/dynamic-101")).to.be(true); + c1.off("connect_error"); + success(done, io); + }, 100); + }); + }); + c1.on("connect_error", () => { + done(new Error("Client 1 should not get an error")); + }); + }); + it("should attach a child namespace to its parent upon manual creation", () => { const io = new Server(0); const parentNamespace = io.of(/^\/dynamic-\d+$/); From 4158a9ffec641571bc6791655adae7e840f57abb Mon Sep 17 00:00:00 2001 From: Jaroslav Minarik Date: Thu, 20 Jul 2023 14:53:30 +1000 Subject: [PATCH 2/3] Add connection clean up to tests --- test/namespaces.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/namespaces.ts b/test/namespaces.ts index 8150690b91..4f26ef4520 100644 --- a/test/namespaces.ts +++ b/test/namespaces.ts @@ -667,7 +667,8 @@ describe("namespaces", () => { c1.on("connect_error", () => { setTimeout(() => { expect(io._nsps.has("/dynamic-101")).to.be(true); - success(done, io); + expect(io._nsps.get("/dynamic-101")!.sockets.size).to.be(0); + success(done, io, c1); }, 100); }); }); @@ -686,7 +687,7 @@ describe("namespaces", () => { c1.on("connect_error", () => { setTimeout(() => { expect(io._nsps.has("/dynamic-101")).to.be(false); - success(done, io); + success(done, io, c1); }, 100); }); }); @@ -712,8 +713,8 @@ describe("namespaces", () => { c2.on("connect_error", () => { setTimeout(() => { expect(io._nsps.has("/dynamic-101")).to.be(true); - c1.off("connect_error"); - success(done, io); + expect(io._nsps.get("/dynamic-101")!.sockets.size).to.be(1); + success(done, io, c1, c2); }, 100); }); }); From 066649bbdf4bf9f1fbd357d6ce564a1a61060ef0 Mon Sep 17 00:00:00 2001 From: Jaroslav Minarik Date: Thu, 20 Jul 2023 16:09:28 +1000 Subject: [PATCH 3/3] Move some of the cleaup handlers back to original positions as per PR comment --- lib/socket.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/socket.ts b/lib/socket.ts index 4825c67c1e..f520605344 100644 --- a/lib/socket.ts +++ b/lib/socket.ts @@ -758,6 +758,8 @@ export class Socket< } this._cleanup(); + this.client._remove(this); + this.connected = false; this.emitReserved("disconnect", reason, description); return; } @@ -770,8 +772,6 @@ export class Socket< _cleanup() { this.leaveAll(); this.nsp._remove(this); - this.client._remove(this); - this.connected = false; this.join = noop; }