Skip to content

Commit

Permalink
fix: prevent crash when using custom headers
Browse files Browse the repository at this point in the history
It seems that the Deno.upgradeWebSocket() method does not allow to
provide additional headers during the WebSocket handshake.

Exception:

> TypeError: Headers are immutable.
>    at Headers.set (deno:ext/fetch/20_headers.js:384:15)
>    at /packages/engine.io/lib/transports/websocket.ts:60:24
>    at Headers.forEach (deno:ext/webidl/00_webidl.js:1001:13)
>    at WS.onRequest (/packages/engine.io/lib/transports/websocket.ts:59:21)
>    at Server.handshake (/packages/engine.io/lib/server.ts:344:31)
  • Loading branch information
darrachequesne committed Sep 13, 2022
1 parent 193a9b5 commit dfe3122
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 6 deletions.
2 changes: 1 addition & 1 deletion packages/engine.io/lib/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ export class Server extends EventEmitter<never, never, ServerReservedEvents> {
if (req.headers.has("upgrade")) {
const transport = new WS(this.opts);

const promise = transport.onRequest(req, responseHeaders);
const promise = transport.onRequest(req);

socket._maybeUpgrade(transport);

Expand Down
6 changes: 2 additions & 4 deletions packages/engine.io/lib/transports/websocket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export class WS extends Transport {
}
}

public onRequest(req: Request, responseHeaders: Headers): Promise<Response> {
public onRequest(req: Request): Promise<Response> {
const { socket, response } = Deno.upgradeWebSocket(req);

this.socket = socket;
Expand Down Expand Up @@ -56,9 +56,7 @@ export class WS extends Transport {
this.onClose();
};

responseHeaders.forEach((value, key) => {
response.headers.set(key, value);
});
// note: response.headers is immutable, so it seems we can't add headers here

return Promise.resolve(response);
}
Expand Down
26 changes: 25 additions & 1 deletion packages/engine.io/test/response_headers.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { assertEquals, describe, it } from "../../../test_deps.ts";
import { Server } from "../lib/server.ts";
import { enableLogs, parseSessionID, testServe } from "./util.ts";
import {
enableLogs,
parseSessionID,
testServe,
testServeWithAsyncResults,
} from "./util.ts";

await enableLogs();

Expand Down Expand Up @@ -43,4 +48,23 @@ describe("response headers", () => {
await dataResponse.body?.cancel();
});
});

it("should not crash when using WebSocket (noop)", () => {
const engine = new Server({
editHandshakeHeaders: (responseHeaders) => {
responseHeaders.set("abc", "123");
},
editResponseHeaders: (responseHeaders) => {
responseHeaders.set("def", "456");
},
});

return testServeWithAsyncResults(engine, 1, (port, done) => {
const socket = new WebSocket(
`ws://localhost:${port}/engine.io/?EIO=4&transport=websocket`,
);

socket.onopen = done;
});
});
});

0 comments on commit dfe3122

Please sign in to comment.