[Web Socket][Android] Sec-WebSocket-Protocol header should have a space after the comma

[justinmann]

Description

Here is the protocol string concat for Android: https://github.com/zxcpoiu/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/modules/websocket/WebSocketModule.java#L110

Here is the same logic from a Websocket library: https://github.com/theturtle32/WebSocket-Node/blob/master/lib/WebSocketClient.js#L200C53-L200C62****

I am not sure this is violation of the RFC: https://www.rfc-editor.org/rfc/rfc6455

Because it is not conventional, it does break some servers. Specifically the Deepgram Websocket Server fails on Android, but works on iOS.

Steps to reproduce

yarn run

React Native Version

0.73.0

Affected Platforms

Runtime - Android

Output of npx react-native info

System:
  OS: macOS 14.2.1
  CPU: (10) arm64 Apple M1 Pro
  Memory: 113.02 MB / 16.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 18.20.2
    path: /usr/local/bin/node
  Yarn:
    version: 1.22.22
    path: ~/Documents/GitHub/app/node_modules/.bin/yarn
  npm:
    version: 10.5.0
    path: /usr/local/bin/npm
  Watchman:
    version: 2024.03.25.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.15.2
    path: /Users/admin/.rbenv/shims/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.5
      - iOS 17.5
      - macOS 14.5
      - tvOS 17.5
      - visionOS 1.2
      - watchOS 10.5
  Android SDK: Not Found
IDEs:
  Android Studio: 2022.1 AI-221.6008.13.2211.9477386
  Xcode:
    version: 15.4/15F31d
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 11.0.18
    path: /usr/bin/javac
  Ruby:
    version: 2.7.6
    path: /Users/admin/.rbenv/shims/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react: Not Found
  react-native: Not Found
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: Not found
  newArchEnabled: Not found
iOS:
  hermesEnabled: Not found
  newArchEnabled: Not found

info React Native v0.74.2 is now available (your project is running on v0.72.8).
info Changelog: https://github.com/facebook/react-native/releases/tag/v0.74.2
info Diff: https://react-native-community.github.io/upgrade-helper/?from=0.72.8
info For more info, check out "https://reactnative.dev/docs/upgrading?os=macos".

Stacktrace or Logs

// @ts-ignore
    this._socket = new WebSocket(url.toString(), null, {
      headers: {
        'Sec-WebSocket-Protocol': ['token', this.key].join(', '),
      },
    });

Reproducer

https://github.com/react-native-community/reproducer-react-native

Screenshots and Videos

No response

[github-actions[bot]]

:warning:

Newer Version of React Native is Available!

:information_source:

You are on a supported minor version, but it looks like there's a newer patch available - 0.73.8. Please upgrade to the highest patch for your minor or latest and verify if the issue persists (alternatively, create a new project and repro the issue in it). If it does not repro, please let us know so we can close out this issue. This helps us ensure we are looking at issues that still exist in the most recent releases.

[github-actions[bot]]

:warning:	Missing Reproducible Example
:information_source:	We could not detect a reproducible example in your issue report. Please provide either: If your bug is UI related: a Snack If your bug is build/update related: use our Reproducer Template. A reproducer needs to be in a GitHub repository under your username.

[cortinico]

I am not sure this is violation of the RFC: rfc-editor.org/rfc/rfc6455

Can you point specifically to what in the RFC we're violating?

[justinmann]

As I stated above, I am not sure this is an RFC violation, it is just does not match the convention used by other WebSocket client libraries (including the iOS library for React-Native).

I believe this is relevant RFC: https://www.rfc-editor.org/rfc/rfc2616#section-4.2 "Multiple message-header fields with the same field-name MAY be present in a message if and only if the entire field-value for that header field is defined as a comma-separated list [i.e., #(values)]. It MUST be possible to combine the multiple header fields into one "field-name: field-value" pair, without changing the semantics of the message, by appending each subsequent field-value to the first, each separated by a comma."

The RFC seems to be more of a suggestion and it just says to use a "," but makes no mention of adding a space ", ".

[cortinico]

The RFC seems to be more of a suggestion and it just says to use a "," but makes no mention of adding a space ", ".

Yeah this is a bit of a weak statement. If you could provide more evidence that adding a space after the , is breaking the specs we could potentially change it.