diff --git a/packages/engine.io/lib/server.ts b/packages/engine.io/lib/server.ts index c7c571613..aa44ce09b 100644 --- a/packages/engine.io/lib/server.ts +++ b/packages/engine.io/lib/server.ts @@ -397,17 +397,45 @@ export abstract class BaseServer extends EventEmitter { const apply = (i) => { debug("applying middleware n°%d", i + 1); - this.middlewares[i](req, res, (err?: any) => { - if (err) { - return callback(err); + + // next(), a throw, or a rejection may each reach callback. Only the first + // one wins: a middleware that calls next() and then throws must not fire + // callback a second time. Errors thrown by next()'s own continuation are + // not middleware failures and are rethrown. + let settled = false; + let continuationError: any; + + const done = (err?: any) => { + if (settled) { + debug("middleware already settled, ignoring extra callback"); + return; + } + settled = true; + + try { + if (err) { + callback(err); + } else if (i + 1 < this.middlewares.length) { + apply(i + 1); + } else { + callback(); + } + } catch (e) { + continuationError = e; + throw e; } + }; - if (i + 1 < this.middlewares.length) { - apply(i + 1); - } else { - callback(); + try { + Promise.resolve(this.middlewares[i](req, res, done)).catch((err) => { + done(err); + }); + } catch (e) { + if (continuationError) { + throw continuationError; } - }); + done(e); + } }; apply(0); @@ -1042,6 +1070,16 @@ function abortRequest( ? errorContext.message : Server.errorMessages[errorCode]; + // A middleware may have already committed the response before failing. + // Writing again throws "Cannot write headers after they are sent". + if (res.headersSent) { + debug("response headers already sent, skipping error response"); + if (!res.writableEnded && !res.finished) { + res.end(); + } + return; + } + res.writeHead(statusCode, { "Content-Type": "application/json" }); res.end( JSON.stringify({ @@ -1116,7 +1154,7 @@ const validHdrChars = [ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 // ... 255 -] +]; function checkInvalidHeaderChar(val?: string) { val += ""; diff --git a/packages/engine.io/lib/userver.ts b/packages/engine.io/lib/userver.ts index 76ecc9911..a9c7a425a 100644 --- a/packages/engine.io/lib/userver.ts +++ b/packages/engine.io/lib/userver.ts @@ -1,4 +1,5 @@ import debugModule from "debug"; +import { STATUS_CODES } from "http"; import { AttachOptions, BaseServer, Server } from "./server"; import { HttpRequest, HttpResponse, TemplatedApp } from "uWebSockets.js"; import transports from "./transports-uws"; @@ -269,6 +270,15 @@ export class uServer extends BaseServer { errorCode: number, errorContext?: { message?: string }, ) { + // uWS throws if HttpResponse is used after a successful end(). + if (res instanceof ResponseWrapper && res.headersSent) { + debug("response headers already sent, skipping error response"); + if (!res.writableEnded) { + res.end(); + } + return; + } + const statusCode = errorCode === Server.errors.FORBIDDEN ? "403 Forbidden" @@ -291,21 +301,45 @@ export class uServer extends BaseServer { class ResponseWrapper { private statusWritten: boolean = false; + private ended = false; private headers = []; private isAborted = false; constructor(readonly res: HttpResponse) {} + public get headersSent() { + return this.statusWritten || this.ended; + } + + public get writableEnded() { + return this.ended; + } + public set statusCode(status: number) { if (!status) { return; } - // FIXME: handle all status codes? - this.writeStatus(status === 200 ? "200 OK" : "204 No Content"); + const reason = STATUS_CODES[status] || "Unknown"; + this.writeStatus(`${status} ${reason}`); } - public writeHead(status: number) { - this.statusCode = status; + public writeHead(status?: number, headers?: any, maybeHeaders?: any) { + if (typeof status === "number") { + this.statusCode = status; + } + + const headerFields = + headers !== null && typeof headers === "object" ? headers : maybeHeaders; + if ( + !headerFields || + typeof headerFields !== "object" || + Array.isArray(headerFields) + ) { + return; + } + Object.keys(headerFields).forEach((key) => { + this.setHeader(key, headerFields[key]); + }); } public setHeader(key, value) { @@ -354,15 +388,20 @@ class ResponseWrapper { }); } - public end(data) { - if (this.isAborted) return; + public end(data?) { + if (this.isAborted || this.ended) return; + this.ended = true; this.res.cork(() => { if (!this.statusWritten) { // status will be inferred as "200 OK" this.writeBufferedHeaders(); } - this.res.end(data); + if (data === undefined) { + this.res.end(); + } else { + this.res.end(data); + } }); } diff --git a/packages/engine.io/test/middlewares.js b/packages/engine.io/test/middlewares.js index 60e1cc6b2..1acac81ad 100644 --- a/packages/engine.io/test/middlewares.js +++ b/packages/engine.io/test/middlewares.js @@ -1,10 +1,11 @@ -const listen = require("./common").listen; +const { listen } = require("./common"); +const { Server } = require(".."); const expect = require("expect.js"); const request = require("superagent"); const { WebSocket } = require("ws"); const helmet = require("helmet"); const session = require("express-session"); -const { ClientSocket } = require("./common"); +const { ClientSocket, createPartialDone } = require("./common"); describe("middlewares", () => { it("should apply middleware (polling)", (done) => { @@ -249,6 +250,312 @@ describe("middlewares", () => { }); }); + it("should fail on synchronous middleware errors (polling)", (done) => { + const partialDone = createPartialDone(done, 2); + const engine = listen((port) => { + engine.use(() => { + throw new Error("will always fail"); + }); + + engine.on("connection_error", (err) => { + expect(err.req).to.be.ok(); + expect(err.code).to.eql(3); + expect(err.message).to.eql("Bad request"); + expect(err.context.name).to.eql("MIDDLEWARE_FAILURE"); + partialDone(); + }); + + request + .get(`http://localhost:${port}/engine.io/`) + .query({ EIO: 4, transport: "polling" }) + .end((err, res) => { + expect(err).to.be.an(Error); + expect(res.status).to.eql(400); + + if (engine.httpServer) { + engine.httpServer.close(); + } + partialDone(); + }); + }); + }); + + it("should fail on rejected middleware promises (polling)", (done) => { + const partialDone = createPartialDone(done, 2); + const engine = listen((port) => { + engine.use(() => { + return Promise.reject(new Error("will always fail")); + }); + + engine.on("connection", () => { + done(new Error("should not connect")); + }); + + engine.on("connection_error", (err) => { + expect(err.req).to.be.ok(); + expect(err.code).to.eql(3); + expect(err.message).to.eql("Bad request"); + expect(err.context.name).to.eql("MIDDLEWARE_FAILURE"); + partialDone(); + }); + + request + .get(`http://localhost:${port}/engine.io/`) + .query({ EIO: 4, transport: "polling" }) + .end((err, res) => { + expect(err).to.be.an(Error); + expect(res.status).to.eql(400); + + if (engine.httpServer) { + engine.httpServer.close(); + } + partialDone(); + }); + }); + }); + + it("should ignore a throw after next() and continue (polling)", (done) => { + let connectionErrors = 0; + let connections = 0; + const engine = listen((port) => { + engine.use((req, res, next) => { + next(); + throw new Error("after next"); + }); + + engine.use((req, res, next) => { + res.setHeader("foo", "bar"); + next(); + }); + + engine.on("connection_error", () => { + connectionErrors++; + }); + + engine.on("connection", () => { + connections++; + }); + + request + .get(`http://localhost:${port}/engine.io/`) + .query({ EIO: 4, transport: "polling" }) + .end((err, res) => { + expect(err).to.be(null); + expect(res.status).to.eql(200); + expect(res.headers["foo"]).to.eql("bar"); + expect(connectionErrors).to.eql(0); + expect(connections).to.eql(1); + + if (engine.httpServer) { + engine.httpServer.close(); + } + done(); + }); + }); + }); + + it("should ignore a throw after next(err) and fail once (polling)", (done) => { + let connectionErrors = 0; + const engine = listen((port) => { + engine.use((req, res, next) => { + next(new Error("will always fail")); + throw new Error("after next(err)"); + }); + + engine.use(() => { + done(new Error("next middleware should not run")); + }); + + engine.on("connection", () => { + done(new Error("should not connect")); + }); + + engine.on("connection_error", (err) => { + connectionErrors++; + expect(err.req).to.be.ok(); + expect(err.code).to.eql(3); + expect(err.message).to.eql("Bad request"); + expect(err.context.name).to.eql("MIDDLEWARE_FAILURE"); + }); + + request + .get(`http://localhost:${port}/engine.io/`) + .query({ EIO: 4, transport: "polling" }) + .end((err, res) => { + expect(err).to.be.an(Error); + expect(res.status).to.eql(400); + expect(connectionErrors).to.eql(1); + + if (engine.httpServer) { + engine.httpServer.close(); + } + done(); + }); + }); + }); + + it("should ignore a rejection after next() and continue (polling)", (done) => { + let connectionErrors = 0; + let connections = 0; + const engine = listen((port) => { + engine.use((req, res, next) => { + next(); + return Promise.reject(new Error("after next")); + }); + + engine.on("connection_error", () => { + connectionErrors++; + }); + + engine.on("connection", () => { + connections++; + }); + + request + .get(`http://localhost:${port}/engine.io/`) + .query({ EIO: 4, transport: "polling" }) + .end((err, res) => { + expect(err).to.be(null); + expect(res.status).to.eql(200); + expect(connectionErrors).to.eql(0); + expect(connections).to.eql(1); + + if (engine.httpServer) { + engine.httpServer.close(); + } + done(); + }); + }); + }); + + it("should propagate errors thrown while handling next()", () => { + const engine = new Server({ transports: ["polling"] }); + let calls = 0; + + engine.use((_req, _res, next) => { + next(); + }); + + try { + expect(() => { + engine._applyMiddlewares({}, {}, () => { + calls++; + throw new Error("continuation failed"); + }); + }).to.throwException((err) => { + expect(err.message).to.eql("continuation failed"); + }); + expect(calls).to.eql(1); + } finally { + engine.close(); + } + }); + + it("should keep the middleware response when it throws after sending (polling)", (done) => { + let connectionErrors = 0; + const engine = listen((port) => { + engine.use((_req, res) => { + res.writeHead(503, { "Content-Type": "text/plain" }); + res.end("nope"); + throw new Error("after end"); + }); + + engine.on("connection", () => { + done(new Error("should not connect")); + }); + + engine.on("connection_error", (err) => { + connectionErrors++; + expect(err.context.name).to.eql("MIDDLEWARE_FAILURE"); + }); + + request + .get(`http://localhost:${port}/engine.io/`) + .query({ EIO: 4, transport: "polling" }) + .end((err, res) => { + expect(err).to.be.an(Error); + expect(res.status).to.eql(503); + expect(res.text).to.eql("nope"); + expect(connectionErrors).to.eql(1); + + if (engine.httpServer) { + engine.httpServer.close(); + } + done(); + }); + }); + }); + + it("should keep the middleware response when it rejects after sending (polling)", (done) => { + let connectionErrors = 0; + const engine = listen((port) => { + engine.use((_req, res) => { + res.writeHead(503, { "Content-Type": "text/plain" }); + res.end("nope"); + return Promise.reject(new Error("after end")); + }); + + engine.on("connection", () => { + done(new Error("should not connect")); + }); + + engine.on("connection_error", (err) => { + connectionErrors++; + expect(err.context.name).to.eql("MIDDLEWARE_FAILURE"); + }); + + request + .get(`http://localhost:${port}/engine.io/`) + .query({ EIO: 4, transport: "polling" }) + .end((err, res) => { + expect(err).to.be.an(Error); + expect(res.status).to.eql(503); + expect(res.text).to.eql("nope"); + expect(connectionErrors).to.eql(1); + + if (engine.httpServer) { + engine.httpServer.close(); + } + done(); + }); + }); + }); + + it("should invoke the callback once when next() is called twice (polling)", (done) => { + let runs = 0; + let connections = 0; + const engine = listen((port) => { + engine.use((req, res, next) => { + next(); + next(); + }); + + engine.use((req, res, next) => { + runs++; + next(); + }); + + engine.on("connection", () => { + connections++; + }); + + request + .get(`http://localhost:${port}/engine.io/`) + .query({ EIO: 4, transport: "polling" }) + .end((err, res) => { + expect(err).to.be(null); + expect(res.status).to.eql(200); + expect(runs).to.eql(1); + expect(connections).to.eql(1); + + if (engine.httpServer) { + engine.httpServer.close(); + } + done(); + }); + }); + }); + it("should fail on errors (polling)", (done) => { const engine = listen((port) => { engine.use((req, res, next) => {