From 974930fa53831f90578af14e4a9d43610f5465b1 Mon Sep 17 00:00:00 2001
From: Damien Arrachequesne <damien.arrachequesne@gmail.com>
Date: Wed, 25 May 2022 17:51:14 +0200
Subject: [PATCH] fix: prevent the socket from joining a room after
 disconnection

Calling `socket.join()` after disconnection would lead to a memory
leak, because the room was never removed from the memory:

```js
io.on("connection", (socket) => {
  socket.disconnect();
  socket.join("room1"); // leak
});
```

Related:

- https://github.com/socketio/socket.io/issues/4067
- https://github.com/socketio/socket.io/issues/4380
---
 lib/namespace.ts  | 52 +++++++++++++++++++++++++----------------------
 lib/socket.ts     | 14 ++++++++++++-
 test/socket.io.ts | 37 ++++++++++++++++++++++++++++++++-
 3 files changed, 77 insertions(+), 26 deletions(-)

diff --git a/lib/namespace.ts b/lib/namespace.ts
index bec55c1623..b4a6f25249 100644
--- a/lib/namespace.ts
+++ b/lib/namespace.ts
@@ -218,34 +218,38 @@ export class Namespace<
     const socket = new Socket(this, client, query);
     this.run(socket, (err) => {
       process.nextTick(() => {
-        if ("open" == client.conn.readyState) {
-          if (err) {
-            if (client.conn.protocol === 3) {
-              return socket._error(err.data || err.message);
-            } else {
-              return socket._error({
-                message: err.message,
-                data: err.data,
-              });
-            }
+        if ("open" !== client.conn.readyState) {
+          debug("next called after client was closed - ignoring socket");
+          socket._cleanup();
+          return;
+        }
+
+        if (err) {
+          debug("middleware error, sending CONNECT_ERROR packet to the client");
+          socket._cleanup();
+          if (client.conn.protocol === 3) {
+            return socket._error(err.data || err.message);
+          } else {
+            return socket._error({
+              message: err.message,
+              data: err.data,
+            });
           }
+        }
 
-          // track socket
-          this.sockets.set(socket.id, socket);
+        // track socket
+        this.sockets.set(socket.id, socket);
 
-          // it's paramount that the internal `onconnect` logic
-          // fires before user-set events to prevent state order
-          // violations (such as a disconnection before the connection
-          // logic is complete)
-          socket._onconnect();
-          if (fn) fn();
+        // it's paramount that the internal `onconnect` logic
+        // fires before user-set events to prevent state order
+        // violations (such as a disconnection before the connection
+        // logic is complete)
+        socket._onconnect();
+        if (fn) fn();
 
-          // fire user-set events
-          this.emitReserved("connect", socket);
-          this.emitReserved("connection", socket);
-        } else {
-          debug("next called after client was closed - ignoring socket");
-        }
+        // fire user-set events
+        this.emitReserved("connect", socket);
+        this.emitReserved("connection", socket);
       });
     });
     return socket;
diff --git a/lib/socket.ts b/lib/socket.ts
index 583db00a49..cac5c4b581 100644
--- a/lib/socket.ts
+++ b/lib/socket.ts
@@ -112,6 +112,8 @@ export interface Handshake {
  */
 export type Event = [string, ...any[]];
 
+function noop() {}
+
 export class Socket<
   ListenEvents extends EventsMap = DefaultEventsMap,
   EmitEvents extends EventsMap = ListenEvents,
@@ -511,7 +513,7 @@ export class Socket<
     if (!this.connected) return this;
     debug("closing socket - reason %s", reason);
     this.emitReserved("disconnecting", reason);
-    this.leaveAll();
+    this._cleanup();
     this.nsp._remove(this);
     this.client._remove(this);
     this.connected = false;
@@ -519,6 +521,16 @@ export class Socket<
     return;
   }
 
+  /**
+   * Makes the socket leave all the rooms it was part of and prevents it from joining any other room
+   *
+   * @private
+   */
+  _cleanup() {
+    this.leaveAll();
+    this.join = noop;
+  }
+
   /**
    * Produces an `error` packet.
    *
diff --git a/test/socket.io.ts b/test/socket.io.ts
index f595d0dbd5..209b51f1c6 100644
--- a/test/socket.io.ts
+++ b/test/socket.io.ts
@@ -1,6 +1,6 @@
 "use strict";
 
-import { Server, Socket, Namespace } from "../lib";
+import { Server, Socket, Namespace } from "..";
 import { createServer } from "http";
 import fs = require("fs");
 import { join } from "path";
@@ -11,6 +11,7 @@ import type { AddressInfo } from "net";
 import * as io_v2 from "socket.io-client-v2";
 import type { SocketId } from "socket.io-adapter";
 import { io as ioc, Socket as ClientSocket } from "socket.io-client";
+import { createClient } from "./support/util";
 
 import "./support/util";
 import "./utility-methods";
@@ -2109,6 +2110,40 @@ describe("socket.io", () => {
       });
     });
 
+    it("should leave all rooms joined after a middleware failure", (done) => {
+      const io = new Server(0);
+      const client = createClient(io, "/");
+
+      io.use((socket, next) => {
+        socket.join("room1");
+        next(new Error("nope"));
+      });
+
+      client.on("connect_error", () => {
+        expect(io.of("/").adapter.rooms.size).to.eql(0);
+
+        io.close();
+        done();
+      });
+    });
+
+    it("should not join rooms after disconnection", (done) => {
+      const io = new Server(0);
+      const client = createClient(io, "/");
+
+      io.on("connection", (socket) => {
+        socket.disconnect();
+        socket.join("room1");
+      });
+
+      client.on("disconnect", () => {
+        expect(io.of("/").adapter.rooms.size).to.eql(0);
+
+        io.close();
+        done();
+      });
+    });
+
     describe("onAny", () => {
       it("should call listener", (done) => {
         const srv = createServer();