fix!: do not automatically return `Sec-WebSocket-Protocol` header

[teemingc]

fixes https://github.com/unjs/crossws/issues/141

This PR includes a breaking change that modifies the behaviour of the Node and Deno adapters to no longer automatically accept the first WebSocket protocol the client sends. This makes the behaviour the same across all adapters and prevents errors for Node and Deno when trying to return the Sec-WebSocket-Protocol header from the upgrade hook.

[pi0]

Thx for PR. I want to try avoiding breaking change if possible. Couldn't we check to only do this if custom Sec-WebSocket-Protocol is returned in upgrade hook?

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Please upload report for BASE (main@39485dc). Learn more about missing BASE report.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #142   +/-   ##
=======================================
  Coverage        ?   76.53%           
=======================================
  Files           ?        9           
  Lines           ?      733           
  Branches        ?      147           
=======================================
  Hits            ?      561           
  Misses          ?      170           
  Partials        ?        2           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pi0]

(if we could have a test would be nice then we can try different options)

[teemingc]

Thx for PR. I want to try avoiding breaking change if possible. Couldn't we check to only do this if custom Sec-WebSocket-Protocol is returned in upgrade hook?

I'm not sure if we should do that because we also need to support the case where the user omits the sec-websocket-protocol header to disagree with using the provided subprotocols during the handshake:

The absence of such a field is equivalent to the null value (meaning that if the server does not wish to agree to one of the suggested subprotocols, it MUST NOT send back a |Sec-WebSocket-Protocol| header field in its response). The empty string is not the same as the null value for these purposes and is not a legal value for this field.

https://datatracker.ietf.org/doc/html/rfc6455#section-4.2.2

I'll add a test soon.

[pi0]

we also need to support the case where the user omits the sec-websocket-protocol header to disagree with using the provided subprotocols during the handshake:

Perhaps we can allow headers["sec-websocket-protocol"] = "" | null as signal?

I have not done enough research, but if both Deno and Node.js WS have a default behavior of accepting the first protocol, I (presume) there has been a reason. (being able to override/omit it is totally valid though)

(also thanks for adding tests ❤️)

[teemingc]

I've added tests but there's an issue with Bun not being able to configure the automatic sub-protocol returning behaviour. I've filed an issue for it https://github.com/oven-sh/bun/issues/18243

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

:loudspeaker: Thoughts on this report? Let us know!