Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebSocket subprotocol is not adhered to when using fetch. #106

Closed
f5io opened this issue Nov 30, 2021 · 1 comment
Closed

WebSocket subprotocol is not adhered to when using fetch. #106

f5io opened this issue Nov 30, 2021 · 1 comment
Labels
bug Something isn't working fixed-in-next Fixed in next release

Comments

@f5io
Copy link

f5io commented Nov 30, 2021

I'm currently having issues with how subprotocols are handled whilst trying to create a WebSocket proxy to an upstream WebSocket server. The offending code seems to be the instantiation of the StandardWebSocket from ws here in src/modules/standards.ts.

From reading the ws source code I can see that the constructor takes an optional second argument of subprotocols. My problem lies in the fact that the subprotocol is forwarded from my client so exists in the headers under Sec-WebSocket-Protocol. Therefore the request is passed on to the upstream, but because the protocols are not correctly passed to the constructor, ws errors when the response from the upstream contains the same header value as it is considered unexpected.

There is a reasonably quick solution for this, as far as I am aware, which is to extract Sec-WebSocket-Protocol headers at the point of constructing the StandardWebSocket, for example...

let protocol = [];
if (request.headers.get("sec-websocket-protocol") != null) {
  protocol = request.headers.get("sec-websocket-protocol").split(/,\s?/g);
}
const ws = new StandardWebSocket(request.url, protocol, {
  followRedirects: request.redirect === "follow",
  maxRedirects: request.follow,
  headers,
});

I am not aware if this is a limitation of Cloudflare Workers themselves and is purely mirroring their behaviour. Any help would be appreciated. I am also willing to open a PR to potentially fix this, however I am not sure how I would go about testing it.

@mrbbot mrbbot added bug Something isn't working fixed-in-next Fixed in next release labels Dec 2, 2021
@mrbbot
Copy link
Contributor

mrbbot commented Dec 8, 2021

Hey! 👋 miniflare@2.0.0-rc.3 has just been released, including the fix for this. You can find the changelog here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed-in-next Fixed in next release
Projects
None yet
Development

No branches or pull requests

2 participants