From 7096e98a02295a62c8ea2aa56461d4875887092d Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne Date: Thu, 29 Apr 2021 16:41:16 +0200 Subject: [PATCH] feat: add a "connection_error" event The "connection_error" event will be emitted when one of the following errors occurs: - Transport unknown - Session ID unknown - Bad handshake method - Bad request - Forbidden - Unsupported protocol version Syntax: ```js server.on("connection_error", (err) => { console.log(err.req); // the request object console.log(err.code); // the error code, for example 1 console.log(err.message); // the error message, for example "Session ID unknown" console.log(err.context); // some additional error context }); ``` Related: - https://github.com/socketio/socket.io/issues/3819 - https://github.com/socketio/engine.io/issues/576 --- README.md | 18 +++++ lib/server.js | 148 +++++++++++++++++++++++++------------ test/common.js | 11 +++ test/server.js | 193 ++++++++++++++++++++++++++++++++++++++++++++----- 4 files changed, 305 insertions(+), 65 deletions(-) diff --git a/README.md b/README.md index 3b3659f05..729fca81c 100644 --- a/README.md +++ b/README.md @@ -204,6 +204,24 @@ The main server/manager. _Inherits from EventEmitter_. - Fired when a new connection is established. - **Arguments** - `Socket`: a Socket object +- `connection_error` + - Fired when an error occurs when establishing the connection. + - **Arguments** + - `error`: an object with following properties: + - `req` (`http.IncomingMessage`): the request that was dropped + - `code` (`Number`): one of `Server.errors` + - `message` (`string`): one of `Server.errorMessages` + - `context` (`Object`): extra info about the error + +| Code | Message | +| ---- | ------- | +| 0 | "Transport unknown" +| 1 | "Session ID unknown" +| 2 | "Bad handshake method" +| 3 | "Bad request" +| 4 | "Forbidden" +| 5 | "Unsupported protocol version" + ##### Properties diff --git a/lib/server.js b/lib/server.js index bf604ed48..b3ef79625 100644 --- a/lib/server.js +++ b/lib/server.js @@ -109,15 +109,19 @@ class Server extends EventEmitter { const transport = req._query.transport; if (!~this.opts.transports.indexOf(transport)) { debug('unknown transport "%s"', transport); - return fn(Server.errors.UNKNOWN_TRANSPORT, false); + return fn(Server.errors.UNKNOWN_TRANSPORT, { transport }); } // 'Origin' header check const isOriginInvalid = checkInvalidHeaderChar(req.headers.origin); if (isOriginInvalid) { + const origin = req.headers.origin; req.headers.origin = null; debug("origin header invalid"); - return fn(Server.errors.BAD_REQUEST, false); + return fn(Server.errors.BAD_REQUEST, { + name: "INVALID_ORIGIN", + origin + }); } // sid check @@ -125,21 +129,40 @@ class Server extends EventEmitter { if (sid) { if (!this.clients.hasOwnProperty(sid)) { debug('unknown sid "%s"', sid); - return fn(Server.errors.UNKNOWN_SID, false); + return fn(Server.errors.UNKNOWN_SID, { + sid + }); } - if (!upgrade && this.clients[sid].transport.name !== transport) { + const previousTransport = this.clients[sid].transport.name; + if (!upgrade && previousTransport !== transport) { debug("bad request: unexpected transport without upgrade"); - return fn(Server.errors.BAD_REQUEST, false); + return fn(Server.errors.BAD_REQUEST, { + name: "TRANSPORT_MISMATCH", + transport, + previousTransport + }); } } else { // handshake is GET only - if ("GET" !== req.method) - return fn(Server.errors.BAD_HANDSHAKE_METHOD, false); - if (!this.opts.allowRequest) return fn(null, true); - return this.opts.allowRequest(req, fn); + if ("GET" !== req.method) { + return fn(Server.errors.BAD_HANDSHAKE_METHOD, { + method: req.method + }); + } + + if (!this.opts.allowRequest) return fn(); + + return this.opts.allowRequest(req, (message, success) => { + if (!success) { + return fn(Server.errors.FORBIDDEN, { + message + }); + } + fn(); + }); } - fn(null, true); + fn(); } /** @@ -186,9 +209,15 @@ class Server extends EventEmitter { this.prepare(req); req.res = res; - const callback = (err, success) => { - if (!success) { - sendErrorMessage(req, res, err); + const callback = (errorCode, errorContext) => { + if (errorCode !== undefined) { + this.emit("connection_error", { + req, + code: errorCode, + message: Server.errorMessages[errorCode], + context: errorContext + }); + sendErrorMessage(req, res, errorCode, errorContext); return; } @@ -231,6 +260,15 @@ class Server extends EventEmitter { const protocol = req._query.EIO === "4" ? 4 : 3; // 3rd revision by default if (protocol === 3 && !this.opts.allowEIO3) { debug("unsupported protocol version"); + this.emit("connection_error", { + req, + code: Server.errors.UNSUPPORTED_PROTOCOL_VERSION, + message: + Server.errorMessages[Server.errors.UNSUPPORTED_PROTOCOL_VERSION], + context: { + protocol + } + }); sendErrorMessage( req, req.res, @@ -244,6 +282,15 @@ class Server extends EventEmitter { id = await this.generateId(req); } catch (e) { debug("error while generating an id"); + this.emit("connection_error", { + req, + code: Server.errors.BAD_REQUEST, + message: Server.errorMessages[Server.errors.BAD_REQUEST], + context: { + name: "ID_GENERATION_ERROR", + error: e + } + }); sendErrorMessage(req, req.res, Server.errors.BAD_REQUEST); return; } @@ -266,6 +313,15 @@ class Server extends EventEmitter { } } catch (e) { debug('error handshaking to transport "%s"', transportName); + this.emit("connection_error", { + req, + code: Server.errors.BAD_REQUEST, + message: Server.errorMessages[Server.errors.BAD_REQUEST], + context: { + name: "TRANSPORT_HANDSHAKE_ERROR", + error: e + } + }); sendErrorMessage(req, req.res, Server.errors.BAD_REQUEST); return; } @@ -304,9 +360,15 @@ class Server extends EventEmitter { this.prepare(req); const self = this; - this.verify(req, true, function(err, success) { - if (!success) { - abortConnection(socket, err); + this.verify(req, true, (errorCode, errorContext) => { + if (errorCode) { + this.emit("connection_error", { + req, + code: errorCode, + message: Server.errorMessages[errorCode], + context: errorContext + }); + abortConnection(socket, errorCode, errorContext); return; } @@ -469,52 +531,46 @@ Server.errorMessages = { /** * Sends an Engine.IO Error Message * - * @param {http.ServerResponse} response - * @param {code} error code + * @param req - the request object + * @param res - the response object + * @param errorCode - the error code + * @param errorContext - additional error context + * * @api private */ -function sendErrorMessage(req, res, code) { - const headers = { "Content-Type": "application/json" }; - - const isForbidden = !Server.errorMessages.hasOwnProperty(code); - if (isForbidden) { - res.writeHead(403, headers); - res.end( - JSON.stringify({ - code: Server.errors.FORBIDDEN, - message: code || Server.errorMessages[Server.errors.FORBIDDEN] - }) - ); - return; - } - if (res !== undefined) { - res.writeHead(400, headers); - res.end( - JSON.stringify({ - code: code, - message: Server.errorMessages[code] - }) - ); - } +function sendErrorMessage(req, res, errorCode, errorContext) { + const statusCode = errorCode === Server.errors.FORBIDDEN ? 403 : 400; + const message = + errorContext && errorContext.message + ? errorContext.message + : Server.errorMessages[errorCode]; + + res.writeHead(statusCode, { "Content-Type": "application/json" }); + res.end( + JSON.stringify({ + code: errorCode, + message + }) + ); } /** * Closes the connection * * @param {net.Socket} socket - * @param {code} error code + * @param {string} errorCode - the error code + * @param {object} errorContext - additional error context + * * @api private */ -function abortConnection(socket, code) { +function abortConnection(socket, errorCode, errorContext) { socket.on("error", () => { debug("ignoring error from closed connection"); }); if (socket.writable) { - const message = Server.errorMessages.hasOwnProperty(code) - ? Server.errorMessages[code] - : String(code || ""); + const message = errorContext.message || Server.errorMessages[errorCode]; const length = Buffer.byteLength(message); socket.write( "HTTP/1.1 400 Bad Request\r\n" + diff --git a/test/common.js b/test/common.js index 95021ca79..1e747e412 100644 --- a/test/common.js +++ b/test/common.js @@ -34,3 +34,14 @@ exports.eioc = eioc; */ require("s").extend(); + +exports.createPartialDone = (done, count) => { + let i = 0; + return () => { + if (++i === count) { + done(); + } else if (i > count) { + done(new Error(`partialDone() called too many times: ${i} > ${count}`)); + } + }; +}; diff --git a/test/server.js b/test/server.js index c411aae70..53e57f2ef 100644 --- a/test/server.js +++ b/test/server.js @@ -7,8 +7,7 @@ const path = require("path"); const exec = require("child_process").exec; const zlib = require("zlib"); const eio = require(".."); -const eioc = require("./common").eioc; -const listen = require("./common").listen; +const { eioc, listen, createPartialDone } = require("./common"); const expect = require("expect.js"); const request = require("superagent"); const cookieMod = require("cookie"); @@ -18,9 +17,30 @@ const cookieMod = require("cookie"); */ describe("server", () => { + let engine, client; + + afterEach(() => { + if (engine) { + engine.httpServer.close(); + } + if (client) { + client.close(); + } + }); + describe("verification", () => { it("should disallow non-existent transports", done => { - listen(port => { + const partialDone = createPartialDone(done, 2); + + engine = listen(port => { + engine.on("connection_error", err => { + expect(err.req).to.be.an(http.IncomingMessage); + expect(err.code).to.be(0); + expect(err.message).to.be("Transport unknown"); + expect(err.context.transport).to.be("tobi"); + partialDone(); + }); + request .get("http://localhost:%d/engine.io/default/".s(port)) .query({ transport: "tobi" }) // no tobi transport - outrageous @@ -29,14 +49,24 @@ describe("server", () => { expect(res.status).to.be(400); expect(res.body.code).to.be(0); expect(res.body.message).to.be("Transport unknown"); - done(); + partialDone(); }); }); }); it("should disallow `constructor` as transports", done => { + const partialDone = createPartialDone(done, 2); + // make sure we check for actual properties - not those present on every {} - listen(port => { + engine = listen(port => { + engine.on("connection_error", err => { + expect(err.req).to.be.an(http.IncomingMessage); + expect(err.code).to.be(0); + expect(err.message).to.be("Transport unknown"); + expect(err.context.transport).to.be("constructor"); + partialDone(); + }); + request .get("http://localhost:%d/engine.io/default/".s(port)) .set("Origin", "http://engine.io") @@ -46,13 +76,23 @@ describe("server", () => { expect(res.status).to.be(400); expect(res.body.code).to.be(0); expect(res.body.message).to.be("Transport unknown"); - done(); + partialDone(); }); }); }); it("should disallow non-existent sids", done => { - listen(port => { + const partialDone = createPartialDone(done, 2); + + engine = listen(port => { + engine.on("connection_error", err => { + expect(err.req).to.be.an(http.IncomingMessage); + expect(err.code).to.be(1); + expect(err.message).to.be("Session ID unknown"); + expect(err.context.sid).to.be("test"); + partialDone(); + }); + request .get("http://localhost:%d/engine.io/default/".s(port)) .set("Origin", "http://engine.io") @@ -62,19 +102,29 @@ describe("server", () => { expect(res.status).to.be(400); expect(res.body.code).to.be(1); expect(res.body.message).to.be("Session ID unknown"); - done(); + partialDone(); }); }); }); it("should disallow requests that are rejected by `allowRequest`", done => { - listen( + const partialDone = createPartialDone(done, 2); + + engine = listen( { allowRequest: (req, fn) => { fn("Thou shall not pass", false); } }, port => { + engine.on("connection_error", err => { + expect(err.req).to.be.an(http.IncomingMessage); + expect(err.code).to.be(4); + expect(err.message).to.be("Forbidden"); + expect(err.context.message).to.be("Thou shall not pass"); + partialDone(); + }); + request .get("http://localhost:%d/engine.io/default/".s(port)) .set("Origin", "http://engine.io") @@ -84,7 +134,7 @@ describe("server", () => { expect(res.status).to.be(403); expect(res.body.code).to.be(4); expect(res.body.message).to.be("Thou shall not pass"); - done(); + partialDone(); }); } ); @@ -302,14 +352,24 @@ describe("server", () => { }); it("should disallow connection that are rejected by `generateId`", done => { - const engine = listen({ allowUpgrades: false }, port => { + const partialDone = createPartialDone(done, 2); + + engine = listen({ allowUpgrades: false }, port => { engine.generateId = () => { return Promise.reject(new Error("nope")); }; + engine.on("connection_error", err => { + expect(err.req).to.be.an(http.IncomingMessage); + expect(err.code).to.be(3); + expect(err.message).to.be("Bad request"); + expect(err.context.name).to.be("ID_GENERATION_ERROR"); + partialDone(); + }); + const socket = new eioc.Socket("ws://localhost:%d".s(port)); socket.on("error", () => { - done(); + partialDone(); }); }); }); @@ -410,13 +470,25 @@ describe("server", () => { }); it("default to polling when proxy doesn't support websocket", done => { - const engine = listen({ allowUpgrades: false }, port => { + const partialDone = createPartialDone(done, 2); + + engine = listen({ allowUpgrades: false }, port => { engine.on("connection", socket => { socket.on("message", msg => { if ("echo" === msg) socket.send(msg); }); }); + engine.on("connection_error", err => { + expect(err.req).to.be.an(http.IncomingMessage); + expect(err.code).to.be(3); + expect(err.message).to.be("Bad request"); + expect(err.context.name).to.be("TRANSPORT_MISMATCH"); + expect(err.context.transport).to.be("websocket"); + expect(err.context.previousTransport).to.be("polling"); + partialDone(); + }); + var socket = new eioc.Socket("ws://localhost:%d".s(port)); socket.on("open", () => { request @@ -430,7 +502,7 @@ describe("server", () => { socket.send("echo"); socket.on("message", msg => { expect(msg).to.be("echo"); - done(); + partialDone(); }); }); }); @@ -460,12 +532,24 @@ describe("server", () => { }); }); - it("should disallow bad requests", done => { - listen( + it("should disallow bad requests (handshake error)", done => { + const partialDone = createPartialDone(done, 2); + + engine = listen( { cors: { credentials: true, origin: "http://engine.io" } }, port => { + engine.on("connection_error", err => { + expect(err.req).to.be.an(http.IncomingMessage); + expect(err.code).to.be(3); + expect(err.message).to.be("Bad request"); + expect(err.context.name).to.be("TRANSPORT_HANDSHAKE_ERROR"); + expect(err.context.error).to.be.an(Error); + expect(err.context.error.name).to.be("TypeError"); + partialDone(); + }); + request .get("http://localhost:%d/engine.io/default/".s(port)) .set("Origin", "http://engine.io") @@ -481,18 +565,90 @@ describe("server", () => { expect(res.header["access-control-allow-origin"]).to.be( "http://engine.io" ); - done(); + partialDone(); }); } ); }); + it("should disallow invalid origin header", done => { + const partialDone = createPartialDone(done, 2); + + engine = listen(port => { + // we can't send an invalid header through request.get + // so add an invalid char here + engine.prepare = function(req) { + eio.Server.prototype.prepare.call(engine, req); + req.headers.origin += "\n"; + }; + + engine.on("connection_error", err => { + expect(err.req).to.be.an(http.IncomingMessage); + expect(err.code).to.be(3); + expect(err.message).to.be("Bad request"); + expect(err.context.name).to.be("INVALID_ORIGIN"); + expect(err.context.origin).to.be("http://engine.io/\n"); + partialDone(); + }); + + request + .get("http://localhost:%d/engine.io/default/".s(port)) + .set("Origin", "http://engine.io/") + .query({ transport: "websocket" }) + .end((err, res) => { + expect(err).to.be.an(Error); + expect(res.status).to.be(400); + expect(res.body.code).to.be(3); + expect(res.body.message).to.be("Bad request"); + partialDone(); + }); + }); + }); + + it("should disallow invalid handshake method", done => { + const partialDone = createPartialDone(done, 2); + + engine = listen(port => { + engine.on("connection_error", err => { + expect(err.req).to.be.an(http.IncomingMessage); + expect(err.code).to.be(2); + expect(err.message).to.be("Bad handshake method"); + expect(err.context.method).to.be("OPTIONS"); + partialDone(); + }); + + request + .options("http://localhost:%d/engine.io/default/".s(port)) + .query({ transport: "polling" }) + .end((err, res) => { + expect(err).to.be.an(Error); + expect(res.status).to.be(400); + expect(res.body.code).to.be(2); + expect(res.body.message).to.be("Bad handshake method"); + partialDone(); + }); + }); + }); + it("should disallow unsupported protocol versions", done => { + const partialDone = createPartialDone(done, 2); + const httpServer = http.createServer(); const engine = eio({ allowEIO3: false }); engine.attach(httpServer); httpServer.listen(() => { const port = httpServer.address().port; + + engine.on("connection_error", err => { + expect(err.req).to.be.an(http.IncomingMessage); + expect(err.code).to.be(5); + expect(err.message).to.be("Unsupported protocol version"); + expect(err.context.protocol).to.be(3); + + httpServer.close(); + partialDone(); + }); + request .get("http://localhost:%d/engine.io/".s(port)) .query({ transport: "polling", EIO: 3 }) @@ -501,8 +657,7 @@ describe("server", () => { expect(res.status).to.be(400); expect(res.body.code).to.be(5); expect(res.body.message).to.be("Unsupported protocol version"); - engine.close(); - done(); + partialDone(); }); }); });