diff --git a/CHANGELOG.md b/CHANGELOG.md index b9cc377c5e..29267b4e50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,23 @@ +# [2.5.0](https://github.com/socketio/socket.io/compare/2.4.1...2.5.0) (2022-06-26) + + +### Bug Fixes + +* fix race condition in dynamic namespaces ([05e1278](https://github.com/socketio/socket.io/commit/05e1278cfa99f3ecf3f8f0531ffe57d850e9a05b)) +* ignore packet received after disconnection ([22d4bdf](https://github.com/socketio/socket.io/commit/22d4bdf00d1a03885dc0171125faddfaef730066)) +* only set 'connected' to true after middleware execution ([226cc16](https://github.com/socketio/socket.io/commit/226cc16165f9fe60f16ff4d295fb91c8971cde35)) +* prevent the socket from joining a room after disconnection ([f223178](https://github.com/socketio/socket.io/commit/f223178eb655a7713303b21a78f9ef9e161d6458)) + + + +## [2.4.1](https://github.com/socketio/socket.io/compare/2.4.0...2.4.1) (2021-01-07) + + +### Reverts + +* fix(security): do not allow all origins by default ([a169050](https://github.com/socketio/socket.io/commit/a1690509470e9dd5559cec4e60908ca6c23e9ba0)) + + # [2.4.0](https://github.com/socketio/socket.io/compare/2.3.0...2.4.0) (2021-01-04) diff --git a/lib/client.js b/lib/client.js index 32d179f971..b1f9a22a1c 100644 --- a/lib/client.js +++ b/lib/client.js @@ -68,7 +68,6 @@ Client.prototype.connect = function(name, query){ this.server.checkNamespace(name, query, (dynamicNsp) => { if (dynamicNsp) { - debug('dynamic namespace %s was created', dynamicNsp.name); this.doConnect(name, query); } else { debug('creation of namespace %s was denied', name); diff --git a/lib/index.js b/lib/index.js index 3cae6c10de..bf0d01e21d 100644 --- a/lib/index.js +++ b/lib/index.js @@ -54,7 +54,7 @@ function Server(srv, opts){ this.parser = opts.parser || parser; this.encoder = new this.parser.Encoder(); this.adapter(opts.adapter || Adapter); - this.origins(opts.origins || []); + this.origins(opts.origins || '*:*'); this.sockets = this.of('/'); if (srv) this.attach(srv, opts); } @@ -67,18 +67,31 @@ function Server(srv, opts){ */ Server.prototype.checkRequest = function(req, fn) { - const origin = req.headers.origin; + var origin = req.headers.origin || req.headers.referer; - if (typeof this._origins === 'function') { - return this._origins(origin, fn); - } + // file:// URLs produce a null Origin which can't be authorized via echo-back + if ('null' == origin || null == origin) origin = '*'; + if (!!origin && typeof(this._origins) == 'function') return this._origins(origin, fn); + if (this._origins.indexOf('*:*') !== -1) return fn(null, true); if (origin) { - fn(null, this._origins.includes(origin)); - } else { - const noOriginIsValid = this._origins.length === 0; - fn(null, noOriginIsValid); + try { + var parts = url.parse(origin); + var defaultPort = 'https:' == parts.protocol ? 443 : 80; + parts.port = parts.port != null + ? parts.port + : defaultPort; + var ok = + ~this._origins.indexOf(parts.protocol + '//' + parts.hostname + ':' + parts.port) || + ~this._origins.indexOf(parts.hostname + ':' + parts.port) || + ~this._origins.indexOf(parts.hostname + ':*') || + ~this._origins.indexOf('*:' + parts.port); + debug('origin %s is %svalid', origin, !!ok ? '' : 'not '); + return fn(null, !!ok); + } catch (ex) { + } } + fn(null, false); }; /** @@ -169,11 +182,17 @@ Server.prototype.checkNamespace = function(name, query, fn){ return fn(false); } nextFn.value(name, query, (err, allow) => { - if (err || !allow) { - run(); - } else { - fn(this.parentNsps.get(nextFn.value).createChild(name)); + if (err || !allow) { + return run(); + } + if (this.nsps[name]) { + // the namespace was created in the meantime + debug("dynamic namespace %s already exists", name); + return fn(this.nsps[name]); } + const namespace = this.parentNsps.get(nextFn.value).createChild(name); + debug("dynamic namespace %s was created", name); + fn(namespace); }); }; @@ -224,7 +243,7 @@ Server.prototype.adapter = function(v){ Server.prototype.origins = function(v){ if (!arguments.length) return this._origins; - this._origins = typeof v === 'string' ? [v] : v; + this._origins = v; return this; }; 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 4e97eb55dd..bfdd77e856 100644 --- a/lib/socket.js +++ b/lib/socket.js @@ -66,8 +66,8 @@ function Socket(nsp, client, query){ this.conn = client.conn; this.rooms = {}; this.acks = {}; - this.connected = true; - this.disconnected = false; + this.connected = false; + this.disconnected = true; this.handshake = this.buildHandshake(query); this.fns = []; this.flags = {}; @@ -300,6 +300,8 @@ Socket.prototype.leaveAll = function(){ Socket.prototype.onconnect = function(){ debug('socket connected - writing packet'); + this.connected = true; + this.disconnected = false; this.nsp.connected[this.id] = this; this.join(this.id); var skip = this.nsp.name === '/' && this.nsp.fns.length === 0; @@ -445,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; @@ -525,7 +527,11 @@ Socket.prototype.dispatch = function(event){ if (err) { return self.error(err.data || err.message); } - emit.apply(self, event); + if (self.connected) { + emit.apply(self, event); + } else { + debug("ignore packet received after disconnection"); + } }); } this.run(event, dispatchSocket); @@ -570,3 +576,8 @@ Socket.prototype.run = function(event, fn){ run(0); }; + +Socket.prototype._cleanup = function () { + this.leaveAll(); + this.join = function noop() {}; +} diff --git a/package-lock.json b/package-lock.json index 940796ad51..4a84fed5ce 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "socket.io", - "version": "2.4.0", + "version": "2.4.1", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -32,7 +32,7 @@ "backo2": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/backo2/-/backo2-1.0.2.tgz", - "integrity": "sha1-MasayLEpNjRj41s+u2n038+6eUc=" + "integrity": "sha512-zj6Z6M7Eq+PBZ7PQxl5NT665MvJdAkzp0f60nAJ+sLaSCBPMwVak5ZegFbgVCzFcCJTKFoMizvM5Ld7+JrRJHA==" }, "balanced-match": { "version": "1.0.0", @@ -92,7 +92,7 @@ "component-bind": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/component-bind/-/component-bind-1.0.0.tgz", - "integrity": "sha1-AMYIq33Nk4l8AAllGx06jh5zu9E=" + "integrity": "sha512-WZveuKPeKAG9qY+FkYDeADzdHyTYdIboXS59ixDeRJL5ZhxpqUnxSOwop4FQjMsiYm3/Or8cegVbpAHNA7pHxw==" }, "component-emitter": { "version": "1.2.1", @@ -102,7 +102,7 @@ "component-inherit": { "version": "0.0.3", "resolved": "https://registry.npmjs.org/component-inherit/-/component-inherit-0.0.3.tgz", - "integrity": "sha1-ZF/ErfWLcrZJ1crmUTVhnbJv8UM=" + "integrity": "sha512-w+LhYREhatpVqTESyGFg3NlP6Iu0kEKUHETY9GoZP/pQyW4mHFZuFWRUCIqVPZ36ueVLtoOEZaAqbCF2RDndaA==" }, "concat-map": { "version": "0.0.1", @@ -148,9 +148,9 @@ "dev": true }, "engine.io": { - "version": "3.5.0", - "resolved": "https://registry.npmjs.org/engine.io/-/engine.io-3.5.0.tgz", - "integrity": "sha512-21HlvPUKaitDGE4GXNtQ7PLP0Sz4aWLddMPw2VTyFz1FVZqu/kZsJUO8WNpKuE/OCL7nkfRaOui2ZCJloGznGA==", + "version": "3.6.0", + "resolved": "https://registry.npmjs.org/engine.io/-/engine.io-3.6.0.tgz", + "integrity": "sha512-Kc8fo5bbg8F4a2f3HPHTEpGyq/IRIQpyeHu3H1ThR14XDD7VrLcsGBo16HUpahgp8YkHJDaU5gNxJZbuGcuueg==", "requires": { "accepts": "~1.3.4", "base64id": "2.0.0", @@ -161,9 +161,9 @@ } }, "engine.io-client": { - "version": "3.5.0", - "resolved": "https://registry.npmjs.org/engine.io-client/-/engine.io-client-3.5.0.tgz", - "integrity": "sha512-12wPRfMrugVw/DNyJk34GQ5vIVArEcVMXWugQGGuw2XxUSztFNmJggZmv8IZlLyEdnpO1QB9LkcjeWewO2vxtA==", + "version": "3.5.2", + "resolved": "https://registry.npmjs.org/engine.io-client/-/engine.io-client-3.5.2.tgz", + "integrity": "sha512-QEqIp+gJ/kMHeUun7f5Vv3bteRHppHH/FMBQX/esFj/fuYfjyUKWGMo3VCvIP/V8bE9KcjHmRZrhIz2Z9oNsDA==", "requires": { "component-emitter": "~1.3.0", "component-inherit": "0.0.3", @@ -174,7 +174,7 @@ "parseqs": "0.0.6", "parseuri": "0.0.6", "ws": "~7.4.2", - "xmlhttprequest-ssl": "~1.5.4", + "xmlhttprequest-ssl": "~1.6.2", "yeast": "0.1.2" }, "dependencies": { @@ -194,7 +194,7 @@ "ms": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", - "integrity": "sha1-VgiurfwAvmwpAd9fmGF4jeDVl8g=" + "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==" } } }, @@ -274,7 +274,7 @@ "has-cors": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/has-cors/-/has-cors-1.1.0.tgz", - "integrity": "sha1-XkdHk/fqmEPRu5nCPu9J/xJv/zk=" + "integrity": "sha512-g5VNKdkFuUuVCP9gYfDJHjK2nqdQJ7aDLTnycnc2+RvsOQbuLdF5pm7vuE5J76SEBIQjs4kQY/BWq74JUmjbXA==" }, "he": { "version": "1.1.1", @@ -285,7 +285,7 @@ "indexof": { "version": "0.0.1", "resolved": "https://registry.npmjs.org/indexof/-/indexof-0.0.1.tgz", - "integrity": "sha1-gtwzbSMrkGIXnQWrMpOmYFn9Q10=" + "integrity": "sha512-i0G7hLJ1z0DE8dsqJa2rycj9dBmNKgXBvotXtZYXakU9oivfB9Uj2ZBC27qqef2U58/ZLwalxa1X/RDCdkHtVg==" }, "inflight": { "version": "1.0.6", @@ -3211,9 +3211,9 @@ "integrity": "sha512-WzZRUj1kUjrTIrUKpZLEzFZ1OLj5FwLlAFQs9kuZJzJi5DKdU7FsWc36SNmA8iDOtwBQyT8FkrriRM8vXLYz8g==" }, "socket.io-client": { - "version": "2.4.0", - "resolved": "https://registry.npmjs.org/socket.io-client/-/socket.io-client-2.4.0.tgz", - "integrity": "sha512-M6xhnKQHuuZd4Ba9vltCLT9oa+YvTsP8j9NcEiLElfIg8KeYPyhWOes6x4t+LTAC8enQbE/995AdTem2uNyKKQ==", + "version": "2.5.0", + "resolved": "https://registry.npmjs.org/socket.io-client/-/socket.io-client-2.5.0.tgz", + "integrity": "sha512-lOO9clmdgssDykiOmVQQitwBAF3I6mYcQAo7hQ7AM6Ny5X7fp8hIJ3HcQs3Rjz4SoggoxA1OgrQyY8EgTbcPYw==", "requires": { "backo2": "1.0.2", "component-bind": "1.0.0", @@ -3244,12 +3244,12 @@ "ms": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", - "integrity": "sha1-VgiurfwAvmwpAd9fmGF4jeDVl8g=" + "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==" }, "socket.io-parser": { - "version": "3.3.1", - "resolved": "https://registry.npmjs.org/socket.io-parser/-/socket.io-parser-3.3.1.tgz", - "integrity": "sha512-1QLvVAe8dTz+mKmZ07Swxt+LAo4Y1ff50rlyoEx00TQmDFVQYPfcqGvIDJLGaBdhdNCecXtyKpD+EgKGcmmbuQ==", + "version": "3.3.2", + "resolved": "https://registry.npmjs.org/socket.io-parser/-/socket.io-parser-3.3.2.tgz", + "integrity": "sha512-FJvDBuOALxdCI9qwRrO/Rfp9yfndRtc1jSgVgV8FDraihmSP/MLGD5PEuJrNfjALvcQ+vMDM/33AWOYP/JSjDg==", "requires": { "component-emitter": "~1.3.0", "debug": "~3.1.0", @@ -3319,7 +3319,7 @@ "to-array": { "version": "0.1.4", "resolved": "https://registry.npmjs.org/to-array/-/to-array-0.1.4.tgz", - "integrity": "sha1-F+bBH3PdTz10zaek/zI46a2b+JA=" + "integrity": "sha512-LhVdShQD/4Mk4zXNroIQZJC+Ap3zgLcDuwEdcmLv9CCO73NWockQDwyUnW/m8VX/EElfL6FcYx7EeutN4HJA6A==" }, "util-deprecate": { "version": "1.0.2", @@ -3339,14 +3339,14 @@ "integrity": "sha512-T4tewALS3+qsrpGI/8dqNMLIVdq/g/85U98HPMa6F0m6xTbvhXU6RCQLqPH3+SlomNV/LdY6RXEbBpMH6EOJnA==" }, "xmlhttprequest-ssl": { - "version": "1.5.5", - "resolved": "https://registry.npmjs.org/xmlhttprequest-ssl/-/xmlhttprequest-ssl-1.5.5.tgz", - "integrity": "sha1-wodrBhaKrcQOV9l+gRkayPQ5iz4=" + "version": "1.6.3", + "resolved": "https://registry.npmjs.org/xmlhttprequest-ssl/-/xmlhttprequest-ssl-1.6.3.tgz", + "integrity": "sha512-3XfeQE/wNkvrIktn2Kf0869fC0BN6UpydVasGIeSm2B1Llihf7/0UfZM+eCkOw3P7bP4+qPgqhm7ZoxuJtFU0Q==" }, "yeast": { "version": "0.1.2", "resolved": "https://registry.npmjs.org/yeast/-/yeast-0.1.2.tgz", - "integrity": "sha1-AI4G2AlDIMNy28L47XagymyKxBk=" + "integrity": "sha512-8HFIh676uyGYP6wP13R/j6OJ/1HwJ46snpvzE7aHAN3Ryqh2yX6Xox2B4CUmTwwOIzlG3Bs7ocsP5dZH/R1Qbg==" } } } diff --git a/package.json b/package.json index 6309ee7aad..975856394c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "socket.io", - "version": "2.4.0", + "version": "2.5.0", "description": "node.js realtime framework server", "keywords": [ "realtime", @@ -25,10 +25,10 @@ }, "dependencies": { "debug": "~4.1.0", - "engine.io": "~3.5.0", + "engine.io": "~3.6.0", "has-binary2": "~1.0.2", "socket.io-adapter": "~1.1.0", - "socket.io-client": "2.4.0", + "socket.io-client": "2.5.0", "socket.io-parser": "~3.4.0" }, "devDependencies": { diff --git a/test/socket.io.js b/test/socket.io.js index 7252bb78e7..7fe059d4a9 100644 --- a/test/socket.io.js +++ b/test/socket.io.js @@ -73,7 +73,7 @@ describe('socket.io', function(){ it('should be able to set origins to engine.io', function() { var srv = io(http()); srv.set('origins', 'http://hostname.com:*'); - expect(srv.origins()).to.eql(['http://hostname.com:*']); + expect(srv.origins()).to.be('http://hostname.com:*'); }); it('should be able to set authorization and send error packet', function(done) { @@ -262,6 +262,17 @@ describe('socket.io', function(){ }); }); + it('should allow request when origin defined an the same is specified', function(done) { + var sockets = io({ origins: 'http://foo.example:*' }).listen('54015'); + request.get('http://localhost:54015/socket.io/default/') + .set('origin', 'http://foo.example') + .query({ transport: 'polling' }) + .end(function (err, res) { + expect(res.status).to.be(200); + done(); + }); + }); + it('should allow request when origin defined as function and same is supplied', function(done) { var sockets = io({ origins: function(origin,callback){ if (origin == 'http://foo.example') { @@ -296,7 +307,7 @@ describe('socket.io', function(){ it('should allow request when origin defined as function and no origin is supplied', function(done) { var sockets = io({ origins: function(origin,callback){ - if (origin === undefined) { + if (origin == '*') { return callback(null, true); } return callback(null, false); @@ -309,6 +320,17 @@ describe('socket.io', function(){ }); }); + it('should default to port 443 when protocol is https', function(done) { + var sockets = io({ origins: 'https://foo.example:443' }).listen('54036'); + request.get('http://localhost:54036/socket.io/default/') + .set('origin', 'https://foo.example') + .query({ transport: 'polling' }) + .end(function (err, res) { + expect(res.status).to.be(200); + done(); + }); + }); + it('should allow request if custom function in opts.allowRequest returns true', function(done){ var sockets = io(http().listen(54022), { allowRequest: function (req, callback) { return callback(null, true); @@ -345,17 +367,6 @@ describe('socket.io', function(){ done(); }); }); - - it('should disallow any origin by default', (done) => { - io().listen('54025'); - request.get('http://localhost:54025/socket.io/default/') - .set('origin', 'https://foo.example') - .query({ transport: 'polling' }) - .end((err, res) => { - expect(res.status).to.be(403); - done(); - }); - }); }); describe('close', function(){ @@ -934,6 +945,42 @@ describe('socket.io', function(){ }); }); }); + + it("should handle race conditions with dynamic namespaces (#4136)", (done) => { + const srv = http(); + const sio = io(srv); + const counters = { + connected: 0, + created: 0, + events: 0, + }; + const buffer = []; + srv.listen(() => { + const handler = () => { + if (++counters.events === 2) { + done(); + } + }; + + sio + .of((name, query, next) => { + buffer.push(next); + if (buffer.length === 2) { + buffer.forEach((next) => next(null, true)); + } + }) + .on("connection", (socket) => { + if (++counters.connected === 2) { + sio.of("/dynamic-101").emit("message"); + } + }); + + let one = client(srv, "/dynamic-101"); + let two = client(srv, "/dynamic-101"); + one.on("message", handler); + two.on("message", handler); + }); + }); }); }); @@ -1827,6 +1874,70 @@ describe('socket.io', function(){ }); }); + it("should ignore a packet received after disconnection", (done) => { + const srv = http(); + const sio = io(srv); + + srv.listen(() => { + const clientSocket = client(srv); + + const success = () => { + clientSocket.close(); + sio.close(); + done(); + }; + + sio.on("connection", (socket) => { + socket.on("test", () => { + done(new Error("should not happen")); + }); + socket.on("disconnect", success); + }); + + clientSocket.on("connect", () => { + clientSocket.emit("test", Buffer.alloc(10)); + clientSocket.disconnect(); + }); + }); + }); + + 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); @@ -2368,6 +2479,25 @@ describe('socket.io', function(){ if (++count === 2) done(); }); }); + + it("should only set `connected` to true after the middleware execution", (done) => { + const httpServer = http(); + const sio = io(httpServer); + + const clientSocket = client(httpServer, "/"); + + sio.use((socket, next) => { + expect(socket.connected).to.be(false); + expect(socket.disconnected).to.be(true); + next(); + }); + + sio.on("connection", (socket) => { + expect(socket.connected).to.be(true); + expect(socket.disconnected).to.be(false); + done(); + }); + }); }); describe('socket middleware', function(done){