Invalid WebSocket frame: RSV2 and RSV3 must be clear

[marcolanaro]

Good afternoon,
I'm writing to report an important websocket issue.
I would have loved to send a PR, but the reality of the facts is that so far I've not been able to even reproduce the issue.
I'm sure this is going to be useful for someone else and we can find some workaround together.

Impact

The server crash with:

code: "WS_ERR_UNEXPECTED_RSV_2_3"
message: "Invalid WebSocket frame: RSV2 and RSV3 must be clear"
stack: "RangeError: Invalid WebSocket frame: RSV2 and RSV3 must be clear
    at Receiver.getInfo (/app/node_modules/ws/lib/receiver.js:176:14)
    at Receiver.startLoop (/app/node_modules/ws/lib/receiver.js:136:22)
    at Receiver._write (/app/node_modules/ws/lib/receiver.js:83:10)
    at writeOrBuffer (node:internal/streams/writable:392:12)
    at _write (node:internal/streams/writable:333:10)
    at Writable.write (node:internal/streams/writable:337:10)
    at Socket.socketOnData (/app/node_modules/ws/lib/websocket.js:1272:35)
    at Socket.emit (node:events:513:28)
    at addChunk (node:internal/streams/readable:324:12)
    at readableAddChunk (node:internal/streams/readable:297:9)"
type: "RangeError"

I believe this started happening since an upgrade to node 18.

Preliminary Analysis

Digging over the internet, I found some recent resources that well describe what happens.
Recent conversation on websocket/ws issue 1354 .
Recent workaround on trpc 2522.

Summary: it looks like some safari browsers are buggy and set some bits without negotiating them with the server.

If the workaround on trpc actually works, I'm not sure why this would not be enough:
https://github.com/mercurius-js/mercurius/blob/master/lib/subscription-client.js#L86

[mcollina]

Thanks for reporting! Would you like to send a Pull Request to address this issue?

[mcollina]

Also a unit test would be amazing.

[marcolanaro]

I can reliably crash mercurius locally with the following snippet:

const WebSocket = require('ws');

const ws = new WebSocket('ws://127.0.0.1:1337/graphql');

ws.on('open', function open() {
  ws._socket.write(Buffer.from([0x92, 0x00]));
});

Hopefully will help us finding a resolution!

[marcolanaro]

I think I got it.
In @fastify/websocket we would need to add this:

connection.on('error', (error) => {
  fastify.log.error(error);
});

in https://github.com/fastify/fastify-websocket/blob/master/index.js#L63

I'll work on a PR later.

[marcolanaro]

I've opened a PR on @fastify/websocket: fastify/fastify-websocket#228.

[marcolanaro]

@mcollina thank you for merging the PR.
As soon as you release a new version I'll be happy to create a PR here.
I would love to get my fix out before the weekend :)

[mcollina]

The @fastify/websocket change shipped!

[marcolanaro]

@mcollina the PR is ready ❤️

[mcollina]

This was fixed in @fastify/websocket v7.1.1.

[marcolanaro]

I just figured out the issue still happen in some scenarios. Will dig more tomorrow, but I'm able to crash my server defining the protocol, e.g.:

const WebSocket = require('ws');

const ws = new WebSocket('ws://127.0.0.1:1337/graphql', 'graphql-ws');

ws.on('open', function open() {
  ws._socket.write(Buffer.from([0xa2, 0x00]));
});