From 31b709a7c23c0fc7096c72d6024169c1f166f82d Mon Sep 17 00:00:00 2001 From: Paul Faria Date: Sun, 17 May 2015 16:20:30 -0400 Subject: Initial work on #6061. --- components/script/dom/websocket.rs | 135 ++++++++++++++++++++++++++++++++++--- 1 file changed, 127 insertions(+), 8 deletions(-) (limited to 'components/script/dom/websocket.rs') diff --git a/components/script/dom/websocket.rs b/components/script/dom/websocket.rs index 75c868b50db..b4e00457714 100644 --- a/components/script/dom/websocket.rs +++ b/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::Syntax; +use dom::bindings::error::Error::{InvalidAccess, Syntax}; use dom::bindings::global::{GlobalField, GlobalRef}; use dom::bindings::js::{Temporary, JSRef, Rootable}; use dom::bindings::refcounted::Trusted; @@ -33,6 +32,7 @@ use websocket::stream::WebSocketStream; use websocket::client::request::Url; use websocket::Client; +use url::{SchemeData, SchemeType, UrlParser}; #[derive(PartialEq, Copy, Clone)] #[jstraceable] @@ -63,6 +63,112 @@ pub struct WebSocket { sendCloseFrame: Cell } +fn web_socket_scheme_types(scheme: &str) -> SchemeType { + match scheme { + "ws" => SchemeType::Relative(80), + "wss" => SchemeType::Relative(443), + _ => SchemeType::NonRelative, + } +} + +fn parse_web_socket_url(url_str: &str) -> Fallible<(Url, String, u16, String, bool)> { + // https://html.spec.whatwg.org/multipage/comms.html#parse-a-websocket-url's-components + // 1. No basepath specified, so it's absolute by default + // 2. UrlParser defaults to UTF-8 encoding + // 3. Specifying only ws and wss + let parsed_url = UrlParser::new() + .scheme_type_mapper(web_socket_scheme_types) + .parse(url_str); + + if parsed_url.is_err(){ + return Err(Error::Syntax); + } + + let parsed_url = parsed_url.unwrap(); + + // 3. Didn't match ws or wss + if let SchemeData::NonRelative(_) = parsed_url.scheme_data { + return Err(Error::Syntax); + } + + // 4. If the parsed url has a non-null fragment, fail + if parsed_url.fragment != None { + return Err(Error::Syntax); + } + + // 5. Set secure false if scheme is ws otherwise if scheme is wss set true + let secure = match parsed_url.scheme.as_ref() { + "ws" => false, + "wss" => true, + _ => unreachable!() + }; + + // 6. Set host to parsed url's host + let host = parsed_url.host().unwrap().serialize(); + + // 7. If the resulting parsed URL has a port component that is not the empty + // string, then let port be that component's value; otherwise, there is no + // explicit port. + let port = match parsed_url.port() { + Some(p) => p, + + // 8. If there is no explicit port, then: if secure is false, let port + // be 80, otherwise let port be 443. + None => if secure { + 443 + } else { + 80 + }, + }; + + // 9. Let resource name be the value of the resulting parsed URL's path + // component (which might be empty). + let base_resource = parsed_url.path().unwrap().connect("/"); + let mut resource = base_resource.as_ref(); + + // 10. If resource name is the empty string, set it to a single character + // U+002F SOLIDUS (/). + if resource == "" { + resource = "/"; + } + + let mut resource = resource.to_owned(); + + // 11. If the resulting parsed URL has a non-null query component, then + // append a single U+003F QUESTION MARK character (?) to resource name, + // followed by the value of the query component. + match parsed_url.query_pairs() { + Some(pairs) => { + let mut joined_pairs = pairs.iter() + .map(|pair| { + let mut keyValue = String::new(); + keyValue.push_str(pair.0.as_ref()); + keyValue.push('='); + keyValue.push_str(pair.1.as_ref()); + keyValue + }); + + let mut base_pair = String::new(); + base_pair.push_str(joined_pairs.next().unwrap().as_ref()); + + resource.push('?'); + + let query_string = joined_pairs.fold(base_pair, |mut current, next| { + current.push('&'); + current.push_str(next.as_ref()); + current + }); + + resource.push_str(query_string.as_ref()); + }, + None => (), + } + + // 12. Return host, port, resource name, and secure. + // FIXME remove parsed_url once it's no longer used in WebSocket::new + Ok((parsed_url, host, port, resource.to_string(), secure)) +} + impl WebSocket { pub fn new_inherited(global: GlobalRef, url: DOMString) -> WebSocket { WebSocket { @@ -83,11 +189,12 @@ impl WebSocket { } - pub fn new(global: GlobalRef, url: DOMString) -> Temporary { + pub fn new(global: GlobalRef, url: DOMString) -> Fallible> { /*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 */ @@ -95,7 +202,18 @@ impl WebSocket { global, WebSocketBinding::Wrap).root(); let ws_root = ws_root.r(); - let parsed_url = Url::parse(&ws_root.url).unwrap(); + + let parse_url_result = parse_web_socket_url(&ws_root.url); + if let Err(e) = parse_url_result { + return Err(e); + } + + // FIXME extract the right variables once Client::connect implementation is + // fixed to follow the RFC 6455 properly + let Ok((parsed_url, _, _, _, _)) = parse_url_result; + + // 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(); @@ -106,8 +224,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(); @@ -132,11 +251,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> { - Ok(WebSocket::new(global, url)) + WebSocket::new(global, url) } } -- cgit v1.2.3 From 0362d254b8ba111f1f09886b8ec6802ce4c50464 Mon Sep 17 00:00:00 2001 From: Paul Faria Date: Mon, 18 May 2015 17:28:16 -0400 Subject: Removing trailing whitespace --- components/script/dom/websocket.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'components/script/dom/websocket.rs') diff --git a/components/script/dom/websocket.rs b/components/script/dom/websocket.rs index b4e00457714..5c1ea116c26 100644 --- a/components/script/dom/websocket.rs +++ b/components/script/dom/websocket.rs @@ -133,7 +133,7 @@ fn parse_web_socket_url(url_str: &str) -> Fallible<(Url, String, u16, String, bo } let mut resource = resource.to_owned(); - + // 11. If the resulting parsed URL has a non-null query component, then // append a single U+003F QUESTION MARK character (?) to resource name, // followed by the value of the query component. @@ -150,7 +150,7 @@ fn parse_web_socket_url(url_str: &str) -> Fallible<(Url, String, u16, String, bo let mut base_pair = String::new(); base_pair.push_str(joined_pairs.next().unwrap().as_ref()); - + resource.push('?'); let query_string = joined_pairs.fold(base_pair, |mut current, next| { @@ -158,7 +158,7 @@ fn parse_web_socket_url(url_str: &str) -> Fallible<(Url, String, u16, String, bo current.push_str(next.as_ref()); current }); - + resource.push_str(query_string.as_ref()); }, None => (), @@ -226,7 +226,7 @@ impl WebSocket { //Do nothing else. Let the close finish. 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(); -- cgit v1.2.3 From 8e78564dc335f625b74eaf067889733ba292b3d7 Mon Sep 17 00:00:00 2001 From: Paul Faria Date: Mon, 18 May 2015 17:39:19 -0400 Subject: Fixing one missed trailing whitespace, and fixed url --- components/script/dom/websocket.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'components/script/dom/websocket.rs') diff --git a/components/script/dom/websocket.rs b/components/script/dom/websocket.rs index 5c1ea116c26..26f3066e511 100644 --- a/components/script/dom/websocket.rs +++ b/components/script/dom/websocket.rs @@ -72,14 +72,14 @@ fn web_socket_scheme_types(scheme: &str) -> SchemeType { } fn parse_web_socket_url(url_str: &str) -> Fallible<(Url, String, u16, String, bool)> { - // https://html.spec.whatwg.org/multipage/comms.html#parse-a-websocket-url's-components + // https://html.spec.whatwg.org/multipage/#parse-a-websocket-url's-components // 1. No basepath specified, so it's absolute by default // 2. UrlParser defaults to UTF-8 encoding // 3. Specifying only ws and wss let parsed_url = UrlParser::new() .scheme_type_mapper(web_socket_scheme_types) .parse(url_str); - + if parsed_url.is_err(){ return Err(Error::Syntax); } -- cgit v1.2.3 From fe0b77d6692e5f551cb7d2559487a5ee5a36ed0f Mon Sep 17 00:00:00 2001 From: Paul Faria Date: Tue, 19 May 2015 10:04:02 -0400 Subject: Responded to code review comments, general cleanup --- components/script/dom/websocket.rs | 80 ++++++++++++++------------------------ 1 file changed, 29 insertions(+), 51 deletions(-) (limited to 'components/script/dom/websocket.rs') diff --git a/components/script/dom/websocket.rs b/components/script/dom/websocket.rs index 26f3066e511..1f757f5859f 100644 --- a/components/script/dom/websocket.rs +++ b/components/script/dom/websocket.rs @@ -73,47 +73,41 @@ fn web_socket_scheme_types(scheme: &str) -> SchemeType { 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 - // 1. No basepath specified, so it's absolute by default - // 2. UrlParser defaults to UTF-8 encoding - // 3. Specifying only ws and wss + // Steps 1, 2, and 3 let parsed_url = UrlParser::new() .scheme_type_mapper(web_socket_scheme_types) .parse(url_str); - if parsed_url.is_err(){ - return Err(Error::Syntax); - } - - let parsed_url = parsed_url.unwrap(); + let parsed_url = match parsed_url { + Ok(parsed_url) => parsed_url, + Err(_) => return Err(Error::Syntax), + }; // 3. Didn't match ws or wss if let SchemeData::NonRelative(_) = parsed_url.scheme_data { return Err(Error::Syntax); } - // 4. If the parsed url has a non-null fragment, fail + // Step 4 if parsed_url.fragment != None { return Err(Error::Syntax); } - // 5. Set secure false if scheme is ws otherwise if scheme is wss set true + // Step 5 let secure = match parsed_url.scheme.as_ref() { "ws" => false, "wss" => true, _ => unreachable!() }; - // 6. Set host to parsed url's host + // Step 6 let host = parsed_url.host().unwrap().serialize(); - // 7. If the resulting parsed URL has a port component that is not the empty - // string, then let port be that component's value; otherwise, there is no - // explicit port. + // Step 7 let port = match parsed_url.port() { Some(p) => p, - // 8. If there is no explicit port, then: if secure is false, let port - // be 80, otherwise let port be 443. + // Step 8 None => if secure { 443 } else { @@ -121,52 +115,41 @@ fn parse_web_socket_url(url_str: &str) -> Fallible<(Url, String, u16, String, bo }, }; - // 9. Let resource name be the value of the resulting parsed URL's path - // component (which might be empty). - let base_resource = parsed_url.path().unwrap().connect("/"); - let mut resource = base_resource.as_ref(); + // Step 9 + let mut resource = parsed_url.path().unwrap().connect("/"); - // 10. If resource name is the empty string, set it to a single character - // U+002F SOLIDUS (/). + // Step 10 if resource == "" { - resource = "/"; + resource = "/".to_owned(); } - let mut resource = resource.to_owned(); - - // 11. If the resulting parsed URL has a non-null query component, then - // append a single U+003F QUESTION MARK character (?) to resource name, - // followed by the value of the query component. + // Step 11 match parsed_url.query_pairs() { Some(pairs) => { - let mut joined_pairs = pairs.iter() - .map(|pair| { - let mut keyValue = String::new(); - keyValue.push_str(pair.0.as_ref()); - keyValue.push('='); - keyValue.push_str(pair.1.as_ref()); - keyValue - }); - - let mut base_pair = String::new(); - base_pair.push_str(joined_pairs.next().unwrap().as_ref()); + fn append_query_components(s: &mut String, key: &str, value: &str) { + s.push_str(key); + s.push('='); + s.push_str(value); + } resource.push('?'); - let query_string = joined_pairs.fold(base_pair, |mut current, next| { + let mut iterator = pairs.iter(); + let first = iterator.next().unwrap(); + append_query_components(&mut resource, first.0.as_ref(), first.1.as_ref()); + + iterator.fold(&mut resource, |mut current, next| { current.push('&'); - current.push_str(next.as_ref()); + append_query_components(&mut current, next.0.as_ref(), next.1.as_ref()); current }); - - resource.push_str(query_string.as_ref()); }, None => (), } - // 12. Return host, port, resource name, and secure. + // Step 12 // FIXME remove parsed_url once it's no longer used in WebSocket::new - Ok((parsed_url, host, port, resource.to_string(), secure)) + Ok((parsed_url, host, port, resource, secure)) } impl WebSocket { @@ -203,14 +186,9 @@ impl WebSocket { WebSocketBinding::Wrap).root(); let ws_root = ws_root.r(); - let parse_url_result = parse_web_socket_url(&ws_root.url); - if let Err(e) = parse_url_result { - return Err(e); - } - // FIXME extract the right variables once Client::connect implementation is // fixed to follow the RFC 6455 properly - let Ok((parsed_url, _, _, _, _)) = parse_url_result; + 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 -- cgit v1.2.3 From 51ae7334f52b1133dbecf455ba71e89fa0152ef1 Mon Sep 17 00:00:00 2001 From: Paul Faria Date: Tue, 19 May 2015 17:54:14 -0400 Subject: Responded to more code review comments. Simplified code a lot. --- components/script/dom/websocket.rs | 76 ++++++-------------------------------- 1 file changed, 12 insertions(+), 64 deletions(-) (limited to 'components/script/dom/websocket.rs') diff --git a/components/script/dom/websocket.rs b/components/script/dom/websocket.rs index 1f757f5859f..10146c5062c 100644 --- a/components/script/dom/websocket.rs +++ b/components/script/dom/websocket.rs @@ -32,8 +32,6 @@ use websocket::stream::WebSocketStream; use websocket::client::request::Url; use websocket::Client; -use url::{SchemeData, SchemeType, UrlParser}; - #[derive(PartialEq, Copy, Clone)] #[jstraceable] enum WebSocketRequestState { @@ -63,88 +61,38 @@ pub struct WebSocket { sendCloseFrame: Cell } -fn web_socket_scheme_types(scheme: &str) -> SchemeType { - match scheme { - "ws" => SchemeType::Relative(80), - "wss" => SchemeType::Relative(443), - _ => SchemeType::NonRelative, - } -} - 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, 2, and 3 - let parsed_url = UrlParser::new() - .scheme_type_mapper(web_socket_scheme_types) - .parse(url_str); - + // 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), }; - // 3. Didn't match ws or wss - if let SchemeData::NonRelative(_) = parsed_url.scheme_data { - return Err(Error::Syntax); - } - // Step 4 if parsed_url.fragment != None { return Err(Error::Syntax); } - // Step 5 + // Steps 3 and 5 let secure = match parsed_url.scheme.as_ref() { "ws" => false, "wss" => true, - _ => unreachable!() + _ => return Err(Error::Syntax), // step 3 }; - // Step 6 - let host = parsed_url.host().unwrap().serialize(); - - // Step 7 - let port = match parsed_url.port() { - Some(p) => p, - - // Step 8 - None => if secure { - 443 - } else { - 80 - }, - }; - - // Step 9 - let mut resource = parsed_url.path().unwrap().connect("/"); - - // Step 10 - if resource == "" { - resource = "/".to_owned(); + 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 - match parsed_url.query_pairs() { - Some(pairs) => { - fn append_query_components(s: &mut String, key: &str, value: &str) { - s.push_str(key); - s.push('='); - s.push_str(value); - } - - resource.push('?'); - - let mut iterator = pairs.iter(); - let first = iterator.next().unwrap(); - append_query_components(&mut resource, first.0.as_ref(), first.1.as_ref()); - - iterator.fold(&mut resource, |mut current, next| { - current.push('&'); - append_query_components(&mut current, next.0.as_ref(), next.1.as_ref()); - current - }); - }, - None => (), + if let Some(ref query) = parsed_url.query { + resource.push('?'); + resource.push_str(query); } // Step 12 -- cgit v1.2.3