From f223178eb655a7713303b21a78f9ef9e161d6458 Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne Date: Sun, 26 Jun 2022 08:54:51 +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 Backported from https://github.com/socketio/socket.io/commit/18f3fdab12947a9fee3e9c37cfc1da97027d1473 --- lib/namespace.js | 42 ++++++++++++++++++++++++------------------ lib/socket.js | 7 ++++++- test/socket.io.js | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 19 deletions(-) diff --git a/lib/namespace.js b/lib/namespace.js index bca8a3e99f..7e034bdb47 100644 --- a/lib/namespace.js +++ b/lib/namespace.js @@ -163,25 +163,31 @@ Namespace.prototype.add = function(client, query, fn){ var self = this; this.run(socket, function(err){ process.nextTick(function(){ - if ('open' == client.conn.readyState) { - if (err) return socket.error(err.data || err.message); - - // track socket - self.sockets[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(); - - // fire user-set events - self.emit('connect', socket); - self.emit('connection', socket); - } else { - debug('next called after client was closed - ignoring socket'); + 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(); + return socket.error(err.data || err.message); + } + + // track socket + self.sockets[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(); + + // fire user-set events + self.emit('connect', socket); + self.emit('connection', socket); }); }); return socket; diff --git a/lib/socket.js b/lib/socket.js index dacaa440ca..bfdd77e856 100644 --- a/lib/socket.js +++ b/lib/socket.js @@ -447,7 +447,7 @@ Socket.prototype.onclose = function(reason){ if (!this.connected) return this; debug('closing socket - reason %s', reason); this.emit('disconnecting', reason); - this.leaveAll(); + this._cleanup(); this.nsp.remove(this); this.client.remove(this); this.connected = false; @@ -576,3 +576,8 @@ Socket.prototype.run = function(event, fn){ run(0); }; + +Socket.prototype._cleanup = function () { + this.leaveAll(); + this.join = function noop() {}; +} diff --git a/test/socket.io.js b/test/socket.io.js index aa786495a0..7fe059d4a9 100644 --- a/test/socket.io.js +++ b/test/socket.io.js @@ -1901,6 +1901,43 @@ describe('socket.io', function(){ }); }); + it("should leave all rooms joined after a middleware failure", (done) => { + const srv = http().listen(0); + const sio = io(srv); + const clientSocket = client(srv, "/"); + + sio.use((socket, next) => { + socket.join("room1"); + next(new Error("nope")); + }); + + clientSocket.on("error", () => { + expect(sio.of("/").adapter.rooms).to.eql(0); + + clientSocket.disconnect(); + sio.close(); + done(); + }); + }); + + it("should not join rooms after disconnection", (done) => { + const srv = http().listen(0); + const sio = io(srv); + const clientSocket = client(srv, "/"); + + sio.on("connection", (socket) => { + socket.disconnect(); + socket.join("room1"); + }); + + clientSocket.on("disconnect", () => { + expect(sio.of("/").adapter.rooms).to.eql(0); + + sio.close(); + done(); + }); + }); + it('should always trigger the callback (if provided) when joining a room', function(done){ var srv = http(); var sio = io(srv);