Auto merge of #9400 - jmr0:websocket, r=nox

Fixing websocket subprotocol header validation

This takes care of https://github.com/servo/servo/issues/9034

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9400)
<!-- Reviewable:end -->

components/script/dom/bindings/str.rs

@@ -11,6 +11,7 @@ use std::ops;
 use std::str;
 use std::str::FromStr;
 use util::mem::HeapSizeOf;
+use util::str::is_token;
 
 /// Encapsulates the IDL `ByteString` type.
 #[derive(JSTraceable, Clone, Eq, PartialEq, HeapSizeOf)]
@@ -49,35 +50,7 @@ impl ByteString {
     /// [RFC 2616](http://tools.ietf.org/html/rfc2616#page-17).
     pub fn is_token(&self) -> bool {
         let ByteString(ref vec) = *self;
-        if vec.is_empty() {
+        is_token(vec)
-            return false; // A token must be at least a single character
-        }
-        vec.iter().all(|&x| {
-            // http://tools.ietf.org/html/rfc2616#section-2.2
-            match x {
-                0...31 | 127 => false, // CTLs
-                40 |
-                41 |
-                60 |
-                62 |
-                64 |
-                44 |
-                59 |
-                58 |
-                92 |
-                34 |
-                47 |
-                91 |
-                93 |
-                63 |
-                61 |
-                123 |
-                125 |
-                32 => false, // separators
-                x if x > 127 => false, // non-CHARs
-                _ => true,
-            }
-        })
     }
 
     /// Returns whether `self` is a `field-value`, as defined by

components/script/dom/websocket.rs

@@ -36,11 +36,12 @@ use net_traits::unwrap_websocket_protocol;
 use net_traits::{WebSocketCommunicate, WebSocketConnectData, WebSocketDomAction, WebSocketNetworkEvent};
 use script_thread::ScriptThreadEventCategory::WebSocketEvent;
 use script_thread::{CommonScriptMsg, Runnable, ScriptChan};
+use std::ascii::AsciiExt;
 use std::borrow::ToOwned;
 use std::cell::Cell;
 use std::ptr;
 use std::thread;
-use util::str::DOMString;
+use util::str::{DOMString, is_token};
 use websocket::client::request::Url;
 use websocket::header::{Headers, WebSocketProtocol};
 use websocket::ws::util::url::parse_url;
@@ -220,17 +221,13 @@ impl WebSocket {
         for (i, protocol) in protocols.iter().enumerate() {
             // https://tools.ietf.org/html/rfc6455#section-4.1
             // Handshake requirements, step 10
-            if protocol.is_empty() {
+
+            if protocols[i + 1..].iter().any(|p| p.eq_ignore_ascii_case(protocol)) {
                 return Err(Error::Syntax);
             }
 
-            if protocols[i + 1..].iter().any(|p| p == protocol) {
-                return Err(Error::Syntax);
-            }
-
-            // TODO: also check that no separator characters are used
             // https://tools.ietf.org/html/rfc6455#section-4.1
-            if protocol.chars().any(|c| c < '\u{0021}' || c > '\u{007E}') {
+            if !is_token(protocol.as_bytes()) {
                 return Err(Error::Syntax);
             }
         }

components/util/str.rs

@@ -565,3 +565,37 @@ pub fn search_index(index: usize, indices: CharIndices) -> isize {
     }
     character_count
 }
+
+/// Returns whether `s` is a `token`, as defined by
+/// [RFC 2616](http://tools.ietf.org/html/rfc2616#page-17).
+pub fn is_token(s: &[u8]) -> bool {
+    if s.is_empty() {
+        return false; // A token must be at least a single character
+    }
+    s.iter().all(|&x| {
+        // http://tools.ietf.org/html/rfc2616#section-2.2
+        match x {
+            0...31 | 127 => false, // CTLs
+            40 |
+            41 |
+            60 |
+            62 |
+            64 |
+            44 |
+            59 |
+            58 |
+            92 |
+            34 |
+            47 |
+            91 |
+            93 |
+            63 |
+            61 |
+            123 |
+            125 |
+            32 => false, // separators
+            x if x > 127 => false, // non-CHARs
+            _ => true,
+        }
+    })
+}

tests/wpt/metadata/MANIFEST.json

@@ -33316,7 +33316,22 @@
   },
   "local_changes": {
     "deleted": [],
-    "items": {},
+    "items": {
+      "testharness": {
+        "websockets/Create-asciiSep-protocol-string.htm": [
+          {
+            "path": "websockets/Create-asciiSep-protocol-string.htm",
+            "url": "/websockets/Create-asciiSep-protocol-string.htm"
+          }
+        ],
+        "websockets/Create-protocols-repeated-case-insensitive.htm": [
+          {
+            "path": "websockets/Create-protocols-repeated-case-insensitive.htm",
+            "url": "/websockets/Create-protocols-repeated-case-insensitive.htm"
+          }
+        ]
+      }
+    },
     "reftest_nodes": {}
   },
   "reftest_nodes": {

tests/wpt/web-platform-tests/websockets/Create-asciiSep-protocol-string.htm

@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>W3C WebSocket API - Create WebSocket - ascii protocol string with separator</title>
+    <script type="text/javascript" src="/resources/testharness.js"></script>
+    <script type="text/javascript" src="/resources/testharnessreport.js"></script>
+    <script type="text/javascript" src="websocket.js?pipe=sub"></script>
+</head>
+<body>
+    <div id="log"></div>
+    <script type="text/javascript">
+        if(window.WebSocket) {
+            test(function () {
+                var asciiWithSep = "/echo";
+                var wsocket;
+                assert_throws("SYNTAX_ERR", function () { wsocket = CreateWebSocketWithAsciiSep(asciiWithSep) });
+            }, "W3C WebSocket API - Create WebSocket - Pass a valid URL and a protocol string with an ascii separator character - SYNTAX_ERR is thrown")
+        }
+    </script>
+</body>
+</html>

tests/wpt/web-platform-tests/websockets/Create-protocols-repeated-case-insensitive.htm

@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>W3C WebSocket API - Create WebSocket - repeated protocols with different case</title>
+    <script type="text/javascript" src="/resources/testharness.js"></script>
+    <script type="text/javascript" src="/resources/testharnessreport.js"></script>
+    <script type="text/javascript" src="websocket.js?pipe=sub"></script>
+</head>
+<body>
+    <div id="log"></div>
+    <script type="text/javascript">
+        test(function () {
+            var wsocket;
+            assert_throws("SYNTAX_ERR", function () { wsocket = CreateWebSocketWithRepeatedProtocolsCaseInsensitive() });
+        }, "W3C WebSocket API - Create WebSocket - Pass a valid URL and an array of protocol strings with repeated values but different case - SYNTAX_ERR is thrown")
+    </script>
+</body>
+</html>

tests/wpt/web-platform-tests/websockets/websocket.js

@@ -8,6 +8,7 @@ var __CONTROLPATH = "control";
 var __PROTOCOL = "echo";
 var __PROTOCOLS = ["echo", "chat"];
 var __REPEATED__PROTOCOLS = ["echo", "echo"];
+var __REPEATED__PROTOCOLS_CASE_INSENSITIVE = ["echo", "eCho"];
 var __URL;
 var __IS__WEBSOCKET;
 var __PASS = "Pass";
@@ -47,6 +48,12 @@ function CreateWebSocketNonAsciiProtocol(nonAsciiProtocol) {
     wsocket = new WebSocket(__URL, nonAsciiProtocol);
 }
 
+function CreateWebSocketWithAsciiSep(asciiWithSep) {
+    IsWebSocket();
+    __URL = "ws://" + __SERVER__NAME + ":" + __PORT + "/" + __PATH;
+    wsocket = new WebSocket(__URL, asciiWithSep);
+}
+
 function CreateWebSocketWithBlockedPort(blockedPort) {
     IsWebSocket();
     __URL = "wss://" + __SERVER__NAME + ":" + blockedPort + "/" + __PATH;
@@ -71,6 +78,12 @@ function CreateWebSocketWithRepeatedProtocols() {
     wsocket = new WebSocket(__URL, __REPEATED__PROTOCOLS);
 }
 
+function CreateWebSocketWithRepeatedProtocolsCaseInsensitive() {
+    IsWebSocket();
+    __URL = "ws://" + __SERVER__NAME + ":" + __PORT + "/" + __PATH;
+    wsocket = new WebSocket(__URL, __REPEATED__PROTOCOLS_CASE_INSENSITIVE);
+}
+
 function CreateWebSocket(isSecure, isProtocol, isProtocols) {
     IsWebSocket();
     if (isSecure) {