From 8b3c1206846cb96be780923952eafe0dde7850bf Mon Sep 17 00:00:00 2001 From: Ahmed Charles Date: Mon, 29 Aug 2016 13:45:38 -0700 Subject: [PATCH] feat(server): add path() and query() to Request Closes #896 Closes #897 BREAKING CHANGE: `RequestUri::AbsolutePath` variant is changed to a struct variant. Consider using `req.path()` or `req.query()` to get the relevant slice. --- .travis.yml | 2 +- doc/guide/server.md | 4 ++-- examples/server.rs | 2 +- src/client/mod.rs | 6 +++-- src/server/request.rs | 20 +++++++++++----- src/uri.rs | 53 +++++++++++++++++++++++++++++++++---------- 6 files changed, 63 insertions(+), 24 deletions(-) diff --git a/.travis.yml b/.travis.yml index 86839084d9..db92750d50 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,7 +22,7 @@ script: - ./.travis/readme.py - cargo build --verbose $FEATURES - cargo test --verbose $FEATURES - - 'for f in ./doc/**/*.md; do rustdoc -L ./target/debug -L ./target/debug/deps --test $f; done' + - 'for f in ./doc/**/*.md; do echo "Running rustdoc on $f"; rustdoc -L ./target/debug -L ./target/debug/deps --test $f; done' addons: apt: diff --git a/doc/guide/server.md b/doc/guide/server.md index 15658a6c8f..5d2fdf4415 100644 --- a/doc/guide/server.md +++ b/doc/guide/server.md @@ -165,7 +165,7 @@ impl Handler for Text { fn on_request(&mut self, req: Request) -> Next { use hyper::RequestUri; let path = match *req.uri() { - RequestUri::AbsolutePath(ref p) => p, + RequestUri::AbsolutePath { path: ref p, .. } => p, RequestUri::AbsoluteUri(ref url) => url.path(), // other 2 forms are for CONNECT and OPTIONS methods _ => "" @@ -299,7 +299,7 @@ impl Handler for Text { fn on_request(&mut self, req: Request) -> Next { use hyper::RequestUri; let path = match *req.uri() { - RequestUri::AbsolutePath(ref p) => p, + RequestUri::AbsolutePath { path: ref p, .. } => p, RequestUri::AbsoluteUri(ref url) => url.path(), _ => "" }; diff --git a/examples/server.rs b/examples/server.rs index e999b4dc8f..a2dbe23c2b 100644 --- a/examples/server.rs +++ b/examples/server.rs @@ -45,7 +45,7 @@ impl Echo { impl Handler for Echo { fn on_request(&mut self, req: Request) -> Next { match *req.uri() { - RequestUri::AbsolutePath(ref path) => match (req.method(), &path[..]) { + RequestUri::AbsolutePath { ref path, .. } => match (req.method(), &path[..]) { (&Get, "/") | (&Get, "/echo") => { info!("GET Index"); self.route = Route::Index; diff --git a/src/client/mod.rs b/src/client/mod.rs index 50bd0d7914..10f1a1a081 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -290,7 +290,6 @@ impl, T: Transport> http::MessageHandler for Message { type Message = http::ClientMessage; fn on_outgoing(&mut self, head: &mut RequestHead) -> Next { - use ::url::Position; let url = self.url.take().expect("Message.url is missing"); if let Some(host) = url.host_str() { head.headers.set(Host { @@ -298,7 +297,10 @@ impl, T: Transport> http::MessageHandler for Message { port: url.port(), }); } - head.subject.1 = RequestUri::AbsolutePath(url[Position::BeforePath..Position::AfterQuery].to_owned()); + head.subject.1 = RequestUri::AbsolutePath { + path: url.path().to_owned(), + query: url.query().map(|q| q.to_owned()), + }; let mut req = self::request::new(head); self.handler.on_request(&mut req) } diff --git a/src/server/request.rs b/src/server/request.rs index cc6d435caf..dffea21e5c 100644 --- a/src/server/request.rs +++ b/src/server/request.rs @@ -66,17 +66,25 @@ impl<'a, T> Request<'a, T> { #[inline] pub fn version(&self) -> &HttpVersion { &self.version } - /* /// The target path of this Request. #[inline] pub fn path(&self) -> Option<&str> { - match *self.uri { - RequestUri::AbsolutePath(ref s) => Some(s), - RequestUri::AbsoluteUri(ref url) => Some(&url[::url::Position::BeforePath..]), - _ => None + match self.uri { + RequestUri::AbsolutePath { path: ref p, .. } => Some(p.as_str()), + RequestUri::AbsoluteUri(ref url) => Some(url.path()), + _ => None, + } + } + + /// The query string of this Request. + #[inline] + pub fn query(&self) -> Option<&str> { + match self.uri { + RequestUri::AbsolutePath { query: ref q, .. } => q.as_ref().map(|x| x.as_str()), + RequestUri::AbsoluteUri(ref url) => url.query(), + _ => None, } } - */ /// Deconstruct this Request into its pieces. /// diff --git a/src/uri.rs b/src/uri.rs index a155ab18aa..51335a6e6a 100644 --- a/src/uri.rs +++ b/src/uri.rs @@ -26,8 +26,13 @@ pub enum RequestUri { /// The most common request target, an absolute path and optional query. /// /// For example, the line `GET /where?q=now HTTP/1.1` would parse the URI - /// as `AbsolutePath("/where?q=now".to_string())`. - AbsolutePath(String), + /// as `AbsolutePath { path: "/where".to_string(), query: Some("q=now".to_string()) }`. + AbsolutePath { + /// The path part of the request uri. + path: String, + /// The query part of the request uri. + query: Option, + }, /// An absolute URI. Used in conjunction with proxies. /// @@ -66,13 +71,22 @@ impl FromStr for RequestUri { } else if bytes == b"*" { Ok(RequestUri::Star) } else if bytes.starts_with(b"/") { - Ok(RequestUri::AbsolutePath(s.to_owned())) + let mut temp = "http://example.com".to_owned(); + temp.push_str(s); + let url = try!(Url::parse(&temp)); + Ok(RequestUri::AbsolutePath { + path: url.path().to_owned(), + query: url.query().map(|q| q.to_owned()), + }) } else if bytes.contains(&b'/') { Ok(RequestUri::AbsoluteUri(try!(Url::parse(s)))) } else { let mut temp = "http://".to_owned(); temp.push_str(s); - try!(Url::parse(&temp[..])); + let url = try!(Url::parse(&temp)); + if url.query().is_some() { + return Err(Error::Uri(UrlError::RelativeUrlWithoutBase)); + } //TODO: compare vs u.authority()? Ok(RequestUri::Authority(s.to_owned())) } @@ -82,7 +96,13 @@ impl FromStr for RequestUri { impl Display for RequestUri { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { - RequestUri::AbsolutePath(ref path) => f.write_str(path), + RequestUri::AbsolutePath { ref path, ref query } => { + try!(f.write_str(path)); + match *query { + Some(ref q) => write!(f, "?{}", q), + None => Ok(()), + } + } RequestUri::AbsoluteUri(ref url) => write!(f, "{}", url), RequestUri::Authority(ref path) => f.write_str(path), RequestUri::Star => f.write_str("*") @@ -92,14 +112,20 @@ impl Display for RequestUri { #[test] fn test_uri_fromstr() { - fn read(s: &str, result: RequestUri) { + fn parse(s: &str, result: RequestUri) { assert_eq!(s.parse::().unwrap(), result); } + fn parse_err(s: &str) { + assert!(s.parse::().is_err()); + } - read("*", RequestUri::Star); - read("http://hyper.rs/", RequestUri::AbsoluteUri(Url::parse("http://hyper.rs/").unwrap())); - read("hyper.rs", RequestUri::Authority("hyper.rs".to_owned())); - read("/", RequestUri::AbsolutePath("/".to_owned())); + parse("*", RequestUri::Star); + parse("**", RequestUri::Authority("**".to_owned())); + parse("http://hyper.rs/", RequestUri::AbsoluteUri(Url::parse("http://hyper.rs/").unwrap())); + parse("hyper.rs", RequestUri::Authority("hyper.rs".to_owned())); + parse_err("hyper.rs?key=value"); + parse_err("hyper.rs/"); + parse("/", RequestUri::AbsolutePath { path: "/".to_owned(), query: None }); } #[test] @@ -111,6 +137,9 @@ fn test_uri_display() { assert_display("*", RequestUri::Star); assert_display("http://hyper.rs/", RequestUri::AbsoluteUri(Url::parse("http://hyper.rs/").unwrap())); assert_display("hyper.rs", RequestUri::Authority("hyper.rs".to_owned())); - assert_display("/", RequestUri::AbsolutePath("/".to_owned())); - + assert_display("/", RequestUri::AbsolutePath { path: "/".to_owned(), query: None }); + assert_display("/where?key=value", RequestUri::AbsolutePath { + path: "/where".to_owned(), + query: Some("key=value".to_owned()), + }); }