Auto merge of #6127 - Nashenas88:websockets-invalid-urls, r=Ms2ger

Fix #6061.

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

components/script/dom/websocket.rs

@@ -9,8 +9,7 @@ use dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull;
 use dom::bindings::codegen::InheritTypes::EventTargetCast;
 use dom::bindings::codegen::InheritTypes::EventCast;
 use dom::bindings::error::{Error, Fallible};
-use dom::bindings::error::Error::InvalidAccess;
+use dom::bindings::error::Error::{InvalidAccess, Syntax};
-use dom::bindings::error::Error::Syntax;
 use dom::bindings::global::{GlobalField, GlobalRef};
 use dom::bindings::js::{Temporary, JSRef, Rootable};
 use dom::bindings::refcounted::Trusted;
@@ -34,7 +33,6 @@ use websocket::stream::WebSocketStream;
 use websocket::client::request::Url;
 use websocket::Client;
 
-
 #[derive(PartialEq, Copy, Clone)]
 #[jstraceable]
 enum WebSocketRequestState {
@@ -64,6 +62,45 @@ pub struct WebSocket {
     sendCloseFrame: Cell<bool>
 }
 
+fn parse_web_socket_url(url_str: &str) -> Fallible<(Url, String, u16, String, bool)> {
+    // https://html.spec.whatwg.org/multipage/#parse-a-websocket-url's-components
+    // Steps 1 and 2
+    let parsed_url = Url::parse(url_str);
+    let parsed_url = match parsed_url {
+        Ok(parsed_url) => parsed_url,
+        Err(_) => return Err(Error::Syntax),
+    };
+
+    // Step 4
+    if parsed_url.fragment != None {
+        return Err(Error::Syntax);
+    }
+
+    // Steps 3 and 5
+    let secure = match parsed_url.scheme.as_ref() {
+        "ws" => false,
+        "wss" => true,
+        _ => return Err(Error::Syntax), // step 3
+    };
+
+    let host = parsed_url.host().unwrap().serialize(); // Step 6
+    let port = parsed_url.port_or_default().unwrap(); // Step 7
+    let mut resource = parsed_url.path().unwrap().connect("/"); // Step 9
+    if resource.is_empty() {
+        resource = "/".to_owned(); // Step 10
+    }
+
+    // Step 11
+    if let Some(ref query) = parsed_url.query {
+        resource.push('?');
+        resource.push_str(query);
+    }
+
+    // Step 12
+    // FIXME remove parsed_url once it's no longer used in WebSocket::new
+    Ok((parsed_url, host, port, resource, secure))
+}
+
 impl WebSocket {
     pub fn new_inherited(global: GlobalRef, url: DOMString) -> WebSocket {
         WebSocket {
@@ -84,11 +121,12 @@ impl WebSocket {
 
     }
 
-    pub fn new(global: GlobalRef, url: DOMString) -> Temporary<WebSocket> {
+    pub fn new(global: GlobalRef, url: DOMString) -> Fallible<Temporary<WebSocket>> {
         /*TODO: This constructor is only a prototype, it does not accomplish the specs
           defined here:
           http://html.spec.whatwg.org
-          All 9 items must be satisfied.
+          Item 1 is already satisfied.
+          The remaining 8 items must be satisfied.
           TODO: This constructor should be responsible for spawning a thread for the
           receive loop after ws_root.r().Open() - See comment
         */
@@ -96,7 +134,13 @@ impl WebSocket {
                                          global,
                                          WebSocketBinding::Wrap).root();
         let ws_root = ws_root.r();
-        let parsed_url = Url::parse(&ws_root.url).unwrap();
+
+        // FIXME extract the right variables once Client::connect implementation is
+        // fixed to follow the RFC 6455 properly
+        let (parsed_url, _, _, _, _) = try!(parse_web_socket_url(&ws_root.url));
+
+        // TODO Client::connect does not conform to RFC 6455
+        // see https://github.com/cyderize/rust-websocket/issues/38
         let request = Client::connect(parsed_url).unwrap();
         let response = request.send().unwrap();
         response.validate().unwrap();
@@ -107,8 +151,9 @@ impl WebSocket {
         let failed = ws_root.failed.get();
         if failed && (ready_state == WebSocketRequestState::Closed || ready_state == WebSocketRequestState::Closing) {
             //Do nothing else. Let the close finish.
-            return Temporary::from_rooted(ws_root);
+            return Ok(Temporary::from_rooted(ws_root));
         }
+
         let (temp_sender, temp_receiver) = response.begin().split();
         let mut other_sender = ws_root.sender.borrow_mut();
         let mut other_receiver = ws_root.receiver.borrow_mut();
@@ -133,11 +178,11 @@ impl WebSocket {
           it confirms the websocket is now closed. This requires the close event
           to be fired (dispatch_close fires the close event - see implementation below)
         */
-        Temporary::from_rooted(ws_root)
+        Ok(Temporary::from_rooted(ws_root))
     }
 
     pub fn Constructor(global: GlobalRef, url: DOMString) -> Fallible<Temporary<WebSocket>> {
-        Ok(WebSocket::new(global, url))
+        WebSocket::new(global, url)
     }
 }
 

tests/wpt/metadata/websockets/Create-Secure-url-with-space.htm.ini

@@ -1,3 +0,0 @@
-[Create-Secure-url-with-space.htm]
-  type: testharness
-  expected: TIMEOUT

tests/wpt/metadata/websockets/Create-invalid-urls.htm.ini

@@ -1,3 +0,0 @@
-[Create-invalid-urls.htm]
-  type: testharness
-  expected: TIMEOUT

tests/wpt/metadata/websockets/Create-non-absolute-url.htm.ini

@@ -1,3 +0,0 @@
-[Create-non-absolute-url.htm]
-  type: testharness
-  expected: TIMEOUT

tests/wpt/metadata/websockets/Create-wrong-scheme.htm.ini

@@ -1,3 +0,0 @@
-[Create-wrong-scheme.htm]
-  type: testharness
-  expected: TIMEOUT

tests/wpt/metadata/websockets/constructor/002.html.ini

@@ -1,3 +0,0 @@
-[002.html]
-  type: testharness
-  expected: TIMEOUT

tests/wpt/metadata/websockets/constructor/006.html.ini

@@ -1,3 +1,4 @@
 [006.html]
   type: testharness
-  expected: TIMEOUT
+  [WebSockets: converting first arguments]
+    expected: FAIL