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.
hyperium · Aug 29, 2016 · 8b3c120 · 8b3c120
1 parent a228486
commit 8b3c120
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 24 deletions.
diff --git 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
@@ -165,7 +165,7 @@ impl Handler<Http> for Text {
     fn on_request(&mut self, req: Request<Http>) -> 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<Http> for Text {
     fn on_request(&mut self, req: Request<Http>) -> 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
@@ -45,7 +45,7 @@ impl Echo {
 impl Handler<HttpStream> for Echo {
     fn on_request(&mut self, req: Request<HttpStream>) -> 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
@@ -290,15 +290,17 @@ impl<H: Handler<T>, T: Transport> http::MessageHandler<T> for Message<H, T> {
     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 {
                 hostname: host.to_owned(),
                 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
@@ -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
@@ -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<String>,
+    },
 
     /// 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::<RequestUri>().unwrap(), result);
     }
+    fn parse_err(s: &str) {
+        assert!(s.parse::<RequestUri>().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()),
+    });
 }