Prevent duplicate protocols

[penalosa]

I'm trying to use SvelteKit with Cloudflare Pages, and as part of that I'd like to be able to develop locally using wrangler pages dev. I noticed that had an option to use --experimental-local, which I added. When I ran the full command wrangler pages dev --compatibility-date=2023-02-03 --experimental-local -- npm run dev (and after upgrading Wrangler's Miniflare version to 3.0.0-next.10), I found that the Pages dev command crashed with the following error:

/Users/penalosa/dev/wrangler2/packages/wrangler/wrangler-dist/cli.js:26719
            throw a;
            ^

TypeError: First argument must be a valid error code number
    at Sender.close (/Users/penalosa/dev/wrangler2/node_modules/@miniflare/tre/node_modules/ws/lib/sender.js:162:13)
    at WebSocket.close (/Users/penalosa/dev/wrangler2/node_modules/@miniflare/tre/node_modules/ws/lib/websocket.js:300:18)
    at _WebSocket.<anonymous> (/Users/penalosa/dev/wrangler2/node_modules/@miniflare/tre/dist/src/index.js:2562:10)
    at [nodejs.internal.kHybridDispatch] (node:internal/event_target:689:20)
    at _WebSocket.dispatchEvent (node:internal/event_target:631:26)
    at _WebSocket.dispatchEvent (/Users/penalosa/dev/wrangler2/node_modules/@miniflare/tre/dist/src/index.js:2132:18)
    at _WebSocket.queuingDispatchToPair_fn (/Users/penalosa/dev/wrangler2/node_modules/@miniflare/tre/dist/src/index.js:2511:10)
    at [kClose] (/Users/penalosa/dev/wrangler2/node_modules/@miniflare/tre/dist/src/index.js:2498:82)
    at WebSocket.<anonymous> (/Users/penalosa/dev/wrangler2/node_modules/@miniflare/tre/dist/src/index.js:2546:19)
    at WebSocket.emit (node:events:525:35)

Node.js v18.7.0

I knew this was related to HMR, as it was the only websocket that my SvelteKit page opened. Curiously, the HMR websocket connection showed as having failed to open in my browser DevTools, with Firefox logging a "Failed to connect" error. However, a successful handshake was shown (the Firefox error came slightly later). After a bit of further digging, it appeared that both sides of the WebSocket pair were being closed with the code 1006, a reserved code. Here start the digressions that were ultimately fruitless, included for completeness:

• Checking whether Vite or svelte-hmr (the HMR package that Svelte uses) could ever incorrectly send a 1006 close code (they don't)
• Manually verifying the sec-websocket-key and sec-websocket-accept headers (they matched)

I then moved on to checking the connection with curl, and noticed something rather curious that I'd missed in the browser DevTools:

> GET / HTTP/1.1
> Host: localhost:8788
> Sec-WebSocket-Protocol: vite-hmr
> ...

< HTTP/1.1 101 Switching Protocols
< Sec-WebSocket-Protocol: vite-hmr, vite-hmr
< ...

The protocol vite-hmr was being sent, but the protocol vite-hmr, vite-hmr was being received. This meant that the browser was rejecting the WebSocket connection, even though the handshake had "succeeded", and presumably closing the connection with 1006, in accordance with https://www.rfc-editor.org/rfc/rfc6455#section-11.3.4.

the |Sec-WebSocket-Protocol| header field MUST NOT appear more than once in an HTTP response.

After many (many) added console.log statements, I tracked down the offending chunk of code to the #webSocketExtraHeaders property. When headers are requested from it (this.#webSocketServer.on("headers"), a duplicate Sec-WebSocket-Protocol was being added (the only other extra headers present, Connection and Sec-WebSocket-Accept, were already being filtered out). Thus ends my tale of woe—culminating in a one line change 😂.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 269b5b3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[mrbbot]

Closing this in favour of #493, this fix means you wouldn't be able to handle WebSocket protocols in Worker code:

export default {
  async fetch() {
    const [ws1, ws2] = Object.values(new WebSocketPair());
    return new Response(null, {
      webSocket: ws1,
      headers: {
        "Sec-WebSocket-Protocol": "protocol" // would get stripped
      }
    });
  }
}