Skip to content

Commit

Permalink
fix: close clients with no namespace
Browse files Browse the repository at this point in the history
After a given timeout, a client that did not join any namespace will be
closed in order to prevent malicious clients from using the server
resources.

The timeout defaults to 45 seconds, in order not to interfere with the
Engine.IO heartbeat mechanism (30 seconds).
  • Loading branch information
darrachequesne committed Oct 15, 2020
1 parent 58b66f8 commit 91cd255
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 16 deletions.
14 changes: 14 additions & 0 deletions lib/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export class Client {
private readonly decoder: Decoder;
private sockets: Map<SocketId, Socket> = new Map();
private nsps: Map<string, Socket> = new Map();
private connectTimeout: NodeJS.Timeout;

/**
* Client constructor.
Expand Down Expand Up @@ -58,6 +59,15 @@ export class Client {
this.conn.on("data", this.ondata);
this.conn.on("error", this.onerror);
this.conn.on("close", this.onclose);

this.connectTimeout = setTimeout(() => {
if (this.nsps.size === 0) {
debug("no namespace joined yet, close the client");
this.close();
} else {
debug("the client has already joined a namespace, nothing to do");
}
}, this.server._connectTimeout);
}

/**
Expand Down Expand Up @@ -97,6 +107,10 @@ export class Client {
* @private
*/
private doConnect(name: string, auth: object) {
if (this.connectTimeout) {
clearTimeout(this.connectTimeout);
this.connectTimeout = null;
}
const nsp = this.server.of(name);

const socket = nsp.add(this, auth, () => {
Expand Down
68 changes: 52 additions & 16 deletions lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,23 @@ type Transport = "polling" | "websocket";

interface EngineOptions {
/**
* how many ms without a pong packet to consider the connection closed (5000)
* how many ms without a pong packet to consider the connection closed
* @default 5000
*/
pingTimeout: number;
/**
* how many ms before sending a new ping packet (25000)
* how many ms before sending a new ping packet
* @default 25000
*/
pingInterval: number;
/**
* how many ms before an uncompleted transport upgrade is cancelled (10000)
* how many ms before an uncompleted transport upgrade is cancelled
* @default 10000
*/
upgradeTimeout: number;
/**
* how many bytes or characters a message can be, before closing the session (to avoid DoS). Default value is 1E5.
* how many bytes or characters a message can be, before closing the session (to avoid DoS).
* @default 1e5 (100 KB)
*/
maxHttpBufferSize: number;
/**
Expand All @@ -55,19 +59,23 @@ interface EngineOptions {
fn: (err: string | null | undefined, success: boolean) => void
) => void;
/**
* to allow connections to (['polling', 'websocket'])
* the low-level transports that are enabled
* @default ["polling", "websocket"]
*/
transports: Transport[];
/**
* whether to allow transport upgrades (true)
* whether to allow transport upgrades
* @default true
*/
allowUpgrades: boolean;
/**
* parameters of the WebSocket permessage-deflate extension (see ws module api docs). Set to false to disable. (false)
* parameters of the WebSocket permessage-deflate extension (see ws module api docs). Set to false to disable.
* @default false
*/
perMessageDeflate: boolean | object;
/**
* parameters of the http compression for the polling transports (see zlib api docs). Set to false to disable. (true)
* parameters of the http compression for the polling transports (see zlib api docs). Set to false to disable.
* @default true
*/
httpCompression: boolean | object;
/**
Expand All @@ -82,7 +90,8 @@ interface EngineOptions {
initialPacket: any;
/**
* configuration of the cookie that contains the client sid to send as part of handshake response headers. This cookie
* might be used for sticky-session. Defaults to not sending any cookie (false)
* might be used for sticky-session. Defaults to not sending any cookie.
* @default false
*/
cookie: CookieSerializeOptions | boolean;
/**
Expand All @@ -93,15 +102,18 @@ interface EngineOptions {

interface AttachOptions {
/**
* name of the path to capture (/engine.io).
* name of the path to capture
* @default "/engine.io"
*/
path: string;
/**
* destroy unhandled upgrade requests (true)
* destroy unhandled upgrade requests
* @default true
*/
destroyUpgrade: boolean;
/**
* milliseconds after which unhandled requests are ended (1000)
* milliseconds after which unhandled requests are ended
* @default 1000
*/
destroyUpgradeTimeout: number;
}
Expand All @@ -110,21 +122,30 @@ interface EngineAttachOptions extends EngineOptions, AttachOptions {}

interface ServerOptions extends EngineAttachOptions {
/**
* name of the path to capture (/socket.io)
* name of the path to capture
* @default "/socket.io"
*/
path: string;
/**
* whether to serve the client files (true)
* whether to serve the client files
* @default true
*/
serveClient: boolean;
/**
* the adapter to use. Defaults to an instance of the Adapter that ships with socket.io which is memory based.
* the adapter to use
* @default the in-memory adapter (https://github.com/socketio/socket.io-adapter)
*/
adapter: any;
/**
* the parser to use. Defaults to an instance of the Parser that ships with socket.io.
* the parser to use
* @default the default parser (https://github.com/socketio/socket.io-parser)
*/
parser: any;
/**
* how many ms before a client without namespace is closed
* @default 45000
*/
connectTimeout: number;
}

export class Server extends EventEmitter {
Expand Down Expand Up @@ -154,6 +175,7 @@ export class Server extends EventEmitter {
private eio;
private engine;
private _path: string;
private _connectTimeout: number;
private httpServer: http.Server;

/**
Expand All @@ -173,6 +195,7 @@ export class Server extends EventEmitter {
srv = null;
}
this.path(opts.path || "/socket.io");
this.connectTimeout(opts.connectTimeout || 45000);
this.serveClient(false !== opts.serveClient);
this._parser = opts.parser || parser;
this.encoder = new this._parser.Encoder();
Expand Down Expand Up @@ -263,6 +286,19 @@ export class Server extends EventEmitter {
return this;
}

/**
* Set the delay after which a client without namespace is closed
* @param v
* @public
*/
public connectTimeout(v: number): Server;
public connectTimeout(): number;
public connectTimeout(v?: number): Server | number {
if (v === undefined) return this._connectTimeout;
this._connectTimeout = v;
return this;
}

/**
* Sets the adapter for rooms.
*
Expand Down
19 changes: 19 additions & 0 deletions test/socket.io.ts
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,25 @@ describe("socket.io", () => {
);
});

it("should close a client without namespace", done => {
const srv = createServer();
const sio = new Server(srv, {
connectTimeout: 10
});

srv.listen(() => {
const socket = client(srv);

socket.io.engine.write = () => {}; // prevent the client from sending a CONNECT packet

socket.on("disconnect", () => {
socket.close();
sio.close();
done();
});
});
});

describe("dynamic namespaces", () => {
it("should allow connections to dynamic namespaces with a regex", done => {
const srv = createServer();
Expand Down

0 comments on commit 91cd255

Please sign in to comment.