From 27b6af2800ca368cda314a94ff1936d5142bc782 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 19 Jun 2018 16:45:26 +0600 Subject: [PATCH] refactor route matching --- src/application.rs | 37 ++++---- src/de.rs | 17 ++-- src/fs.rs | 8 +- src/httprequest.rs | 7 +- src/param.rs | 129 ++++++++++++++++++---------- src/router.rs | 204 +++++++++++++++++++++++++++++---------------- src/scope.rs | 22 ++--- src/test.rs | 4 +- src/uri.rs | 9 +- 9 files changed, 262 insertions(+), 175 deletions(-) diff --git a/src/application.rs b/src/application.rs index e53382de4..ca5e8786c 100644 --- a/src/application.rs +++ b/src/application.rs @@ -73,35 +73,32 @@ impl HttpApplication { HandlerType::Normal(idx) } else { let inner = self.as_ref(); - let path: &'static str = - unsafe { &*(&req.path()[inner.prefix..] as *const _) }; - let path_len = path.len(); + req.match_info_mut().set_tail(0); + 'outer: for idx in 0..inner.handlers.len() { match inner.handlers[idx] { PrefixHandlerType::Handler(ref prefix, _) => { let m = { + let path = &req.path()[inner.prefix..]; + let path_len = path.len(); + path.starts_with(prefix) && (path_len == prefix.len() || path.split_at(prefix.len()).1.starts_with('/')) }; if m { - let prefix_len = inner.prefix + prefix.len(); - let path: &'static str = - unsafe { &*(&req.path()[prefix_len..] as *const _) }; - - req.set_prefix_len(prefix_len as u16); - if path.is_empty() { - req.match_info_mut().add("tail", "/"); - } else { - req.match_info_mut().add("tail", path); - } + let prefix_len = (inner.prefix + prefix.len()) as u16; + let url = req.url().clone(); + req.set_prefix_len(prefix_len); + req.match_info_mut().set_url(url); + req.match_info_mut().set_tail(prefix_len); return HandlerType::Handler(idx); } } PrefixHandlerType::Scope(ref pattern, _, ref filters) => { if let Some(prefix_len) = - pattern.match_prefix_with_params(path, req.match_info_mut()) + pattern.match_prefix_with_params(req, inner.prefix) { for filter in filters { if !filter.check(req) { @@ -109,12 +106,12 @@ impl HttpApplication { } } - let prefix_len = inner.prefix + prefix_len; - let path: &'static str = - unsafe { &*(&req.path()[prefix_len..] as *const _) }; - - req.set_prefix_len(prefix_len as u16); - req.match_info_mut().set("tail", path); + let prefix_len = (inner.prefix + prefix_len) as u16; + let url = req.url().clone(); + req.set_prefix_len(prefix_len); + let params = req.match_info_mut(); + params.set_tail(prefix_len); + params.set_url(url); return HandlerType::Handler(idx); } } diff --git a/src/de.rs b/src/de.rs index ad3327870..ecb2fa9ae 100644 --- a/src/de.rs +++ b/src/de.rs @@ -1,9 +1,7 @@ use serde::de::{self, Deserializer, Error as DeError, Visitor}; -use std::borrow::Cow; -use std::convert::AsRef; -use std::slice::Iter; use httprequest::HttpRequest; +use param::ParamsIter; macro_rules! unsupported_type { ($trait_fn:ident, $name:expr) => { @@ -191,7 +189,7 @@ impl<'de, S: 'de> Deserializer<'de> for PathDeserializer<'de, S> { } struct ParamsDeserializer<'de> { - params: Iter<'de, (Cow<'de, str>, Cow<'de, str>)>, + params: ParamsIter<'de>, current: Option<(&'de str, &'de str)>, } @@ -202,10 +200,7 @@ impl<'de> de::MapAccess<'de> for ParamsDeserializer<'de> { where K: de::DeserializeSeed<'de>, { - self.current = self - .params - .next() - .map(|&(ref k, ref v)| (k.as_ref(), v.as_ref())); + self.current = self.params.next().map(|ref item| (item.0, item.1)); match self.current { Some((key, _)) => Ok(Some(seed.deserialize(Key { key })?)), None => Ok(None), @@ -381,7 +376,7 @@ impl<'de> Deserializer<'de> for Value<'de> { } struct ParamsSeq<'de> { - params: Iter<'de, (Cow<'de, str>, Cow<'de, str>)>, + params: ParamsIter<'de>, } impl<'de> de::SeqAccess<'de> for ParamsSeq<'de> { @@ -392,9 +387,7 @@ impl<'de> de::SeqAccess<'de> for ParamsSeq<'de> { T: de::DeserializeSeed<'de>, { match self.params.next() { - Some(item) => Ok(Some(seed.deserialize(Value { - value: item.1.as_ref(), - })?)), + Some(item) => Ok(Some(seed.deserialize(Value { value: item.1 })?)), None => Ok(None), } } diff --git a/src/fs.rs b/src/fs.rs index 9d5659f72..e1cc58a5e 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -1190,7 +1190,7 @@ mod tests { assert_eq!(resp.status(), StatusCode::NOT_FOUND); let mut req = HttpRequest::default(); - req.match_info_mut().add("tail", ""); + req.match_info_mut().add_static("tail", ""); st.show_index = true; let resp = st.handle(req).respond_to(&HttpRequest::default()).unwrap(); @@ -1207,7 +1207,7 @@ mod tests { fn test_redirect_to_index() { let mut st = StaticFiles::new(".").index_file("index.html"); let mut req = HttpRequest::default(); - req.match_info_mut().add("tail", "tests"); + req.match_info_mut().add_static("tail", "tests"); let resp = st.handle(req).respond_to(&HttpRequest::default()).unwrap(); let resp = resp.as_msg(); @@ -1218,7 +1218,7 @@ mod tests { ); let mut req = HttpRequest::default(); - req.match_info_mut().add("tail", "tests/"); + req.match_info_mut().add_static("tail", "tests/"); let resp = st.handle(req).respond_to(&HttpRequest::default()).unwrap(); let resp = resp.as_msg(); @@ -1233,7 +1233,7 @@ mod tests { fn test_redirect_to_index_nested() { let mut st = StaticFiles::new(".").index_file("mod.rs"); let mut req = HttpRequest::default(); - req.match_info_mut().add("tail", "src/client"); + req.match_info_mut().add_static("tail", "src/client"); let resp = st.handle(req).respond_to(&HttpRequest::default()).unwrap(); let resp = resp.as_msg(); diff --git a/src/httprequest.rs b/src/httprequest.rs index cf6869f2e..9f051aa86 100644 --- a/src/httprequest.rs +++ b/src/httprequest.rs @@ -40,7 +40,7 @@ pub struct HttpInnerMessage { pub(crate) flags: MessageFlags, pub headers: HeaderMap, pub extensions: Extensions, - pub params: Params<'static>, + pub params: Params, pub addr: Option, pub payload: Option, pub prefix: u16, @@ -268,6 +268,11 @@ impl HttpRequest { self.as_ref().url.path() } + #[inline] + pub(crate) fn url(&self) -> &InnerUrl { + &self.as_ref().url + } + /// Get *ConnectionInfo* for correct request. pub fn connection_info(&self) -> &ConnectionInfo { if self.extensions().get::().is_none() { diff --git a/src/param.rs b/src/param.rs index 48d1f3a3f..02c1aea8a 100644 --- a/src/param.rs +++ b/src/param.rs @@ -1,13 +1,12 @@ use http::StatusCode; use smallvec::SmallVec; use std; -use std::borrow::Cow; use std::ops::Index; use std::path::PathBuf; -use std::slice::Iter; use std::str::FromStr; use error::{InternalError, ResponseError, UriSegmentError}; +use uri::Url; /// A trait to abstract the idea of creating a new instance of a type from a /// path parameter. @@ -19,72 +18,78 @@ pub trait FromParam: Sized { fn from_param(s: &str) -> Result; } +#[derive(Debug, Clone, Copy)] +pub(crate) enum ParamItem { + Static(&'static str), + UrlSegment(u16, u16), +} + /// Route match information /// /// If resource path contains variable patterns, `Params` stores this variables. #[derive(Debug)] -pub struct Params<'a>(SmallVec<[(Cow<'a, str>, Cow<'a, str>); 3]>); +pub struct Params { + url: Url, + pub(crate) tail: u16, + segments: SmallVec<[(&'static str, ParamItem); 3]>, +} -impl<'a> Params<'a> { - pub(crate) fn new() -> Params<'a> { - Params(SmallVec::new()) +impl Params { + pub(crate) fn new() -> Params { + Params { + url: Url::default(), + tail: 0, + segments: SmallVec::new(), + } } pub(crate) fn clear(&mut self) { - self.0.clear(); + self.segments.clear(); } - pub(crate) fn add(&mut self, name: N, value: V) - where - N: Into>, - V: Into>, - { - self.0.push((name.into(), value.into())); + pub(crate) fn set_url(&mut self, url: Url) { + self.url = url; } - pub(crate) fn set(&mut self, name: N, value: V) - where - N: Into>, - V: Into>, - { - let name = name.into(); - let value = value.into(); - for item in &mut self.0 { - if item.0 == name { - item.1 = value; - return; - } - } - self.0.push((name, value)); + pub(crate) fn set_tail(&mut self, tail: u16) { + self.tail = tail; } - pub(crate) fn remove(&mut self, name: &str) { - for idx in (0..self.0.len()).rev() { - if self.0[idx].0 == name { - self.0.remove(idx); - return; - } - } + pub(crate) fn add(&mut self, name: &'static str, value: ParamItem) { + self.segments.push((name, value)); + } + + pub(crate) fn add_static(&mut self, name: &'static str, value: &'static str) { + self.segments.push((name, ParamItem::Static(value))); } /// Check if there are any matched patterns pub fn is_empty(&self) -> bool { - self.0.is_empty() + self.segments.is_empty() } /// Check number of extracted parameters pub fn len(&self) -> usize { - self.0.len() + self.segments.len() } /// Get matched parameter by name without type conversion - pub fn get(&'a self, key: &str) -> Option<&'a str> { - for item in self.0.iter() { + pub fn get(&self, key: &str) -> Option<&str> { + for item in self.segments.iter() { if key == item.0 { - return Some(item.1.as_ref()); + return match item.1 { + ParamItem::Static(s) => Some(s), + ParamItem::UrlSegment(s, e) => { + Some(&self.url.path()[(s as usize)..(e as usize)]) + } + }; } } - None + if key == "tail" { + Some(&self.url.path()[(self.tail as usize)..]) + } else { + None + } } /// Get matched `FromParam` compatible parameter by name. @@ -101,7 +106,7 @@ impl<'a> Params<'a> { /// } /// # fn main() {} /// ``` - pub fn query(&'a self, key: &str) -> Result::Err> { + pub fn query(&self, key: &str) -> Result::Err> { if let Some(s) = self.get(key) { T::from_param(s) } else { @@ -110,12 +115,41 @@ impl<'a> Params<'a> { } /// Return iterator to items in parameter container - pub fn iter(&self) -> Iter<(Cow<'a, str>, Cow<'a, str>)> { - self.0.iter() + pub fn iter(&self) -> ParamsIter { + ParamsIter { + idx: 0, + params: self, + } } } -impl<'a, 'b, 'c: 'a> Index<&'b str> for &'c Params<'a> { +#[derive(Debug)] +pub struct ParamsIter<'a> { + idx: usize, + params: &'a Params, +} + +impl<'a> Iterator for ParamsIter<'a> { + type Item = (&'a str, &'a str); + + #[inline] + fn next(&mut self) -> Option<(&'a str, &'a str)> { + if self.idx < self.params.len() { + let idx = self.idx; + let res = match self.params.segments[idx].1 { + ParamItem::Static(s) => s, + ParamItem::UrlSegment(s, e) => { + &self.params.url.path()[(s as usize)..(e as usize)] + } + }; + self.idx += 1; + return Some((self.params.segments[idx].0, res)); + } + None + } +} + +impl<'a, 'b> Index<&'b str> for &'a Params { type Output = str; fn index(&self, name: &'b str) -> &str { @@ -124,11 +158,14 @@ impl<'a, 'b, 'c: 'a> Index<&'b str> for &'c Params<'a> { } } -impl<'a, 'c: 'a> Index for &'c Params<'a> { +impl<'a> Index for &'a Params { type Output = str; fn index(&self, idx: usize) -> &str { - self.0[idx].1.as_ref() + match self.segments[idx].1 { + ParamItem::Static(s) => s, + ParamItem::UrlSegment(s, e) => &self.url.path()[(s as usize)..(e as usize)], + } } } diff --git a/src/router.rs b/src/router.rs index c9ff9d705..6592f64e7 100644 --- a/src/router.rs +++ b/src/router.rs @@ -1,12 +1,13 @@ use std::collections::HashMap; use std::hash::{Hash, Hasher}; +use std::mem; use std::rc::Rc; use regex::{escape, Regex}; use error::UrlGenerationError; use httprequest::HttpRequest; -use param::Params; +use param::ParamItem; use resource::ResourceHandler; use server::ServerSettings; @@ -78,11 +79,10 @@ impl Router { if self.0.prefix_len > req.path().len() { return None; } - let path = unsafe { &*(&req.path()[self.0.prefix_len..] as *const str) }; - let route_path = if path.is_empty() { "/" } else { path }; - for (idx, pattern) in self.0.patterns.iter().enumerate() { - if pattern.match_with_params(route_path, req.match_info_mut()) { + if pattern.match_with_params(req, self.0.prefix_len, true) { + let url = req.url().clone(); + req.match_info_mut().set_url(url); req.set_resource(idx); req.set_prefix_len(self.0.prefix_len as u16); return Some(idx); @@ -261,73 +261,128 @@ impl Resource { } /// Are the given path and parameters a match against this resource? - pub fn match_with_params<'a>( - &'a self, path: &'a str, params: &'a mut Params<'a>, + pub fn match_with_params( + &self, req: &mut HttpRequest, plen: usize, insert: bool, ) -> bool { - match self.tp { - PatternType::Static(ref s) => s == path, - PatternType::Dynamic(ref re, ref names, _) => { - if let Some(captures) = re.captures(path) { - let mut idx = 0; - for capture in captures.iter() { - if let Some(ref m) = capture { - if idx != 0 { - params.add(names[idx - 1].as_str(), m.as_str()); - } - idx += 1; - } - } - true + let mut segments: [ParamItem; 24] = unsafe { mem::uninitialized() }; + + let (names, segments_len) = { + let path = &req.path()[plen..]; + if insert { + if path.is_empty() { + "/" } else { - false + path } + } else { + path + }; + + match self.tp { + PatternType::Static(ref s) => return s == path, + PatternType::Dynamic(ref re, ref names, _) => { + if let Some(captures) = re.captures(path) { + let mut idx = 0; + let mut passed = false; + for capture in captures.iter() { + if let Some(ref m) = capture { + if !passed { + passed = true; + continue; + } + segments[idx] = ParamItem::UrlSegment( + (plen + m.start()) as u16, + (plen + m.end()) as u16, + ); + idx += 1; + } + } + (names, idx) + } else { + return false; + } + } + PatternType::Prefix(ref s) => return path.starts_with(s), } - PatternType::Prefix(ref s) => path.starts_with(s), + }; + + let len = req.path().len(); + let params = req.match_info_mut(); + params.set_tail(len as u16); + for idx in 0..segments_len { + let name = unsafe { &*(names[idx].as_str() as *const _) }; + params.add(name, segments[idx]); } + true } /// Is the given path a prefix match and do the parameters match against this resource? - pub fn match_prefix_with_params<'a>( - &'a self, path: &'a str, params: &'a mut Params<'a>, + pub fn match_prefix_with_params( + &self, req: &mut HttpRequest, plen: usize, ) -> Option { - match self.tp { - PatternType::Static(ref s) => if s == path { - Some(s.len()) - } else { - None - }, - PatternType::Dynamic(ref re, ref names, len) => { - if let Some(captures) = re.captures(path) { - let mut idx = 0; - let mut pos = 0; - for capture in captures.iter() { - if let Some(ref m) = capture { - if idx != 0 { - params.add(names[idx - 1].as_str(), m.as_str()); - } - idx += 1; - pos = m.end(); - } - } - Some(pos + len) + let mut segments: [ParamItem; 24] = unsafe { mem::uninitialized() }; + + let (names, segments_len, tail_len) = { + let path = &req.path()[plen..]; + let path = if path.is_empty() { "/" } else { path }; + + match self.tp { + PatternType::Static(ref s) => if s == path { + return Some(s.len()); } else { - None + return None; + }, + PatternType::Dynamic(ref re, ref names, len) => { + if let Some(captures) = re.captures(path) { + let mut idx = 0; + let mut pos = 0; + let mut passed = false; + for capture in captures.iter() { + if let Some(ref m) = capture { + if !passed { + passed = true; + continue; + } + + segments[idx] = ParamItem::UrlSegment( + (plen + m.start()) as u16, + (plen + m.end()) as u16, + ); + idx += 1; + pos = m.end(); + } + } + (names, idx, pos + len) + } else { + return None; + } + } + PatternType::Prefix(ref s) => { + return if path == s { + Some(s.len()) + } else if path.starts_with(s) + && (s.ends_with('/') + || path.split_at(s.len()).1.starts_with('/')) + { + if s.ends_with('/') { + Some(s.len() - 1) + } else { + Some(s.len()) + } + } else { + None + } } } - PatternType::Prefix(ref s) => if path == s { - Some(s.len()) - } else if path.starts_with(s) - && (s.ends_with('/') || path.split_at(s.len()).1.starts_with('/')) - { - if s.ends_with('/') { - Some(s.len() - 1) - } else { - Some(s.len()) - } - } else { - None - }, + }; + + let params = req.match_info_mut(); + params.set_tail(tail_len as u16); + for idx in 0..segments_len { + let name = unsafe { &*(names[idx].as_str() as *const _) }; + params.add(name, segments[idx]); } + Some(tail_len) } /// Build resource path. @@ -634,20 +689,22 @@ mod tests { #[test] fn test_parse_param() { - let mut req = HttpRequest::default(); - let re = Resource::new("test", "/user/{id}"); assert!(re.is_match("/user/profile")); assert!(re.is_match("/user/2345")); assert!(!re.is_match("/user/2345/")); assert!(!re.is_match("/user/2345/sdg")); - req.match_info_mut().clear(); - assert!(re.match_with_params("/user/profile", req.match_info_mut())); + let mut req = TestRequest::with_uri("/user/profile").finish(); + let url = req.url().clone(); + req.match_info_mut().set_url(url); + assert!(re.match_with_params(&mut req, 0, true)); assert_eq!(req.match_info().get("id").unwrap(), "profile"); - req.match_info_mut().clear(); - assert!(re.match_with_params("/user/1245125", req.match_info_mut())); + let mut req = TestRequest::with_uri("/user/1245125").finish(); + let url = req.url().clone(); + req.match_info_mut().set_url(url); + assert!(re.match_with_params(&mut req, 0, true)); assert_eq!(req.match_info().get("id").unwrap(), "1245125"); let re = Resource::new("test", "/v{version}/resource/{id}"); @@ -655,8 +712,10 @@ mod tests { assert!(!re.is_match("/v/resource/1")); assert!(!re.is_match("/resource")); - req.match_info_mut().clear(); - assert!(re.match_with_params("/v151/resource/adahg32", req.match_info_mut())); + let mut req = TestRequest::with_uri("/v151/resource/adahg32").finish(); + let url = req.url().clone(); + req.match_info_mut().set_url(url); + assert!(re.match_with_params(&mut req, 0, true)); assert_eq!(req.match_info().get("version").unwrap(), "151"); assert_eq!(req.match_info().get("id").unwrap(), "adahg32"); } @@ -684,15 +743,16 @@ mod tests { assert!(!re.is_match("/name")); let mut req = TestRequest::with_uri("/test2/").finish(); - assert!(re.match_with_params("/test2/", req.match_info_mut())); + let url = req.url().clone(); + req.match_info_mut().set_url(url); + assert!(re.match_with_params(&mut req, 0, true)); assert_eq!(&req.match_info()["name"], "test2"); let mut req = TestRequest::with_uri("/test2/subpath1/subpath2/index.html").finish(); - assert!(re.match_with_params( - "/test2/subpath1/subpath2/index.html", - req.match_info_mut() - )); + let url = req.url().clone(); + req.match_info_mut().set_url(url); + assert!(re.match_with_params(&mut req, 0, true)); assert_eq!(&req.match_info()["name"], "test2"); } diff --git a/src/scope.rs b/src/scope.rs index a40113023..37fb78908 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -322,14 +322,13 @@ impl Scope { impl RouteHandler for Scope { fn handle(&mut self, mut req: HttpRequest) -> AsyncResult { - let path = unsafe { &*(&req.match_info()["tail"] as *const _) }; + let tail = req.match_info().tail as usize; // recognize resources for &(ref pattern, ref resource) in self.resources.iter() { - if pattern.match_with_params(path, req.match_info_mut()) { + if pattern.match_with_params(&mut req, tail, false) { let default = unsafe { &mut *self.default.as_ref().get() }; - req.match_info_mut().remove("tail"); if self.middlewares.borrow().is_empty() { let resource = unsafe { &mut *resource.get() }; return resource.handle(req, Some(default)); @@ -346,23 +345,18 @@ impl RouteHandler for Scope { // nested scopes let len = req.prefix_len() as usize; - let path: &'static str = unsafe { &*(&req.path()[len..] as *const _) }; - 'outer: for &(ref prefix, ref handler, ref filters) in &self.nested { - if let Some(prefix_len) = - prefix.match_prefix_with_params(path, req.match_info_mut()) - { + if let Some(prefix_len) = prefix.match_prefix_with_params(&mut req, len) { for filter in filters { if !filter.check(&mut req) { continue 'outer; } } - let prefix_len = len + prefix_len; - let path: &'static str = - unsafe { &*(&req.path()[prefix_len..] as *const _) }; - - req.set_prefix_len(prefix_len as u16); - req.match_info_mut().set("tail", path); + let url = req.url().clone(); + let prefix_len = (len + prefix_len) as u16; + req.set_prefix_len(prefix_len); + req.match_info_mut().set_tail(prefix_len); + req.match_info_mut().set_url(url); let hnd: &mut RouteHandler<_> = unsafe { (&mut *(handler.get())).as_mut() }; diff --git a/src/test.rs b/src/test.rs index cf9b7f1c1..19e682d8d 100644 --- a/src/test.rs +++ b/src/test.rs @@ -408,7 +408,7 @@ pub struct TestRequest { method: Method, uri: Uri, headers: HeaderMap, - params: Params<'static>, + params: Params, cookies: Option>>, payload: Option, } @@ -508,7 +508,7 @@ impl TestRequest { /// Set request path pattern parameter pub fn param(mut self, name: &'static str, value: &'static str) -> Self { - self.params.add(name, value); + self.params.add_static(name, value); self } diff --git a/src/uri.rs b/src/uri.rs index ce147024b..61ee93525 100644 --- a/src/uri.rs +++ b/src/uri.rs @@ -1,4 +1,5 @@ use http::Uri; +use std::rc::Rc; #[allow(dead_code)] const GEN_DELIMS: &[u8] = b":/?#[]@"; @@ -34,10 +35,10 @@ lazy_static! { static ref DEFAULT_QUOTER: Quoter = { Quoter::new(b"@:", b"/+") }; } -#[derive(Default)] +#[derive(Default, Clone, Debug)] pub(crate) struct Url { uri: Uri, - path: Option, + path: Option>, } impl Url { @@ -95,7 +96,7 @@ impl Quoter { q } - pub fn requote(&self, val: &[u8]) -> Option { + pub fn requote(&self, val: &[u8]) -> Option> { let mut has_pct = 0; let mut pct = [b'%', 0, 0]; let mut idx = 0; @@ -145,7 +146,7 @@ impl Quoter { } if let Some(data) = cloned { - Some(unsafe { String::from_utf8_unchecked(data) }) + Some(unsafe { Rc::new(String::from_utf8_unchecked(data)) }) } else { None }