Skip to content

Can't set response headers in Server.handleUpgrade #5497

@weary-adventurer

Description

@weary-adventurer

Describe the bug
It's not possible to specify headers for an upgrade response that is handled by Server.handleUpgrade.

To Reproduce
Follow the usage example where requests are passed in explicitly:

#### (C) Passing in requests
```js
const engine = require('engine.io');
const server = new engine.Server();
server.on('connection', socket => {
socket.send('hi');
});
//
httpServer.on('upgrade', (req, socket, head) => {
server.handleUpgrade(req, socket, head);
});
httpServer.on('request', (req, res) => {
server.handleRequest(req, res);
});
```

Using this example it's not possible to specify the upgrade response headers. Specifically, it's not possible to set the CORS headers without using the .cors "middleware" option. Since d1f5aa9 using the .cors "middleware" option breaks webtransport, so the only way to make it work is to remove the .cors option and set them up manually.

Expected behavior
It should be possible to set any headers for the upgrade response that is sent out when upgrade is handled by engine.io.

Engine.IO currently uses the kResponseHeaders key on the request object to specify additional headers to send in the upgrade response:

if (typeof this.ws.on === "function") {
this.ws.on("headers", (headersArray, req: EngineRequest) => {
// note: 'ws' uses an array of headers, while Engine.IO uses an object (response.writeHead() accepts both formats)
// we could also try to parse the array and then sync the values, but that will be error-prone
const additionalHeaders = req[kResponseHeaders] || {};
delete req[kResponseHeaders];
const isInitialRequest = !req._query.sid;
if (isInitialRequest) {
this.emit("initial_headers", additionalHeaders, req);
}
this.emit("headers", additionalHeaders, req);
debug("writing headers: %j", additionalHeaders);
Object.keys(additionalHeaders).forEach((key) => {
headersArray.push(`${key}: ${additionalHeaders[key]}`);
});
});
}

I expected it to be possible to set them up from my code in a similar way:

const kResponseHeaders = Symbol.for("responseHeaders");

server.on("upgrade", (req, socket, head) => {
    if (req.url?.startsWith("/engine.io/")) {
        req[kResponseHeaders] = {
            "Access-Control-Allow-Origin": req.headers["origin"] || "*",
            "Access-Control-Allow-Credentials": "true"
        };
        return eio.handleUpgrade(req, socket, head);
    }
    // ...
});

But it is currently defined as a unique Symbol, which prevents above code from working:

const kResponseHeaders = Symbol("responseHeaders");

The new definition can use Symbol.for instead to allow user code to specify extra headers for the websocket upgrade response.

Metadata

Metadata

Assignees

No one assigned

    Labels

    to triageWaiting to be triaged by a member of the team

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions