Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(server): add path() and query() to Request #899

Merged
merged 1 commit into from
Aug 29, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions doc/guide/server.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
_ => ""
Expand Down Expand Up @@ -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(),
_ => ""
};
Expand Down
2 changes: 1 addition & 1 deletion examples/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
20 changes: 14 additions & 6 deletions src/server/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down
53 changes: 41 additions & 12 deletions src/uri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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()))
}
Expand All @@ -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("*")
Expand All @@ -92,14 +112,20 @@ impl Display for RequestUri {

#[test]
fn test_uri_fromstr() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be in favor of this test instead having 2 helpers, to prevent the proliferation of Some(...) in every test. Maybe something like:

assert_eq!(parse("*"), RequestUri::Star);
assert!(parse_err("**"));

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]
Expand All @@ -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()),
});
}