Responded to code review comments, general cleanup

This commit is contained in:
Paul Faria 2015-05-19 10:04:02 -04:00
parent 8e78564dc3
commit fe0b77d669

View file

@ -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)> { 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 // https://html.spec.whatwg.org/multipage/#parse-a-websocket-url's-components
// 1. No basepath specified, so it's absolute by default // Steps 1, 2, and 3
// 2. UrlParser defaults to UTF-8 encoding
// 3. Specifying only ws and wss
let parsed_url = UrlParser::new() let parsed_url = UrlParser::new()
.scheme_type_mapper(web_socket_scheme_types) .scheme_type_mapper(web_socket_scheme_types)
.parse(url_str); .parse(url_str);
if parsed_url.is_err(){ let parsed_url = match parsed_url {
return Err(Error::Syntax); Ok(parsed_url) => parsed_url,
} Err(_) => return Err(Error::Syntax),
};
let parsed_url = parsed_url.unwrap();
// 3. Didn't match ws or wss // 3. Didn't match ws or wss
if let SchemeData::NonRelative(_) = parsed_url.scheme_data { if let SchemeData::NonRelative(_) = parsed_url.scheme_data {
return Err(Error::Syntax); return Err(Error::Syntax);
} }
// 4. If the parsed url has a non-null fragment, fail // Step 4
if parsed_url.fragment != None { if parsed_url.fragment != None {
return Err(Error::Syntax); 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() { let secure = match parsed_url.scheme.as_ref() {
"ws" => false, "ws" => false,
"wss" => true, "wss" => true,
_ => unreachable!() _ => unreachable!()
}; };
// 6. Set host to parsed url's host // Step 6
let host = parsed_url.host().unwrap().serialize(); let host = parsed_url.host().unwrap().serialize();
// 7. If the resulting parsed URL has a port component that is not the empty // Step 7
// string, then let port be that component's value; otherwise, there is no
// explicit port.
let port = match parsed_url.port() { let port = match parsed_url.port() {
Some(p) => p, Some(p) => p,
// 8. If there is no explicit port, then: if secure is false, let port // Step 8
// be 80, otherwise let port be 443.
None => if secure { None => if secure {
443 443
} else { } 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 // Step 9
// component (which might be empty). let mut resource = parsed_url.path().unwrap().connect("/");
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 // Step 10
// U+002F SOLIDUS (/).
if resource == "" { if resource == "" {
resource = "/"; resource = "/".to_owned();
} }
let mut resource = resource.to_owned(); // Step 11
// 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() { match parsed_url.query_pairs() {
Some(pairs) => { Some(pairs) => {
let mut joined_pairs = pairs.iter() fn append_query_components(s: &mut String, key: &str, value: &str) {
.map(|pair| { s.push_str(key);
let mut keyValue = String::new(); s.push('=');
keyValue.push_str(pair.0.as_ref()); s.push_str(value);
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('?'); 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('&');
current.push_str(next.as_ref()); append_query_components(&mut current, next.0.as_ref(), next.1.as_ref());
current current
}); });
resource.push_str(query_string.as_ref());
}, },
None => (), None => (),
} }
// 12. Return host, port, resource name, and secure. // Step 12
// FIXME remove parsed_url once it's no longer used in WebSocket::new // 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 { impl WebSocket {
@ -203,14 +186,9 @@ impl WebSocket {
WebSocketBinding::Wrap).root(); WebSocketBinding::Wrap).root();
let ws_root = ws_root.r(); 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 // FIXME extract the right variables once Client::connect implementation is
// fixed to follow the RFC 6455 properly // 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 // TODO Client::connect does not conform to RFC 6455
// see https://github.com/cyderize/rust-websocket/issues/38 // see https://github.com/cyderize/rust-websocket/issues/38