From 195a89fa918a83c9dcab47a4b09edb464d4e8006 Mon Sep 17 00:00:00 2001 From: Pyfisch Date: Sat, 6 Jun 2015 22:04:01 +0200 Subject: [PATCH] refactor(headers): errors for parse_header Header::parse_header() returns now a hyper Result instead of an option this will enable more precise Error messages in the future, currently most failures are reported as ::Error::Header. BREAKING CHANGE: parse_header returns Result instead of Option, related code did also change --- benches/client.rs | 5 +-- src/error.rs | 11 +++++ src/header/common/accept_ranges.rs | 4 +- .../common/access_control_allow_origin.rs | 12 +++--- src/header/common/authorization.rs | 37 +++++++++------- src/header/common/cache_control.rs | 20 ++++----- src/header/common/cookie.rs | 27 ++++++------ src/header/common/expect.rs | 8 ++-- src/header/common/host.rs | 10 ++--- src/header/common/if_none_match.rs | 6 +-- src/header/common/if_range.rs | 16 +++---- src/header/common/mod.rs | 12 +++--- src/header/common/pragma.rs | 10 ++--- src/header/common/set_cookie.rs | 8 ++-- src/header/common/upgrade.rs | 4 +- src/header/common/vary.rs | 10 ++--- src/header/internals/item.rs | 10 ++--- src/header/mod.rs | 19 ++++---- src/header/parsing.rs | 43 ++++++++----------- 19 files changed, 140 insertions(+), 132 deletions(-) diff --git a/benches/client.rs b/benches/client.rs index 984ce13191..ff11a712ca 100644 --- a/benches/client.rs +++ b/benches/client.rs @@ -58,8 +58,8 @@ impl hyper::header::Header for Foo { fn header_name() -> &'static str { "x-foo" } - fn parse_header(_: &[Vec]) -> Option { - None + fn parse_header(_: &[Vec]) -> hyper::Result { + Err(hyper::Error::Header) } } @@ -104,4 +104,3 @@ fn bench_mock_hyper(b: &mut test::Bencher) { .read_to_string(&mut s).unwrap() }); } - diff --git a/src/error.rs b/src/error.rs index 96844f45d0..762f29e66a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -2,6 +2,7 @@ use std::error::Error as StdError; use std::fmt; use std::io::Error as IoError; +use std::str::Utf8Error; use httparse; use openssl::ssl::error::SslError; @@ -18,6 +19,7 @@ use self::Error::{ Ssl, TooLarge, Http2, + Utf8 }; @@ -45,6 +47,8 @@ pub enum Error { Ssl(SslError), /// An HTTP/2-specific error, coming from the `solicit` library. Http2(Http2Error), + /// Parsing a field as string failed + Utf8(Utf8Error), #[doc(hidden)] __Nonexhaustive(Void) @@ -77,6 +81,7 @@ impl StdError for Error { Io(ref e) => e.description(), Ssl(ref e) => e.description(), Http2(ref e) => e.description(), + Utf8(ref e) => e.description(), Error::__Nonexhaustive(ref void) => match *void {} } } @@ -113,6 +118,12 @@ impl From for Error { } } +impl From for Error { + fn from(err: Utf8Error) -> Error { + Utf8(err) + } +} + impl From for Error { fn from(err: httparse::Error) -> Error { match err { diff --git a/src/header/common/accept_ranges.rs b/src/header/common/accept_ranges.rs index 9b15bbd548..ffcceeeb9d 100644 --- a/src/header/common/accept_ranges.rs +++ b/src/header/common/accept_ranges.rs @@ -52,8 +52,8 @@ pub enum RangeUnit { impl FromStr for RangeUnit { - type Err = (); - fn from_str(s: &str) -> Result { + type Err = ::Error; + fn from_str(s: &str) -> ::Result { match s { "bytes" => Ok(RangeUnit::Bytes), "none" => Ok(RangeUnit::None), diff --git a/src/header/common/access_control_allow_origin.rs b/src/header/common/access_control_allow_origin.rs index 06d758e3e2..fff5ff3d20 100644 --- a/src/header/common/access_control_allow_origin.rs +++ b/src/header/common/access_control_allow_origin.rs @@ -35,16 +35,14 @@ impl Header for AccessControlAllowOrigin { "Access-Control-Allow-Origin" } - fn parse_header(raw: &[Vec]) -> Option { + fn parse_header(raw: &[Vec]) -> ::Result { if raw.len() == 1 { match unsafe { &raw.get_unchecked(0)[..] } { - b"*" => Some(AccessControlAllowOrigin::Any), - b"null" => Some(AccessControlAllowOrigin::Null), - r => if let Ok(s) = str::from_utf8(r) { - Url::parse(s).ok().map(AccessControlAllowOrigin::Value) - } else { None } + b"*" => Ok(AccessControlAllowOrigin::Any), + b"null" => Ok(AccessControlAllowOrigin::Null), + r => Ok(AccessControlAllowOrigin::Value(try!(Url::parse(try!(str::from_utf8(r)))))) } - } else { None } + } else { Err(::Error::Header) } } } diff --git a/src/header/common/authorization.rs b/src/header/common/authorization.rs index f5a05073ed..f7724cbae1 100644 --- a/src/header/common/authorization.rs +++ b/src/header/common/authorization.rs @@ -42,18 +42,25 @@ impl Header for Authorization where ::Err: 'st "Authorization" } - fn parse_header(raw: &[Vec]) -> Option> { - if raw.len() == 1 { - match (from_utf8(unsafe { &raw.get_unchecked(0)[..] }), ::scheme()) { - (Ok(header), Some(scheme)) - if header.starts_with(scheme) && header.len() > scheme.len() + 1 => { - header[scheme.len() + 1..].parse::().map(Authorization).ok() - }, - (Ok(header), None) => header.parse::().map(Authorization).ok(), - _ => None + fn parse_header(raw: &[Vec]) -> ::Result> { + if raw.len() != 1 { + return Err(::Error::Header); + } + let header = try!(from_utf8(unsafe { &raw.get_unchecked(0)[..] })); + return if let Some(scheme) = ::scheme() { + if header.starts_with(scheme) && header.len() > scheme.len() + 1 { + match header[scheme.len() + 1..].parse::().map(Authorization) { + Ok(h) => Ok(h), + Err(_) => Err(::Error::Header) + } + } else { + Err(::Error::Header) } } else { - None + match header.parse::().map(Authorization) { + Ok(h) => Ok(h), + Err(_) => Err(::Error::Header) + } } } } @@ -121,15 +128,15 @@ impl Scheme for Basic { } impl FromStr for Basic { - type Err = (); - fn from_str(s: &str) -> Result { + type Err = ::Error; + fn from_str(s: &str) -> ::Result { match s.from_base64() { Ok(decoded) => match String::from_utf8(decoded) { Ok(text) => { let mut parts = &mut text.split(':'); let user = match parts.next() { Some(part) => part.to_owned(), - None => return Err(()) + None => return Err(::Error::Header) }; let password = match parts.next() { Some(part) => Some(part.to_owned()), @@ -142,12 +149,12 @@ impl FromStr for Basic { }, Err(e) => { debug!("Basic::from_utf8 error={:?}", e); - Err(()) + Err(::Error::Header) } }, Err(e) => { debug!("Basic::from_base64 error={:?}", e); - Err(()) + Err(::Error::Header) } } } diff --git a/src/header/common/cache_control.rs b/src/header/common/cache_control.rs index ab7dd6cac8..c937c01848 100644 --- a/src/header/common/cache_control.rs +++ b/src/header/common/cache_control.rs @@ -30,15 +30,15 @@ impl Header for CacheControl { "Cache-Control" } - fn parse_header(raw: &[Vec]) -> Option { + fn parse_header(raw: &[Vec]) -> ::Result { let directives = raw.iter() - .filter_map(|line| from_one_comma_delimited(&line[..])) + .filter_map(|line| from_one_comma_delimited(&line[..]).ok()) .collect::>>() .concat(); if !directives.is_empty() { - Some(CacheControl(directives)) + Ok(CacheControl(directives)) } else { - None + Err(::Error::Header) } } } @@ -148,35 +148,35 @@ mod tests { #[test] fn test_parse_multiple_headers() { let cache = Header::parse_header(&[b"no-cache".to_vec(), b"private".to_vec()]); - assert_eq!(cache, Some(CacheControl(vec![CacheDirective::NoCache, + assert_eq!(cache.ok(), Some(CacheControl(vec![CacheDirective::NoCache, CacheDirective::Private]))) } #[test] fn test_parse_argument() { let cache = Header::parse_header(&[b"max-age=100, private".to_vec()]); - assert_eq!(cache, Some(CacheControl(vec![CacheDirective::MaxAge(100), + assert_eq!(cache.ok(), Some(CacheControl(vec![CacheDirective::MaxAge(100), CacheDirective::Private]))) } #[test] fn test_parse_quote_form() { let cache = Header::parse_header(&[b"max-age=\"200\"".to_vec()]); - assert_eq!(cache, Some(CacheControl(vec![CacheDirective::MaxAge(200)]))) + assert_eq!(cache.ok(), Some(CacheControl(vec![CacheDirective::MaxAge(200)]))) } #[test] fn test_parse_extension() { let cache = Header::parse_header(&[b"foo, bar=baz".to_vec()]); - assert_eq!(cache, Some(CacheControl(vec![ + assert_eq!(cache.ok(), Some(CacheControl(vec![ CacheDirective::Extension("foo".to_owned(), None), CacheDirective::Extension("bar".to_owned(), Some("baz".to_owned()))]))) } #[test] fn test_parse_bad_syntax() { - let cache: Option = Header::parse_header(&[b"foo=".to_vec()]); - assert_eq!(cache, None) + let cache: ::Result = Header::parse_header(&[b"foo=".to_vec()]); + assert_eq!(cache.ok(), None) } } diff --git a/src/header/common/cookie.rs b/src/header/common/cookie.rs index 6322c400c3..07ff710a5e 100644 --- a/src/header/common/cookie.rs +++ b/src/header/common/cookie.rs @@ -27,26 +27,23 @@ impl Header for Cookie { "Cookie" } - fn parse_header(raw: &[Vec]) -> Option { + fn parse_header(raw: &[Vec]) -> ::Result { let mut cookies = Vec::with_capacity(raw.len()); for cookies_raw in raw.iter() { - match from_utf8(&cookies_raw[..]) { - Ok(cookies_str) => { - for cookie_str in cookies_str.split(';') { - match cookie_str.trim().parse() { - Ok(cookie) => cookies.push(cookie), - Err(_) => return None - } - } - }, - Err(_) => return None - }; + let cookies_str = try!(from_utf8(&cookies_raw[..])); + for cookie_str in cookies_str.split(';') { + if let Ok(cookie) = cookie_str.trim().parse() { + cookies.push(cookie); + } else { + return Err(::Error::Header); + } + } } if !cookies.is_empty() { - Some(Cookie(cookies)) + Ok(Cookie(cookies)) } else { - None + Err(::Error::Header) } } } @@ -88,7 +85,7 @@ fn test_parse() { let h = Header::parse_header(&[b"foo=bar; baz=quux".to_vec()][..]); let c1 = CookiePair::new("foo".to_owned(), "bar".to_owned()); let c2 = CookiePair::new("baz".to_owned(), "quux".to_owned()); - assert_eq!(h, Some(Cookie(vec![c1, c2]))); + assert_eq!(h.ok(), Some(Cookie(vec![c1, c2]))); } #[test] diff --git a/src/header/common/expect.rs b/src/header/common/expect.rs index 0bb9eac601..109ce70723 100644 --- a/src/header/common/expect.rs +++ b/src/header/common/expect.rs @@ -26,7 +26,7 @@ impl Header for Expect { "Expect" } - fn parse_header(raw: &[Vec]) -> Option { + fn parse_header(raw: &[Vec]) -> ::Result { if raw.len() == 1 { let text = unsafe { // safe because: @@ -38,12 +38,12 @@ impl Header for Expect { str::from_utf8_unchecked(raw.get_unchecked(0)) }; if UniCase(text) == EXPECT_CONTINUE { - Some(Expect::Continue) + Ok(Expect::Continue) } else { - None + Err(::Error::Header) } } else { - None + Err(::Error::Header) } } } diff --git a/src/header/common/host.rs b/src/header/common/host.rs index 2984f27c32..4f628dc638 100644 --- a/src/header/common/host.rs +++ b/src/header/common/host.rs @@ -22,7 +22,7 @@ impl Header for Host { "Host" } - fn parse_header(raw: &[Vec]) -> Option { + fn parse_header(raw: &[Vec]) -> ::Result { from_one_raw_str(raw).and_then(|mut s: String| { // FIXME: use rust-url to parse this // https://github.com/servo/rust-url/issues/42 @@ -39,7 +39,7 @@ impl Header for Host { None } } - None => return None // this is a bad ipv6 address... + None => return Err(::Error::Header) // this is a bad ipv6 address... } } else { slice.rfind(':') @@ -56,7 +56,7 @@ impl Header for Host { None => () } - Some(Host { + Ok(Host { hostname: s, port: port }) @@ -82,14 +82,14 @@ mod tests { #[test] fn test_host() { let host = Header::parse_header([b"foo.com".to_vec()].as_ref()); - assert_eq!(host, Some(Host { + assert_eq!(host.ok(), Some(Host { hostname: "foo.com".to_owned(), port: None })); let host = Header::parse_header([b"foo.com:8080".to_vec()].as_ref()); - assert_eq!(host, Some(Host { + assert_eq!(host.ok(), Some(Host { hostname: "foo.com".to_owned(), port: Some(8080) })); diff --git a/src/header/common/if_none_match.rs b/src/header/common/if_none_match.rs index 7d801045bb..8fb249d526 100644 --- a/src/header/common/if_none_match.rs +++ b/src/header/common/if_none_match.rs @@ -45,10 +45,10 @@ mod tests { #[test] fn test_if_none_match() { - let mut if_none_match: Option; + let mut if_none_match: ::Result; if_none_match = Header::parse_header([b"*".to_vec()].as_ref()); - assert_eq!(if_none_match, Some(IfNoneMatch::Any)); + assert_eq!(if_none_match.ok(), Some(IfNoneMatch::Any)); if_none_match = Header::parse_header([b"\"foobar\", W/\"weak-etag\"".to_vec()].as_ref()); let mut entities: Vec = Vec::new(); @@ -56,7 +56,7 @@ mod tests { let weak_etag = EntityTag::new(true, "weak-etag".to_owned()); entities.push(foobar_etag); entities.push(weak_etag); - assert_eq!(if_none_match, Some(IfNoneMatch::Items(entities))); + assert_eq!(if_none_match.ok(), Some(IfNoneMatch::Items(entities))); } } diff --git a/src/header/common/if_range.rs b/src/header/common/if_range.rs index c3b0f12029..1f02e53966 100644 --- a/src/header/common/if_range.rs +++ b/src/header/common/if_range.rs @@ -36,16 +36,16 @@ impl Header for IfRange { fn header_name() -> &'static str { "If-Range" } - fn parse_header(raw: &[Vec]) -> Option { - let etag: Option = header::parsing::from_one_raw_str(raw); - if etag != None { - return Some(IfRange::EntityTag(etag.unwrap())); + fn parse_header(raw: &[Vec]) -> ::Result { + let etag: ::Result = header::parsing::from_one_raw_str(raw); + if etag.is_ok() { + return Ok(IfRange::EntityTag(etag.unwrap())); } - let date: Option = header::parsing::from_one_raw_str(raw); - if date != None { - return Some(IfRange::Date(date.unwrap())); + let date: ::Result = header::parsing::from_one_raw_str(raw); + if date.is_ok() { + return Ok(IfRange::Date(date.unwrap())); } - None + Err(::Error::Header) } } diff --git a/src/header/common/mod.rs b/src/header/common/mod.rs index 79f9cc3585..987efc3701 100644 --- a/src/header/common/mod.rs +++ b/src/header/common/mod.rs @@ -144,7 +144,7 @@ macro_rules! test_header { let a: Vec> = $raw.iter().map(|x| x.to_vec()).collect(); let val = HeaderField::parse_header(&a[..]); // Test parsing - assert_eq!(val, $typed); + assert_eq!(val.ok(), $typed); // Test formatting if $typed != None { let res: &str = str::from_utf8($raw[0]).unwrap(); @@ -171,7 +171,7 @@ macro_rules! header { fn header_name() -> &'static str { $n } - fn parse_header(raw: &[Vec]) -> Option { + fn parse_header(raw: &[Vec]) -> $crate::Result { $crate::header::parsing::from_comma_delimited(raw).map($id) } } @@ -197,7 +197,7 @@ macro_rules! header { fn header_name() -> &'static str { $n } - fn parse_header(raw: &[Vec]) -> Option { + fn parse_header(raw: &[Vec]) -> $crate::Result { $crate::header::parsing::from_comma_delimited(raw).map($id) } } @@ -223,7 +223,7 @@ macro_rules! header { fn header_name() -> &'static str { $n } - fn parse_header(raw: &[Vec]) -> Option { + fn parse_header(raw: &[Vec]) -> $crate::Result { $crate::header::parsing::from_one_raw_str(raw).map($id) } } @@ -252,11 +252,11 @@ macro_rules! header { fn header_name() -> &'static str { $n } - fn parse_header(raw: &[Vec]) -> Option { + fn parse_header(raw: &[Vec]) -> $crate::Result { // FIXME: Return None if no item is in $id::Only if raw.len() == 1 { if raw[0] == b"*" { - return Some($id::Any) + return Ok($id::Any) } } $crate::header::parsing::from_comma_delimited(raw).map(|vec| $id::Items(vec)) diff --git a/src/header/common/pragma.rs b/src/header/common/pragma.rs index 21b0c0db04..50e82966ca 100644 --- a/src/header/common/pragma.rs +++ b/src/header/common/pragma.rs @@ -29,12 +29,12 @@ impl Header for Pragma { "Pragma" } - fn parse_header(raw: &[Vec]) -> Option { + fn parse_header(raw: &[Vec]) -> ::Result { parsing::from_one_raw_str(raw).and_then(|s: String| { let slice = &s.to_ascii_lowercase()[..]; match slice { - "no-cache" => Some(Pragma::NoCache), - _ => Some(Pragma::Ext(s)), + "no-cache" => Ok(Pragma::NoCache), + _ => Ok(Pragma::Ext(s)), } }) } @@ -57,6 +57,6 @@ fn test_parse_header() { let c: Pragma = Header::parse_header([b"FoObar".to_vec()].as_ref()).unwrap(); let d = Pragma::Ext("FoObar".to_owned()); assert_eq!(c, d); - let e: Option = Header::parse_header([b"".to_vec()].as_ref()); - assert_eq!(e, None); + let e: ::Result = Header::parse_header([b"".to_vec()].as_ref()); + assert_eq!(e.ok(), None); } diff --git a/src/header/common/set_cookie.rs b/src/header/common/set_cookie.rs index 8177fb49a7..a2b02f1e7b 100644 --- a/src/header/common/set_cookie.rs +++ b/src/header/common/set_cookie.rs @@ -64,7 +64,7 @@ impl Header for SetCookie { "Set-Cookie" } - fn parse_header(raw: &[Vec]) -> Option { + fn parse_header(raw: &[Vec]) -> ::Result { let mut set_cookies = Vec::with_capacity(raw.len()); for set_cookies_raw in raw { if let Ok(s) = from_utf8(&set_cookies_raw[..]) { @@ -75,9 +75,9 @@ impl Header for SetCookie { } if !set_cookies.is_empty() { - Some(SetCookie(set_cookies)) + Ok(SetCookie(set_cookies)) } else { - None + Err(::Error::Header) } } @@ -120,7 +120,7 @@ fn test_parse() { let mut c1 = Cookie::new("foo".to_owned(), "bar".to_owned()); c1.httponly = true; - assert_eq!(h, Some(SetCookie(vec![c1]))); + assert_eq!(h.ok(), Some(SetCookie(vec![c1]))); } #[test] diff --git a/src/header/common/upgrade.rs b/src/header/common/upgrade.rs index 69bccd02f2..5ae7cf7146 100644 --- a/src/header/common/upgrade.rs +++ b/src/header/common/upgrade.rs @@ -45,8 +45,8 @@ header! { Some(Upgrade(vec![Protocol::new(ProtocolName::WebSocket, None)]))); #[test] fn test3() { - let x: Option = Header::parse_header(&[b"WEbSOCKet".to_vec()]); - assert_eq!(x, Some(Upgrade(vec![Protocol::new(ProtocolName::WebSocket, None)]))); + let x: ::Result = Header::parse_header(&[b"WEbSOCKet".to_vec()]); + assert_eq!(x.ok(), Some(Upgrade(vec![Protocol::new(ProtocolName::WebSocket, None)]))); } } } diff --git a/src/header/common/vary.rs b/src/header/common/vary.rs index 65a32725d5..d49058add9 100644 --- a/src/header/common/vary.rs +++ b/src/header/common/vary.rs @@ -24,15 +24,15 @@ header! { #[test] fn test2() { - let mut vary: Option; + let mut vary: ::Result; vary = Header::parse_header([b"*".to_vec()].as_ref()); - assert_eq!(vary, Some(Vary::Any)); + assert_eq!(vary.ok(), Some(Vary::Any)); vary = Header::parse_header([b"etag,cookie,allow".to_vec()].as_ref()); - assert_eq!(vary, Some(Vary::Items(vec!["eTag".parse().unwrap(), - "cookIE".parse().unwrap(), - "AlLOw".parse().unwrap(),]))); + assert_eq!(vary.ok(), Some(Vary::Items(vec!["eTag".parse().unwrap(), + "cookIE".parse().unwrap(), + "AlLOw".parse().unwrap(),]))); } } } diff --git a/src/header/internals/item.rs b/src/header/internals/item.rs index b9afa8f575..81a637436d 100644 --- a/src/header/internals/item.rs +++ b/src/header/internals/item.rs @@ -60,11 +60,11 @@ impl Item { Some(val) => Some(val), None => { match parse::(self.raw.as_ref().expect("item.raw must exist")) { - Some(typed) => { + Ok(typed) => { unsafe { self.typed.insert(tid, typed); } self.typed.get(tid) }, - None => None + Err(_) => None } } }.map(|typed| unsafe { typed.downcast_ref_unchecked() }) @@ -74,10 +74,10 @@ impl Item { let tid = TypeId::of::(); if self.typed.get_mut(tid).is_none() { match parse::(self.raw.as_ref().expect("item.raw must exist")) { - Some(typed) => { + Ok(typed) => { unsafe { self.typed.insert(tid, typed); } }, - None => () + Err(_) => () } } self.typed.get_mut(tid).map(|typed| unsafe { typed.downcast_mut_unchecked() }) @@ -85,7 +85,7 @@ impl Item { } #[inline] -fn parse(raw: &Vec>) -> Option> { +fn parse(raw: &Vec>) -> ::Result> { Header::parse_header(&raw[..]).map(|h: H| { // FIXME: Use Type ascription let h: Box = Box::new(h); diff --git a/src/header/mod.rs b/src/header/mod.rs index 43b1ec47e2..087ff1e64f 100644 --- a/src/header/mod.rs +++ b/src/header/mod.rs @@ -44,7 +44,7 @@ pub trait Header: Clone + Any + Send + Sync { /// it's not necessarily the case that a Header is *allowed* to have more /// than one field value. If that's the case, you **should** return `None` /// if `raw.len() > 1`. - fn parse_header(raw: &[Vec]) -> Option; + fn parse_header(raw: &[Vec]) -> ::Result; } @@ -420,7 +420,7 @@ mod tests { #[test] fn test_content_type() { let content_type = Header::parse_header([b"text/plain".to_vec()].as_ref()); - assert_eq!(content_type, Some(ContentType(Mime(Text, Plain, vec![])))); + assert_eq!(content_type.ok(), Some(ContentType(Mime(Text, Plain, vec![])))); } #[test] @@ -429,10 +429,10 @@ mod tests { let application_vendor = "application/vnd.github.v3.full+json; q=0.5".parse().unwrap(); let accept = Header::parse_header([b"text/plain".to_vec()].as_ref()); - assert_eq!(accept, Some(Accept(vec![text_plain.clone()]))); + assert_eq!(accept.ok(), Some(Accept(vec![text_plain.clone()]))); let accept = Header::parse_header([b"application/vnd.github.v3.full+json; q=0.5, text/plain".to_vec()].as_ref()); - assert_eq!(accept, Some(Accept(vec![application_vendor, text_plain]))); + assert_eq!(accept.ok(), Some(Accept(vec![application_vendor, text_plain]))); } #[derive(Clone, PartialEq, Debug)] @@ -442,18 +442,21 @@ mod tests { fn header_name() -> &'static str { "content-length" } - fn parse_header(raw: &[Vec]) -> Option { + fn parse_header(raw: &[Vec]) -> ::Result { use std::str::from_utf8; use std::str::FromStr; if raw.len() != 1 { - return None; + return Err(::Error::Header); } // we JUST checked that raw.len() == 1, so raw[0] WILL exist. - match from_utf8(unsafe { &raw.get_unchecked(0)[..] }) { + match match from_utf8(unsafe { &raw.get_unchecked(0)[..] }) { Ok(s) => FromStr::from_str(s).ok(), Err(_) => None - }.map(|u| CrazyLength(Some(false), u)) + }.map(|u| CrazyLength(Some(false), u)) { + Some(x) => Ok(x), + None => Err(::Error::Header), + } } } diff --git a/src/header/parsing.rs b/src/header/parsing.rs index 5d4744de49..5a4e572839 100644 --- a/src/header/parsing.rs +++ b/src/header/parsing.rs @@ -4,44 +4,37 @@ use std::str; use std::fmt::{self, Display}; /// Reads a single raw string when parsing a header -pub fn from_one_raw_str(raw: &[Vec]) -> Option { - if raw.len() != 1 { - return None; - } +pub fn from_one_raw_str(raw: &[Vec]) -> ::Result { + if raw.len() != 1 || unsafe { raw.get_unchecked(0) } == b"" { return Err(::Error::Header) } // we JUST checked that raw.len() == 1, so raw[0] WILL exist. - if let Ok(s) = str::from_utf8(& unsafe { raw.get_unchecked(0) }[..]) { - if s != "" { - return str::FromStr::from_str(s).ok(); - } + let s: &str = try!(str::from_utf8(& unsafe { raw.get_unchecked(0) }[..])); + if let Ok(x) = str::FromStr::from_str(s) { + Ok(x) + } else { + Err(::Error::Header) } - None } /// Reads a comma-delimited raw header into a Vec. #[inline] -pub fn from_comma_delimited(raw: &[Vec]) -> Option> { +pub fn from_comma_delimited(raw: &[Vec]) -> ::Result> { if raw.len() != 1 { - return None; + return Err(::Error::Header); } // we JUST checked that raw.len() == 1, so raw[0] WILL exist. from_one_comma_delimited(& unsafe { raw.get_unchecked(0) }[..]) } /// Reads a comma-delimited raw string into a Vec. -pub fn from_one_comma_delimited(raw: &[u8]) -> Option> { - match str::from_utf8(raw) { - Ok(s) => { - Some(s - .split(',') - .filter_map(|x| match x.trim() { - "" => None, - y => Some(y) - }) - .filter_map(|x| x.parse().ok()) - .collect()) - } - Err(_) => None - } +pub fn from_one_comma_delimited(raw: &[u8]) -> ::Result> { + let s = try!(str::from_utf8(raw)); + Ok(s.split(',') + .filter_map(|x| match x.trim() { + "" => None, + y => Some(y) + }) + .filter_map(|x| x.parse().ok()) + .collect()) } /// Format an array into a comma-delimited string.