Skip to content

Commit

Permalink
fix(client): allow calling Destination::set_host with IPv6 addresses
Browse files Browse the repository at this point in the history
Closes #1661
  • Loading branch information
seanmonstar committed Sep 27, 2018
1 parent 0cfa723 commit 5896fcd
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 4 deletions.
49 changes: 45 additions & 4 deletions src/client/connect/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use std::error::Error as StdError;
use std::mem;

use bytes::{BufMut, BytesMut};
use bytes::{BufMut, Bytes, BytesMut};
use futures::Future;
use http::{uri, Uri};
use tokio_io::{AsyncRead, AsyncWrite};
Expand Down Expand Up @@ -131,13 +131,20 @@ impl Destination {
///
/// Returns an error if the string is not a valid hostname.
pub fn set_host(&mut self, host: &str) -> ::Result<()> {
if host.contains(&['@',':'][..]) {
// Prevent any userinfo setting, it's bad!
if host.contains('@') {
return Err(::error::Parse::Uri.into());
}
let auth = if let Some(port) = self.port() {
format!("{}:{}", host, port).parse().map_err(::error::Parse::from)?
let bytes = Bytes::from(format!("{}:{}", host, port));
uri::Authority::from_shared(bytes)
.map_err(::error::Parse::from)?
} else {
host.parse().map_err(::error::Parse::from)?
let auth = host.parse::<uri::Authority>().map_err(::error::Parse::from)?;
if auth.port().is_some() {
return Err(::error::Parse::Uri.into());
}
auth
};
self.update_uri(move |parts| {
parts.authority = Some(auth);
Expand Down Expand Up @@ -305,6 +312,30 @@ mod tests {
assert_eq!(dst.host(), "seanmonstar.com", "error doesn't modify dst");
assert_eq!(dst.port(), None, "error doesn't modify dst");

// Check port isn't snuck into `set_host`.
dst.set_host("seanmonstar.com:3030").expect_err("set_host sneaky port");
assert_eq!(dst.scheme(), "http", "error doesn't modify dst");
assert_eq!(dst.host(), "seanmonstar.com", "error doesn't modify dst");
assert_eq!(dst.port(), None, "error doesn't modify dst");

// Check userinfo isn't snuck into `set_host`.
dst.set_host("sean@nope").expect_err("set_host sneaky userinfo");
assert_eq!(dst.scheme(), "http", "error doesn't modify dst");
assert_eq!(dst.host(), "seanmonstar.com", "error doesn't modify dst");
assert_eq!(dst.port(), None, "error doesn't modify dst");

// Allow IPv6 hosts
dst.set_host("[::1]").expect("set_host with IPv6");
assert_eq!(dst.host(), "::1");
assert_eq!(dst.port(), None, "IPv6 didn't affect port");

// However, IPv6 with a port is rejected.
dst.set_host("[::2]:1337").expect_err("set_host with IPv6 and sneaky port");
assert_eq!(dst.host(), "::1");
assert_eq!(dst.port(), None);

// -----------------

// Also test that an exist port is set correctly.
let mut dst = Destination {
uri: "http://hyper.rs:8080".parse().expect("initial parse 2"),
Expand Down Expand Up @@ -335,6 +366,16 @@ mod tests {
assert_eq!(dst.scheme(), "http", "error doesn't modify dst");
assert_eq!(dst.host(), "seanmonstar.com", "error doesn't modify dst");
assert_eq!(dst.port(), Some(8080), "error doesn't modify dst");

// Allow IPv6 hosts
dst.set_host("[::1]").expect("set_host with IPv6");
assert_eq!(dst.host(), "::1");
assert_eq!(dst.port(), Some(8080), "IPv6 didn't affect port");

// However, IPv6 with a port is rejected.
dst.set_host("[::2]:1337").expect_err("set_host with IPv6 and sneaky port");
assert_eq!(dst.host(), "::1");
assert_eq!(dst.port(), Some(8080));
}

#[test]
Expand Down
6 changes: 6 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,12 @@ impl From<http::uri::InvalidUri> for Parse {
}
}

impl From<http::uri::InvalidUriBytes> for Parse {
fn from(_: http::uri::InvalidUriBytes) -> Parse {
Parse::Uri
}
}

impl From<http::uri::InvalidUriParts> for Parse {
fn from(_: http::uri::InvalidUriParts) -> Parse {
Parse::Uri
Expand Down

0 comments on commit 5896fcd

Please sign in to comment.