From 4f5327afd4a065585df8436875903138707a970d Mon Sep 17 00:00:00 2001 From: M3rs Date: Fri, 20 Jan 2017 23:41:09 -0600 Subject: [PATCH 1/5] refactor(uri): Remove Url parse Remove usage of Url parse in Uri in order to improve performance. https://github.com/hyperium/hyper/issues/1022 --- src/uri.rs | 75 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 32 deletions(-) diff --git a/src/uri.rs b/src/uri.rs index 8f15662a..d409416f 100644 --- a/src/uri.rs +++ b/src/uri.rs @@ -1,7 +1,7 @@ use std::borrow::Cow; use std::fmt::{Display, self}; use std::str::FromStr; -use url::{self, Url}; +use url::Url; use url::ParseError as UrlError; use Error; @@ -54,43 +54,21 @@ impl Uri { } else if bytes == b"/" { Ok(Uri::default()) } else if bytes.starts_with(b"/") { - let mut temp = "http://example.com".to_owned(); - temp.push_str(s); - let url = try!(Url::parse(&temp)); - let query_len = url.query().unwrap_or("").len(); - let fragment_len = url.fragment().unwrap_or("").len(); Ok(Uri { source: s.to_owned().into(), scheme_end: None, authority_end: None, - query: if query_len > 0 { Some(query_len) } else { None }, - fragment: if fragment_len > 0 { Some(fragment_len) } else { None }, + query: parse_query(s), + fragment: parse_fragment(s), }) } else if s.contains("://") { - let url = try!(Url::parse(s)); - let query_len = url.query().unwrap_or("").len(); - let new_s = url.to_string(); - let authority_end = { - let v: Vec<&str> = new_s.split("://").collect(); - v.last().unwrap() - .split(url.path()) - .next() - .unwrap_or(&new_s) - .len() + if v.len() == 2 { v[0].len() + 3 } else { 0 } - }; - let fragment_len = url.fragment().unwrap_or("").len(); - match url.origin() { - url::Origin::Opaque(_) => Err(Error::Method), - url::Origin::Tuple(scheme, _, _) => { - Ok(Uri { - source: new_s.into(), - scheme_end: Some(scheme.len()), - authority_end: if authority_end > 0 { Some(authority_end) } else { None }, - query: if query_len > 0 { Some(query_len) } else { None }, - fragment: if fragment_len > 0 { Some(fragment_len) } else { None }, - }) - } - } + Ok(Uri { + source: s.to_owned().into(), + scheme_end: parse_scheme(s), + authority_end: parse_authority(s), + query: parse_query(s), + fragment: parse_fragment(s), + }) } else { Ok(Uri { source: s.to_owned().into(), @@ -179,6 +157,39 @@ impl Uri { } } +fn parse_scheme(s: &str) -> Option { + s.find(':') +} + +fn parse_authority(s: &str) -> Option { + let v: Vec<&str> = s.split("://").collect(); + let auth = v.last().unwrap() + .split("/") + .next() + .unwrap_or(s) + .len() + if v.len() == 2 { v[0].len() + 3 } else { 0 }; + + return Some(auth); +} + +fn parse_query(s: &str) -> Option { + match s.find('?') { + Some(i) => { + let frag_pos = s.find('#').unwrap_or(s.len()); + + return Some(frag_pos - i - 1); + }, + None => None, + } +} + +fn parse_fragment(s: &str) -> Option { + match s.find('#') { + Some(i) => Some(s.len() - i - 1), + None => None, + } +} + impl FromStr for Uri { type Err = Error; From 55d13a9afde63c6d5fa2ac3c3d1543c4c0641b75 Mon Sep 17 00:00:00 2001 From: M3rs Date: Sat, 21 Jan 2017 21:20:24 -0600 Subject: [PATCH 2/5] refactor(uri): Remove extra authority logic Remove extra logic in authority getter method which handles default ports. --- src/uri.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/uri.rs b/src/uri.rs index d409416f..a303f573 100644 --- a/src/uri.rs +++ b/src/uri.rs @@ -107,6 +107,7 @@ impl Uri { pub fn authority(&self) -> Option<&str> { if let Some(end) = self.authority_end { let index = self.scheme_end.map(|i| i + 3).unwrap_or(0); + Some(&self.source[index..end]) } else { None From 37b26e21e8ef323eea02f6a715a2030114c764ed Mon Sep 17 00:00:00 2001 From: M3rs Date: Sun, 22 Jan 2017 10:51:06 -0600 Subject: [PATCH 3/5] refactor(uri): Add default path for absolute-form Add default path ("/") for absolute-form, even if not included. This change assumes self.scheme().is_some() indicates that the Uri is in absolute-form. Issue: https://github.com/hyperium/hyper/issues/1022 --- src/uri.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/uri.rs b/src/uri.rs index a303f573..22c767f7 100644 --- a/src/uri.rs +++ b/src/uri.rs @@ -88,6 +88,9 @@ impl Uri { let end = self.source.len() - if query_len > 0 { query_len + 1 } else { 0 } - if fragment_len > 0 { fragment_len + 1 } else { 0 }; if index >= end { + if self.scheme().is_some() { + return "/" // absolute-form MUST have path + } "" } else { &self.source[index..end] @@ -317,7 +320,7 @@ test_parse! { "http://127.0.0.1:80", scheme = Some("http"), - authority = Some("127.0.0.1"), + authority = Some("127.0.0.1:80"), path = "/", query = None, fragment = None, @@ -328,7 +331,7 @@ test_parse! { "https://127.0.0.1:443", scheme = Some("https"), - authority = Some("127.0.0.1"), + authority = Some("127.0.0.1:443"), path = "/", query = None, fragment = None, From 8faf5b8bb1bea34217480039109fb68d2172fa9d Mon Sep 17 00:00:00 2001 From: M3rs Date: Sun, 22 Jan 2017 11:38:45 -0600 Subject: [PATCH 4/5] refactor(uri): Add errors to scheme uri Add errors to scheme uri (contains "://"). Check for: - Valid schemes (ftp, gopher, http, https, ws, wss) - Invalid schemes (blob, file), or anything else -> Err(Error::Method) - Authority is not empty (i.e. "http://") https://github.com/hyperium/hyper/issues/1022 --- src/uri.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/uri.rs b/src/uri.rs index 22c767f7..8d017aac 100644 --- a/src/uri.rs +++ b/src/uri.rs @@ -62,10 +62,24 @@ impl Uri { fragment: parse_fragment(s), }) } else if s.contains("://") { + let scheme = parse_scheme(s); + let auth = parse_authority(s); + if let Some(end) = scheme { + match &s[..end] { + "ftp" | "gopher" | "http" | "https" | "ws" | "wss" => {}, + "blob" | "file" => return Err(Error::Method), + _ => return Err(Error::Method), + } + if let Some(a) = auth { + if (end + 3) == a { + return Err(Error::Method); + } + } + } Ok(Uri { source: s.to_owned().into(), - scheme_end: parse_scheme(s), - authority_end: parse_authority(s), + scheme_end: scheme, + authority_end: auth, query: parse_query(s), fragment: parse_fragment(s), }) From 04560dfe24000ed593e4c14bb7326026a8ef7130 Mon Sep 17 00:00:00 2001 From: M3rs Date: Mon, 23 Jan 2017 12:26:08 -0600 Subject: [PATCH 5/5] refactor(uri): Improve parse_authority safety Improve parse_authority safety with match, replace unwrap. Also, refactor code in contains("://") block using result from the parse_authority to also use match. https://github.com/hyperium/hyper/issues/1022 --- src/uri.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/uri.rs b/src/uri.rs index 8d017aac..77622263 100644 --- a/src/uri.rs +++ b/src/uri.rs @@ -70,10 +70,13 @@ impl Uri { "blob" | "file" => return Err(Error::Method), _ => return Err(Error::Method), } - if let Some(a) = auth { - if (end + 3) == a { - return Err(Error::Method); - } + match auth { + Some(a) => { + if (end + 3) == a { + return Err(Error::Method); + } + }, + None => return Err(Error::Method), } } Ok(Uri { @@ -181,13 +184,13 @@ fn parse_scheme(s: &str) -> Option { fn parse_authority(s: &str) -> Option { let v: Vec<&str> = s.split("://").collect(); - let auth = v.last().unwrap() - .split("/") - .next() - .unwrap_or(s) - .len() + if v.len() == 2 { v[0].len() + 3 } else { 0 }; - - return Some(auth); + match v.last() { + Some(auth) => Some(auth.split("/") + .next() + .unwrap_or(s) + .len() + if v.len() == 2 { v[0].len() + 3 } else { 0 }), + None => None, + } } fn parse_query(s: &str) -> Option {