oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
71.88k stars 2.56k forks source link

websocket server.upgrade fails if Request is not original request #11382

Open beorn opened 1 month ago

beorn commented 1 month ago

What version of Bun is running?

1.1.8

What platform is your computer?

Darwin 23.5.0 arm64 arm

What steps can reproduce the bug?

import { ServerWebSocket } from "bun"

Bun.serve<{}>({
  fetch: async (req, server) => {
    // const res = server.upgrade(req) // works
    const res = server.upgrade(new Request(req.url, req)) // doesn't work
    console.log("upgraded", res)
    if (res) return
    return new Response("Upgrade failed", { status: 500 })
  },
  websocket: {
    open(ws: ServerWebSocket<{}>): void | Promise<void> {
      console.log("open")
    },
    close(ws: ServerWebSocket<{}>, code: number, reason: string): void | Promise<void> {
      console.log("closed", code)
    },
    message(ws, message) {
      console.log("message", message)
    }
  }
})

new WebSocket("http://localhost:3000")

What is the expected behavior?

It succeeds in upgrading the request even if the request was not the exact request that was passed to fetch.

What do you see instead?

It fails (res=false).

Additional information

I'm trying to add support for behind-proxy scenarios that require rewriting the request to take into account X-Forwarded-* headers, similar to Express's "trust proxy" feature.

PaddeK commented 1 month ago

You can set headers in the second parameter of upgrade.. maybe this could do the trick for you.

beorn commented 1 month ago

Thanks, though in this scenario I don't so much need the headers as a new request that is rewritten based on the headers (URL incl. proto changed, headers removed). (I don't think this is expected behavior?)

PaddeK commented 1 month ago

You could be right and this is just another bug in the websocket implementation. I have a different problem with handling upgrades and I hope with each new release that the websocket implementation has been improved. But unfortunately there is not too much interest in websockets, so the priority seems to be quite low.

beorn commented 1 month ago

Perhaps related to

https://github.com/oven-sh/bun/issues/1381 https://github.com/honojs/middleware/issues/81#issuecomment-1509457656