Can't set response headers in Server.handleUpgrade

Open Beginner friendly
#5,497 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
72/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
nodejs, typescript
Domain
api, backend

Research direction

Start in packages/engine.io/lib/server.ts at the unique kResponseHeaders definition and the Server.handleUpgrade response-header handling around lines 713–732. Compare this with the explicit-request usage in the engine.io README; done means user code can provide additional upgrade response headers and the documented CORS example works without the cors middleware.

Written by the indexing model from the issue text.

Description

question

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:
https://github.com/socketio/socket.io/blob/439a8f669c6788b512dd76011c4476830d67515b/packages/engine.io/README.md?plain=1#L40-L58

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 https://github.com/socketio/socket.io/commit/d1f5aa93722a7f1ed729b96f771daf92a3dfdaf7 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:
https://github.com/socketio/socket.io/blob/439a8f669c6788b512dd76011c4476830d67515b/packages/engine.io/lib/server.ts#L713-L732

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:
https://github.com/socketio/socket.io/blob/439a8f669c6788b512dd76011c4476830d67515b/packages/engine.io/lib/server.ts#L27

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

Dominant language
TypeScript
Stars
63.2k
Forks
10.3k
Avg merge
11d 20h
Merged PRs (30d)
2

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from socketio/socket.io

All issues in socketio/socket.io

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.