From 3289f7ec376e9ec88c2f90e2735c8ca8d01c0e97 Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne Date: Wed, 7 Oct 2020 15:54:34 +0200 Subject: [PATCH] feat: remove the implicit connection to the default namespace In previous versions, a client was always connected to the default namespace, even if it requested access to another namespace. This meant that the middlewares registered for the default namespace were triggered in any case, which is a surprising behavior for end users. This also meant that the query option of the Socket on the client-side was not sent in the Socket.IO CONNECT packet for the default namespace: ```js // default namespace: query sent in the query params const socket = io({ query: { abc: "def" } }); // another namespace: query sent in the query params + the CONNECT packet const socket = io("/admin", { query: { abc: "def" } }); ``` The client will now send a CONNECT packet in any case, and the query option of the Socket is renamed to "auth", in order to make a clear distinction with the query option of the Manager (included in the query parameters of the HTTP requests). ```js // server-side io.use((socket, next) => { // not triggered anymore }); io.of("/admin").use((socket, next => { // triggered console.log(socket.handshake.query.abc); // "def" console.log(socket.handshake.auth.abc); // "123" }); // client-side const socket = io("/admin", { query: { abc: "def" }, auth: { abc: "123" } }); ``` --- dist/client.d.ts | 11 ++++--- dist/client.js | 35 +++++++--------------- dist/index.d.ts | 4 +-- dist/index.js | 18 +++--------- dist/namespace.js | 4 --- dist/socket.d.ts | 8 +++-- dist/socket.js | 25 +++++----------- lib/client.ts | 41 +++++++++----------------- lib/index.ts | 21 ++++---------- lib/namespace.ts | 4 --- lib/socket.ts | 30 ++++++++----------- package.json | 6 ++-- test/socket.io.ts | 74 +++++++++++------------------------------------ 13 files changed, 85 insertions(+), 196 deletions(-) diff --git a/dist/client.d.ts b/dist/client.d.ts index 44c0542f35..2a42798592 100644 --- a/dist/client.d.ts +++ b/dist/client.d.ts @@ -11,7 +11,6 @@ export declare class Client { private readonly decoder; private sockets; private nsps; - private connectBuffer; /** * Client constructor. * @@ -31,16 +30,16 @@ export declare class Client { /** * Connects a client to a namespace. * - * @param {String} name namespace - * @param {Object} query the query parameters + * @param {String} name - the namespace + * @param {Object} auth - the auth parameters * @package */ - connect(name: any, query?: {}): void; + connect(name: string, auth?: object): void; /** * Connects a client to a namespace. * - * @param {String} name namespace - * @param {String} query the query parameters + * @param {String} name - the namespace + * @param {Object} auth - the auth parameters */ private doConnect; /** diff --git a/dist/client.js b/dist/client.js index c2293f716e..a37aebb0ca 100644 --- a/dist/client.js +++ b/dist/client.js @@ -1,11 +1,7 @@ "use strict"; -var __importDefault = (this && this.__importDefault) || function (mod) { - return (mod && mod.__esModule) ? mod : { "default": mod }; -}; Object.defineProperty(exports, "__esModule", { value: true }); exports.Client = void 0; const socket_io_parser_1 = require("socket.io-parser"); -const url_1 = __importDefault(require("url")); const debugModule = require("debug"); const debug = debugModule("socket.io:client"); class Client { @@ -19,7 +15,6 @@ class Client { constructor(server, conn) { this.sockets = new Map(); this.nsps = new Map(); - this.connectBuffer = []; this.server = server; this.conn = conn; this.encoder = server.encoder; @@ -50,19 +45,19 @@ class Client { /** * Connects a client to a namespace. * - * @param {String} name namespace - * @param {Object} query the query parameters + * @param {String} name - the namespace + * @param {Object} auth - the auth parameters * @package */ - connect(name, query = {}) { + connect(name, auth = {}) { if (this.server.nsps.has(name)) { debug("connecting to namespace %s", name); - return this.doConnect(name, query); + return this.doConnect(name, auth); } - this.server.checkNamespace(name, query, dynamicNsp => { + this.server.checkNamespace(name, auth, dynamicNsp => { if (dynamicNsp) { debug("dynamic namespace %s was created", dynamicNsp.name); - this.doConnect(name, query); + this.doConnect(name, auth); } else { debug("creation of namespace %s was denied", name); @@ -77,22 +72,14 @@ class Client { /** * Connects a client to a namespace. * - * @param {String} name namespace - * @param {String} query the query parameters + * @param {String} name - the namespace + * @param {Object} auth - the auth parameters */ - doConnect(name, query) { + doConnect(name, auth) { const nsp = this.server.of(name); - if ("/" != name && !this.nsps.has("/")) { - this.connectBuffer.push(name); - return; - } - const socket = nsp.add(this, query, () => { + const socket = nsp.add(this, auth, () => { this.sockets.set(socket.id, socket); this.nsps.set(nsp.name, socket); - if ("/" == nsp.name && this.connectBuffer.length > 0) { - this.connectBuffer.forEach(this.connect, this); - this.connectBuffer = []; - } }); } /** @@ -182,7 +169,7 @@ class Client { */ ondecoded(packet) { if (socket_io_parser_1.PacketType.CONNECT == packet.type) { - this.connect(url_1.default.parse(packet.nsp).pathname, url_1.default.parse(packet.nsp, true).query); + this.connect(packet.nsp, packet.data); } else { const socket = this.nsps.get(packet.nsp); diff --git a/dist/index.d.ts b/dist/index.d.ts index 7d6f439af0..e6b3f42660 100644 --- a/dist/index.d.ts +++ b/dist/index.d.ts @@ -149,12 +149,12 @@ declare class Server extends EventEmitter { * Executes the middleware for an incoming namespace not already created on the server. * * @param {String} name - name of incoming namespace - * @param {Object} query - the query parameters + * @param {Object} auth - the auth parameters * @param {Function} fn - callback * * @package */ - checkNamespace(name: string, query: object, fn: (nsp: Namespace | boolean) => void): void; + checkNamespace(name: string, auth: object, fn: (nsp: Namespace | boolean) => void): void; /** * Sets the client serving path. * diff --git a/dist/index.js b/dist/index.js index b50eaae1bb..a4c69b2ac6 100644 --- a/dist/index.js +++ b/dist/index.js @@ -39,7 +39,6 @@ const parent_namespace_1 = require("./parent-namespace"); Object.defineProperty(exports, "ParentNamespace", { enumerable: true, get: function () { return parent_namespace_1.ParentNamespace; } }); const socket_io_adapter_1 = require("socket.io-adapter"); const parser = __importStar(require("socket.io-parser")); -const socket_io_parser_1 = require("socket.io-parser"); const url_1 = __importDefault(require("url")); const debug_1 = __importDefault(require("debug")); const debug = debug_1.default("socket.io:server"); @@ -131,12 +130,12 @@ class Server extends events_1.EventEmitter { * Executes the middleware for an incoming namespace not already created on the server. * * @param {String} name - name of incoming namespace - * @param {Object} query - the query parameters + * @param {Object} auth - the auth parameters * @param {Function} fn - callback * * @package */ - checkNamespace(name, query, fn) { + checkNamespace(name, auth, fn) { if (this.parentNsps.size === 0) return fn(false); const keysIterator = this.parentNsps.keys(); @@ -145,7 +144,7 @@ class Server extends events_1.EventEmitter { if (nextFn.done) { return fn(false); } - nextFn.value(name, query, (err, allow) => { + nextFn.value(name, auth, (err, allow) => { if (err || !allow) { run(); } @@ -221,14 +220,6 @@ class Server extends events_1.EventEmitter { opts.path = opts.path || this._path; // set origins verification opts.allowRequest = opts.allowRequest || this.checkRequest.bind(this); - if (this.sockets.fns.length > 0) { - this.initEngine(srv, opts); - return this; - } - const connectPacket = { type: socket_io_parser_1.PacketType.CONNECT, nsp: "/" }; - // the CONNECT packet will be merged with Engine.IO handshake, - // to reduce the number of round trips - opts.initialPacket = this.encoder.encode(connectPacket); this.initEngine(srv, opts); return this; } @@ -346,8 +337,7 @@ class Server extends events_1.EventEmitter { */ onconnection(conn) { debug("incoming connection with id %s", conn.id); - const client = new client_1.Client(this, conn); - client.connect("/"); + new client_1.Client(this, conn); return this; } /** diff --git a/dist/namespace.js b/dist/namespace.js index 92aba536a8..4cfc3de015 100644 --- a/dist/namespace.js +++ b/dist/namespace.js @@ -58,10 +58,6 @@ class Namespace extends events_1.EventEmitter { * @return {Namespace} self */ use(fn) { - if (this.server.eio && this.name === "/") { - debug("removing initial packet"); - delete this.server.eio.opts.initialPacket; - } this.fns.push(fn); return this; } diff --git a/dist/socket.d.ts b/dist/socket.d.ts index 45fd9c94ce..858951ec54 100644 --- a/dist/socket.d.ts +++ b/dist/socket.d.ts @@ -39,6 +39,10 @@ export interface Handshake { * The query object */ query: object; + /** + * The auth object + */ + auth: object; } export declare class Socket extends EventEmitter { readonly nsp: Namespace; @@ -58,10 +62,10 @@ export declare class Socket extends EventEmitter { * * @param {Namespace} nsp * @param {Client} client - * @param {Object} query + * @param {Object} auth * @package */ - constructor(nsp: Namespace, client: Client, query: any); + constructor(nsp: Namespace, client: Client, auth: object); /** * Builds the `handshake` BC object */ diff --git a/dist/socket.js b/dist/socket.js index dac931a3b6..3ce0ab7201 100644 --- a/dist/socket.js +++ b/dist/socket.js @@ -27,10 +27,10 @@ class Socket extends events_1.EventEmitter { * * @param {Namespace} nsp * @param {Client} client - * @param {Object} query + * @param {Object} auth * @package */ - constructor(nsp, client, query) { + constructor(nsp, client, auth) { super(); this.nsp = nsp; this.client = client; @@ -43,18 +43,12 @@ class Socket extends events_1.EventEmitter { this.id = nsp.name !== "/" ? nsp.name + "#" + client.id : client.id; this.connected = true; this.disconnected = false; - this.handshake = this.buildHandshake(query); + this.handshake = this.buildHandshake(auth); } /** * Builds the `handshake` BC object */ - buildHandshake(query) { - const self = this; - function buildQuery() { - const requestQuery = url_1.default.parse(self.request.url, true).query; - //if socket-specific query exist, replace query strings in requestQuery - return Object.assign({}, query, requestQuery); - } + buildHandshake(auth) { return { headers: this.request.headers, time: new Date() + "", @@ -64,7 +58,8 @@ class Socket extends events_1.EventEmitter { secure: !!this.request.connection.encrypted, issued: +new Date(), url: this.request.url, - query: buildQuery() + query: url_1.default.parse(this.request.url, true).query, + auth }; } /** @@ -211,13 +206,7 @@ class Socket extends events_1.EventEmitter { debug("socket connected - writing packet"); this.nsp.connected.set(this.id, this); this.join(this.id); - const skip = this.nsp.name === "/" && this.nsp.fns.length === 0; - if (skip) { - debug("packet already sent in initial handshake"); - } - else { - this.packet({ type: socket_io_parser_1.PacketType.CONNECT }); - } + this.packet({ type: socket_io_parser_1.PacketType.CONNECT }); } /** * Called with each packet. Called by `Client`. diff --git a/lib/client.ts b/lib/client.ts index 4d250a1f8f..6157559d94 100644 --- a/lib/client.ts +++ b/lib/client.ts @@ -1,5 +1,4 @@ -import { Decoder, Encoder, PacketType } from "socket.io-parser"; -import url from "url"; +import { Decoder, Encoder, Packet, PacketType } from "socket.io-parser"; import debugModule = require("debug"); import { IncomingMessage } from "http"; import { Server } from "./index"; @@ -18,7 +17,6 @@ export class Client { private readonly decoder: Decoder; private sockets: Map = new Map(); private nsps: Map = new Map(); - private connectBuffer: Array = []; /** * Client constructor. @@ -62,20 +60,20 @@ export class Client { /** * Connects a client to a namespace. * - * @param {String} name namespace - * @param {Object} query the query parameters + * @param {String} name - the namespace + * @param {Object} auth - the auth parameters * @package */ - public connect(name, query = {}) { + public connect(name: string, auth: object = {}) { if (this.server.nsps.has(name)) { debug("connecting to namespace %s", name); - return this.doConnect(name, query); + return this.doConnect(name, auth); } - this.server.checkNamespace(name, query, dynamicNsp => { + this.server.checkNamespace(name, auth, dynamicNsp => { if (dynamicNsp) { debug("dynamic namespace %s was created", dynamicNsp.name); - this.doConnect(name, query); + this.doConnect(name, auth); } else { debug("creation of namespace %s was denied", name); this.packet({ @@ -90,25 +88,15 @@ export class Client { /** * Connects a client to a namespace. * - * @param {String} name namespace - * @param {String} query the query parameters + * @param {String} name - the namespace + * @param {Object} auth - the auth parameters */ - private doConnect(name, query) { + private doConnect(name: string, auth: object) { const nsp = this.server.of(name); - if ("/" != name && !this.nsps.has("/")) { - this.connectBuffer.push(name); - return; - } - - const socket = nsp.add(this, query, () => { + const socket = nsp.add(this, auth, () => { this.sockets.set(socket.id, socket); this.nsps.set(nsp.name, socket); - - if ("/" == nsp.name && this.connectBuffer.length > 0) { - this.connectBuffer.forEach(this.connect, this); - this.connectBuffer = []; - } }); } @@ -199,12 +187,9 @@ export class Client { /** * Called when parser fully decodes a packet. */ - private ondecoded(packet) { + private ondecoded(packet: Packet) { if (PacketType.CONNECT == packet.type) { - this.connect( - url.parse(packet.nsp).pathname, - url.parse(packet.nsp, true).query - ); + this.connect(packet.nsp, packet.data); } else { const socket = this.nsps.get(packet.nsp); if (socket) { diff --git a/lib/index.ts b/lib/index.ts index 3e1f8aeaac..c5065fb872 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -8,7 +8,7 @@ import { Namespace } from "./namespace"; import { ParentNamespace } from "./parent-namespace"; import { Adapter } from "socket.io-adapter"; import * as parser from "socket.io-parser"; -import { Encoder, PacketType } from "socket.io-parser"; +import { Encoder } from "socket.io-parser"; import url from "url"; import debugModule from "debug"; import { Socket } from "./socket"; @@ -258,14 +258,14 @@ class Server extends EventEmitter { * Executes the middleware for an incoming namespace not already created on the server. * * @param {String} name - name of incoming namespace - * @param {Object} query - the query parameters + * @param {Object} auth - the auth parameters * @param {Function} fn - callback * * @package */ public checkNamespace( name: string, - query: object, + auth: object, fn: (nsp: Namespace | boolean) => void ) { if (this.parentNsps.size === 0) return fn(false); @@ -277,7 +277,7 @@ class Server extends EventEmitter { if (nextFn.done) { return fn(false); } - nextFn.value(name, query, (err, allow) => { + nextFn.value(name, auth, (err, allow) => { if (err || !allow) { run(); } else { @@ -379,16 +379,6 @@ class Server extends EventEmitter { // set origins verification opts.allowRequest = opts.allowRequest || this.checkRequest.bind(this); - if (this.sockets.fns.length > 0) { - this.initEngine(srv, opts); - return this; - } - - const connectPacket = { type: PacketType.CONNECT, nsp: "/" }; - // the CONNECT packet will be merged with Engine.IO handshake, - // to reduce the number of round trips - opts.initialPacket = this.encoder.encode(connectPacket); - this.initEngine(srv, opts); return this; @@ -517,8 +507,7 @@ class Server extends EventEmitter { */ private onconnection(conn): Server { debug("incoming connection with id %s", conn.id); - const client = new Client(this, conn); - client.connect("/"); + new Client(this, conn); return this; } diff --git a/lib/namespace.ts b/lib/namespace.ts index feb87d6fad..6b1870ad36 100644 --- a/lib/namespace.ts +++ b/lib/namespace.ts @@ -70,10 +70,6 @@ export class Namespace extends EventEmitter { public use( fn: (socket: Socket, next: (err: Error) => void) => void ): Namespace { - if (this.server.eio && this.name === "/") { - debug("removing initial packet"); - delete this.server.eio.opts.initialPacket; - } this.fns.push(fn); return this; } diff --git a/lib/socket.ts b/lib/socket.ts index c6fe4d649b..3735deebcf 100644 --- a/lib/socket.ts +++ b/lib/socket.ts @@ -65,6 +65,11 @@ export interface Handshake { * The query object */ query: object; + + /** + * The auth object + */ + auth: object; } export class Socket extends EventEmitter { @@ -88,30 +93,23 @@ export class Socket extends EventEmitter { * * @param {Namespace} nsp * @param {Client} client - * @param {Object} query + * @param {Object} auth * @package */ - constructor(readonly nsp: Namespace, readonly client: Client, query) { + constructor(readonly nsp: Namespace, readonly client: Client, auth: object) { super(); this.server = nsp.server; this.adapter = this.nsp.adapter; this.id = nsp.name !== "/" ? nsp.name + "#" + client.id : client.id; this.connected = true; this.disconnected = false; - this.handshake = this.buildHandshake(query); + this.handshake = this.buildHandshake(auth); } /** * Builds the `handshake` BC object */ - private buildHandshake(query): Handshake { - const self = this; - function buildQuery() { - const requestQuery = url.parse(self.request.url, true).query; - //if socket-specific query exist, replace query strings in requestQuery - return Object.assign({}, query, requestQuery); - } - + private buildHandshake(auth: object): Handshake { return { headers: this.request.headers, time: new Date() + "", @@ -121,7 +119,8 @@ export class Socket extends EventEmitter { secure: !!this.request.connection.encrypted, issued: +new Date(), url: this.request.url, - query: buildQuery() + query: url.parse(this.request.url, true).query, + auth }; } @@ -289,12 +288,7 @@ export class Socket extends EventEmitter { debug("socket connected - writing packet"); this.nsp.connected.set(this.id, this); this.join(this.id); - const skip = this.nsp.name === "/" && this.nsp.fns.length === 0; - if (skip) { - debug("packet already sent in initial handshake"); - } else { - this.packet({ type: PacketType.CONNECT }); - } + this.packet({ type: PacketType.CONNECT }); } /** diff --git a/package.json b/package.json index 2c8f5ff55f..cb86b9b2ae 100644 --- a/package.json +++ b/package.json @@ -23,8 +23,8 @@ }, "scripts": { "test": "npm run format:check && tsc && nyc mocha --require ts-node/register --reporter spec --slow 200 --bail --timeout 10000 test/socket.io.ts", - "format:check": "prettier --check 'lib/**/*.ts' 'test/**/*.js'", - "format:fix": "prettier --write 'lib/**/*.ts' 'test/**/*.js'" + "format:check": "prettier --check 'lib/**/*.ts' 'test/**/*.ts'", + "format:fix": "prettier --write 'lib/**/*.ts' 'test/**/*.ts'" }, "dependencies": { "debug": "~4.1.0", @@ -32,7 +32,7 @@ "has-binary2": "~1.0.2", "socket.io-adapter": "~2.0.1", "socket.io-client": "github:socketio/socket.io-client#develop", - "socket.io-parser": "~4.0.0" + "socket.io-parser": "github:socketio/socket.io-parser#develop" }, "devDependencies": { "@types/cookie": "^0.4.0", diff --git a/test/socket.io.ts b/test/socket.io.ts index c8245b1a03..9353653e1f 100644 --- a/test/socket.io.ts +++ b/test/socket.io.ts @@ -507,45 +507,6 @@ describe("socket.io", () => { }); }); - it("should disconnect both default and custom namespace upon disconnect", done => { - const srv = http(); - const sio = io(srv); - srv.listen(() => { - const lolcats = client(srv, "/lolcats"); - let total = 2; - let totald = 2; - let s; - sio.of("/", socket => { - socket.on("disconnect", reason => { - --totald || done(); - }); - --total || close(); - }); - sio.of("/lolcats", socket => { - s = socket; - socket.on("disconnect", reason => { - --totald || done(); - }); - --total || close(); - }); - function close() { - s.disconnect(true); - } - }); - }); - - it("should not crash while disconnecting socket", done => { - const srv = http(); - const sio = io(srv); - srv.listen(() => { - const socket = client(srv, "/ns"); - sio.on("connection", socket => { - socket.disconnect(); - done(); - }); - }); - }); - it("should fire a `disconnecting` event just before leaving all rooms", done => { const srv = http(); const sio = io(srv); @@ -1529,12 +1490,14 @@ describe("socket.io", () => { const sio = io(srv); const client1 = client(srv); const client2 = client(srv, "/connection2", { - query: { key1: "aa", key2: "&=bb" } + auth: { key1: "aa", key2: "&=bb" } }); sio.on("connection", s => {}); sio.of("/connection2").on("connection", s => { - expect(s.handshake.query.key1).to.be("aa"); - expect(s.handshake.query.key2).to.be("&=bb"); + expect(s.handshake.query.key1).to.be(undefined); + expect(s.handshake.query.EIO).to.be("4"); + expect(s.handshake.auth.key1).to.be("aa"); + expect(s.handshake.auth.key2).to.be("&=bb"); done(); }); }); @@ -2195,14 +2158,12 @@ describe("socket.io", () => { const sio = io(srv); let socket; sio.use((s, next) => { - socket.io.engine.on("open", () => { - socket.io.engine.close(); - s.client.conn.on("close", () => { - process.nextTick(next); - setTimeout(() => { - done(); - }, 50); - }); + socket.io.engine.close(); + s.client.conn.on("close", () => { + process.nextTick(next); + setTimeout(() => { + done(); + }, 50); }); }); srv.listen(() => { @@ -2218,11 +2179,14 @@ describe("socket.io", () => { const sio = io(srv); const result = []; - sio.use((socket, next) => { + sio.use(() => { + done(new Error("should not fire")); + }); + sio.of("/chat").use((socket, next) => { result.push(1); setTimeout(next, 50); }); - sio.use((socket, next) => { + sio.of("/chat").use((socket, next) => { result.push(2); setTimeout(next, 50); }); @@ -2230,15 +2194,11 @@ describe("socket.io", () => { result.push(3); setTimeout(next, 50); }); - sio.of("/chat").use((socket, next) => { - result.push(4); - setTimeout(next, 50); - }); srv.listen(() => { const chat = client(srv, "/chat"); chat.on("connect", () => { - expect(result).to.eql([1, 2, 3, 4]); + expect(result).to.eql([1, 2, 3]); done(); }); });