Bug 1819280 - [websocket] Enforce full subprotocol char checks r=valentin

Enforce all subprotocol character checks as per RFC 6455 with a new method.
(See bug for full check details.)

Differential Revision: https://phabricator.services.mozilla.com/D171219

dom/websocket/WebSocket.cpp

@@ -1255,6 +1255,34 @@ class AsyncOpenRunnable final : public WebSocketMainThreadRunnable {
 
 }  // namespace
 
+// Check a protocol entry contains only valid characters
+bool WebSocket::IsValidProtocolString(const nsString& aValue) {
+  // RFC 6455 (4.1): "not including separator characters as defined in RFC 2616"
+  const char16_t illegalCharacters[] = {0x28, 0x29, 0x3C, 0x3E, 0x40, 0x2C,
+                                        0x3B, 0x3A, 0x5C, 0x22, 0x2F, 0x5B,
+                                        0x5D, 0x3F, 0x3D, 0x7B, 0x7D};
+
+  // Cannot be empty string
+  if (aValue.IsEmpty()) {
+    return false;
+  }
+
+  const auto* start = aValue.BeginReading();
+  const auto* end = aValue.EndReading();
+
+  auto charFilter = [&](char16_t c) {
+    // RFC 6455 (4.1 P18): "in the range U+0021 to U+007E"
+    if (c < 0x21 || c > 0x7E) {
+      return true;
+    }
+
+    return std::find(std::begin(illegalCharacters), std::end(illegalCharacters),
+                     c) != std::end(illegalCharacters);
+  };
+
+  return std::find_if(start, end, charFilter) == end;
+}
+
 already_AddRefed<WebSocket> WebSocket::ConstructorCommon(
     const GlobalObject& aGlobal, const nsAString& aUrl,
     const Sequence<nsString>& aProtocols,
@@ -1291,15 +1319,14 @@ already_AddRefed<WebSocket> WebSocket::ConstructorCommon(
   for (uint32_t index = 0, len = aProtocols.Length(); index < len; ++index) {
     const nsString& protocolElement = aProtocols[index];
 
-    if (protocolElement.IsEmpty()) {
+    // Repeated protocols are not allowed
-      aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR);
-      return nullptr;
-    }
     if (protocolArray.Contains(protocolElement)) {
       aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR);
       return nullptr;
     }
-    if (protocolElement.FindChar(',') != -1) /* interferes w/list */ {
+
+    // Protocol string value must match constraints
+    if (!IsValidProtocolString(protocolElement)) {
       aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR);
       return nullptr;
     }
@@ -1718,11 +1745,8 @@ nsresult WebSocketImpl::Init(JSContext* aCx, bool aIsSecure,
 
   // Assign the sub protocol list and scan it for illegal values
   for (uint32_t index = 0; index < aProtocolArray.Length(); ++index) {
-    for (uint32_t i = 0; i < aProtocolArray[index].Length(); ++i) {
+    if (!WebSocket::IsValidProtocolString(aProtocolArray[index])) {
-      if (aProtocolArray[index][i] < static_cast<char16_t>(0x0021) ||
+      return NS_ERROR_DOM_SYNTAX_ERR;
-          aProtocolArray[index][i] > static_cast<char16_t>(0x007E)) {
-        return NS_ERROR_DOM_SYNTAX_ERR;
-      }
     }
 
     if (!mRequestedProtocolList.IsEmpty()) {

dom/websocket/WebSocket.h

@@ -140,6 +140,8 @@ class WebSocket final : public DOMEventTargetHelper {
   nsresult CreateAndDispatchCloseEvent(bool aWasClean, uint16_t aCode,
                                        const nsAString& aReason);
 
+  static bool IsValidProtocolString(const nsString& aValue);
+
   // if there are "strong event listeners" (see comment in WebSocket.cpp) or
   // outgoing not sent messages then this method keeps the object alive
   // when js doesn't have strong references to it.

testing/web-platform/meta/websockets/Create-asciiSep-protocol-string.any.js.ini

@@ -1,40 +1,23 @@
 [Create-asciiSep-protocol-string.any.html]
   expected:
     if (os == "android") and fission: [OK, TIMEOUT]
-  [Create WebSocket - Pass a valid URL and a protocol string with an ascii separator character - SYNTAX_ERR is thrown]
-    expected: FAIL
-
 
 [Create-asciiSep-protocol-string.any.worker.html]
   expected:
     if (os == "android") and fission: [OK, TIMEOUT]
-  [Create WebSocket - Pass a valid URL and a protocol string with an ascii separator character - SYNTAX_ERR is thrown]
-    expected: FAIL
-
 
 [Create-asciiSep-protocol-string.any.html?wss]
   expected:
     if (os == "android") and fission: [OK, TIMEOUT]
-  [Create WebSocket - Pass a valid URL and a protocol string with an ascii separator character - SYNTAX_ERR is thrown]
-    expected: FAIL
-
 
 [Create-asciiSep-protocol-string.any.worker.html?wss]
   expected:
     if (os == "android") and fission: [OK, TIMEOUT]
-  [Create WebSocket - Pass a valid URL and a protocol string with an ascii separator character - SYNTAX_ERR is thrown]
-    expected: FAIL
-
 
 [Create-asciiSep-protocol-string.any.worker.html?wpt_flags=h2]
   expected:
     if (os == "android") and fission: [OK, TIMEOUT]
-  [Create WebSocket - Pass a valid URL and a protocol string with an ascii separator character - SYNTAX_ERR is thrown]
-    expected: FAIL
-
 
 [Create-asciiSep-protocol-string.any.html?wpt_flags=h2]
   expected:
     if (os == "android") and fission: [OK, TIMEOUT]
-  [Create WebSocket - Pass a valid URL and a protocol string with an ascii separator character - SYNTAX_ERR is thrown]
-    expected: FAIL

testing/web-platform/meta/websockets/binaryType-wrong-value.any.js.ini

@@ -16,24 +16,11 @@
 
 [binaryType-wrong-value.any.worker.html?wpt_flags=h2]
   expected:
-    if (os == "android") and debug and not swgl: [OK, ERROR, TIMEOUT]
+    [OK, ERROR, TIMEOUT]
-    if (os == "win") and debug: [OK, ERROR, TIMEOUT]
-    if (os == "android") and not debug: [OK, ERROR]
-  [Create WebSocket - set binaryType to something other than blob or arraybuffer - SYNTAX_ERR is returned - Connection should be closed]
-    expected:
-      if os == "mac": [PASS, NOTRUN]
-
 
 [binaryType-wrong-value.any.html?wpt_flags=h2]
   expected:
-    if (os == "win") and debug and (processor == "x86_64") and swgl: [OK, ERROR, TIMEOUT]
+    [OK, ERROR, TIMEOUT]
-    if (os == "android") and debug and not swgl: [OK, ERROR, TIMEOUT]
-    if (os == "win") and debug and (processor == "x86"): [OK, ERROR, TIMEOUT]
-    if (os == "android") and not debug: [OK, TIMEOUT]
-  [Create WebSocket - set binaryType to something other than blob or arraybuffer - SYNTAX_ERR is returned - Connection should be closed]
-    expected:
-      if os == "mac": [PASS, NOTRUN]
-
 
 [binaryType-wrong-value.any.html]
   expected: