http icon indicating copy to clipboard operation
http copied to clipboard

CupertinoWebSocket: Allow close codes as defined in RFC 6455

Open kpsroka opened this issue 1 year ago • 1 comments

This is a follow-up from issue https://github.com/dart-lang/http/issues/1203.

At this moment, the CupertinoWebSocket.close will throw if passed non-null code parameter that's not equal to 1000 or in [3000, 4999] range.

This is not in line with the RFC 6455 specification, that allows additional codes in range 1000-1010 for client use.

The WHATWG specification of the WebSocket close operation is less permissive, but since:

  • CupertinoWebSocket is not available for browser-based clients, browser-centric specifications should not apply,
  • CupertinoWebSocket wraps NSURLSessionWebSocketTask which allows RFC 6455 close codes, and
  • CupertinoWebSocket refers to RFC 6455 in comments,

I believe that it should permit all client-side codes permitted by RFC 6455.

I'm fine with providing a PR for applying this change.

kpsroka avatar Aug 13 '24 15:08 kpsroka

Hey @kpsroka

In chose the current subset of close codes because they are supported by all WebSocket implementations that I'm aware of.

Maybe it would make sense to modify the conformance tests to allow implementations to support additional close codes: https://github.com/dart-lang/http/blob/master/pkgs/web_socket_conformance_tests/lib/src/close_local_tests.dart

We could change the documentation for WebSocket.close to be something like:

  /// Closes the WebSocket connection and the [events] `Stream`.
  ///
  /// Sends a Close frame to the peer. If the optional [code] and [reason]
  /// arguments are given, they will be included in the Close frame. If no
  /// [code] is set then the peer will see a 1005 status code. If no [reason]
  /// is set then the peer will not receive a reason string.
  ///
- /// Throws an [ArgumentError] if [code] is not 1000 or in the range 3000-4999.
+ /// Throws an [ArgumentError] if the value of [code] is not supported. All implementations
+ /// support the value 1000 and values in the range 3000-4999.
  ...
  Future<void> close([int? code, String? reason]);

It would be great if you'd like to tackle this!

brianquinlan avatar Aug 13 '24 22:08 brianquinlan

Is this stale now or do we still want to land #1295?

mraleph avatar Nov 15 '24 15:11 mraleph

@mraleph Hi, Sorry for dropping this. In the end, since in our (superlist's) app we'd still want to support web, we decided not to use the additional codes available, and use the custom ones. While I believe that this issue has merit outside of our usage, I won't work on this in any foreseeable future. Please close the Issue/PR as you see fit.

kpsroka avatar Nov 15 '24 15:11 kpsroka