Ignore protocol header if handler did not specify allowed protocols (#28)
Closes #27
The spec says "if the server does not agree to any of the client's
requested subprotocols, the only acceptable value is null". The mozilla
docs for implementing a web socket server say "The client may close the
connection if it doesn't get the subprotocol it wants."
From this I take it that the safest thing to do is to ignore the
protocols. A server which wants to be more careful can handle the `null`
protocol in whatever way makes sense.
- Add a null check before searching for overlap in protocol.
- Remove the explicit check for `protocols` with 2 argument callback.
- Rename `protocols` to `requestProtocols`.
- Add tests demonstrating new behavior, remove test for ArgumentError.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 66609de..4a33e02 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 0.2.4-dev
+
+* Allow omitting `protocols` argument even if the `onConnection` callback takes
+ a second argument.
+
## 0.2.3
* Add `pingInterval` argument to `webSocketHandler`, to be passed through
diff --git a/lib/shelf_web_socket.dart b/lib/shelf_web_socket.dart
index c3bedac..c9cbc19 100644
--- a/lib/shelf_web_socket.dart
+++ b/lib/shelf_web_socket.dart
@@ -25,8 +25,8 @@
/// argument. The subprotocol is determined by looking at the client's
/// `Sec-WebSocket-Protocol` header and selecting the first entry that also
/// appears in [protocols]. If no subprotocols are shared between the client and
-/// the server, `null` will be passed instead. Note that if [onConnection] takes
-/// two arguments, [protocols] must be passed.
+/// the server, `null` will be passed instead and no subprotocol heaader will be
+/// sent to the client which may cause it to disconnect.
///
/// [WebSocket subprotocol]: https://tools.ietf.org/html/rfc6455#section-1.9
///
@@ -46,11 +46,6 @@
Iterable<String> allowedOrigins,
Duration pingInterval}) {
if (onConnection is! _BinaryFunction) {
- if (protocols != null) {
- throw ArgumentError('If protocols is non-null, onConnection must '
- 'take two arguments, the WebSocket and the protocol.');
- }
-
var innerOnConnection = onConnection;
onConnection = (webSocket, _) => innerOnConnection(webSocket);
}
diff --git a/lib/src/web_socket_handler.dart b/lib/src/web_socket_handler.dart
index 93d8976..6f3620b 100644
--- a/lib/src/web_socket_handler.dart
+++ b/lib/src/web_socket_handler.dart
@@ -92,11 +92,12 @@
///
/// If no matching protocol can be found, returns `null`.
String _chooseProtocol(Request request) {
- var protocols = request.headers['Sec-WebSocket-Protocol'];
- if (protocols == null) return null;
- for (var protocol in protocols.split(',')) {
- protocol = protocol.trim();
- if (_protocols.contains(protocol)) return protocol;
+ var requestProtocols = request.headers['Sec-WebSocket-Protocol'];
+ if (requestProtocols == null) return null;
+ if (_protocols == null) return null;
+ for (var requestProtocol in requestProtocols.split(',')) {
+ requestProtocol = requestProtocol.trim();
+ if (_protocols.contains(requestProtocol)) return requestProtocol;
}
return null;
}
diff --git a/test/web_socket_test.dart b/test/web_socket_test.dart
index 9a56f53..e77cad9 100644
--- a/test/web_socket_test.dart
+++ b/test/web_socket_test.dart
@@ -67,6 +67,37 @@
}
});
+ test('handles protocol header without allowed protocols', () async {
+ var server = await shelf_io.serve(webSocketHandler((webSocket) {
+ webSocket.sink.close();
+ }), 'localhost', 0);
+
+ try {
+ var webSocket = await WebSocket.connect('ws://localhost:${server.port}',
+ protocols: ['one', 'two', 'three']);
+ expect(webSocket.protocol, isNull);
+ return webSocket.close();
+ } finally {
+ await server.close();
+ }
+ });
+
+ test('allows two argument callbacks without protocols', () async {
+ var server = await shelf_io.serve(webSocketHandler((webSocket, protocol) {
+ expect(protocol, isNull);
+ webSocket.sink.close();
+ }), 'localhost', 0);
+
+ try {
+ var webSocket = await WebSocket.connect('ws://localhost:${server.port}',
+ protocols: ['one', 'two', 'three']);
+ expect(webSocket.protocol, isNull);
+ return webSocket.close();
+ } finally {
+ await server.close();
+ }
+ });
+
group('with a set of allowed origins', () {
HttpServer server;
Uri url;
@@ -170,11 +201,6 @@
expect(http.get(url, headers: headers), hasStatus(400));
});
});
-
- test('throws an error if a unary function is provided with protocols', () {
- expect(() => webSocketHandler((_) => null, protocols: ['foo']),
- throwsArgumentError);
- });
}
Matcher hasStatus(int status) => completion(predicate((response) {