Skip to content

Commit

Permalink
refactor(headers): use String in Access-Control-Allow-Origin header
Browse files Browse the repository at this point in the history
Access-Control-Allow-Origin origins are URLs but they do not need to
be valid, they should just be compared as strings. So to support
invalid URLs hyper should use a string instead.

closes #526

BREAKING CHANGE: Access-Control-Allow-Origin does no longer use Url
  • Loading branch information
pyfisch committed Jul 4, 2015
1 parent 990819a commit ed45862
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 13 deletions.
7 changes: 7 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::error::Error as StdError;
use std::fmt;
use std::io::Error as IoError;
use std::str::Utf8Error;
use std::string::FromUtf8Error;

use httparse;
use url;
Expand Down Expand Up @@ -127,6 +128,12 @@ impl From<Utf8Error> for Error {
}
}

impl From<FromUtf8Error> for Error {
fn from(err: FromUtf8Error) -> Error {
Utf8(err.utf8_error())
}
}

impl From<httparse::Error> for Error {
fn from(err: httparse::Error) -> Error {
match err {
Expand Down
25 changes: 12 additions & 13 deletions src/header/common/access_control_allow_origin.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use std::fmt::{self, Display};
use std::str;

use url::Url;
use header::{Header, HeaderFormat};

/// The `Access-Control-Allow-Origin` response header,
Expand All @@ -20,7 +18,7 @@ use header::{Header, HeaderFormat};
/// * `null`
/// * `*`
/// * `http://google.com/`
///
///
/// # Examples
/// ```
/// use hyper::header::{Headers, AccessControlAllowOrigin};
Expand All @@ -40,11 +38,10 @@ use header::{Header, HeaderFormat};
/// ```
/// ```
/// use hyper::header::{Headers, AccessControlAllowOrigin};
/// use hyper::Url;
///
/// let mut headers = Headers::new();
/// headers.set(
/// AccessControlAllowOrigin::Value(Url::parse("http://hyper.rs").unwrap())
/// AccessControlAllowOrigin::Value("http://hyper.rs".to_owned())
/// );
/// ```
#[derive(Clone, PartialEq, Debug)]
Expand All @@ -54,7 +51,7 @@ pub enum AccessControlAllowOrigin {
/// A hidden origin
Null,
/// Allow one particular origin
Value(Url),
Value(String),
}

impl Header for AccessControlAllowOrigin {
Expand All @@ -63,13 +60,15 @@ impl Header for AccessControlAllowOrigin {
}

fn parse_header(raw: &[Vec<u8>]) -> ::Result<AccessControlAllowOrigin> {
if raw.len() == 1 {
match unsafe { &raw.get_unchecked(0)[..] } {
b"*" => Ok(AccessControlAllowOrigin::Any),
b"null" => Ok(AccessControlAllowOrigin::Null),
r => Ok(AccessControlAllowOrigin::Value(try!(Url::parse(try!(str::from_utf8(r))))))
}
} else { Err(::Error::Header) }
if raw.len() != 1 {
return Err(::Error::Header)
}
let value = unsafe { raw.get_unchecked(0) };
Ok(match &value[..] {
b"*" => AccessControlAllowOrigin::Any,
b"null" => AccessControlAllowOrigin::Null,
_ => AccessControlAllowOrigin::Value(try!(String::from_utf8(value.clone())))
})
}
}

Expand Down

3 comments on commit ed45862

@annevk
Copy link

@annevk annevk commented on ed45862 Jul 6, 2015

Choose a reason for hiding this comment

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

Note also that by definition an origin (even when serialized) is not a URL. You might be able to parse it as one, but if you then serialize it you'll get a slightly different result.

@pyfisch
Copy link
Contributor Author

@pyfisch pyfisch commented on ed45862 Jul 6, 2015

Choose a reason for hiding this comment

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

Yes. Would you change something about the code? Maybe you could add a note to the docs?

@annevk
Copy link

@annevk annevk commented on ed45862 Jul 7, 2015

Choose a reason for hiding this comment

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

I think I would not even store utf-8, just the raw bytes. If you want anything changed in https://fetch.spec.whatwg.org/ please file an issue against the GitHub repository.

Please sign in to comment.