Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CupertinoWebSocket: Allow close codes as defined in RFC 6455 #1294

Open
kpsroka opened this issue Aug 13, 2024 · 3 comments
Open

CupertinoWebSocket: Allow close codes as defined in RFC 6455 #1294

kpsroka opened this issue Aug 13, 2024 · 3 comments
Assignees
Labels
package:cupertino_http Issues related to package:cupertino_http type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@kpsroka
Copy link

kpsroka commented Aug 13, 2024

This is a follow-up from issue #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 kpsroka added package:cupertino_http Issues related to package:cupertino_http type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Aug 13, 2024
@brianquinlan
Copy link
Collaborator

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!

@mraleph
Copy link
Member

mraleph commented Nov 15, 2024

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

@kpsroka
Copy link
Author

kpsroka commented Nov 15, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:cupertino_http Issues related to package:cupertino_http type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants