From 2ca119fd80f9ea5f52f7393776b1d84a3da2e20b Mon Sep 17 00:00:00 2001 From: Jonathan Reem Date: Mon, 15 Sep 2014 19:21:17 -0700 Subject: [PATCH 1/3] Added Typeable dependency. --- Cargo.toml | 3 +++ src/lib.rs | 1 + 2 files changed, 4 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index afca46b6..bb32c536 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,9 @@ git = "https://github.com/reem/rust-intertwine" [dependencies.move-acceptor] git = "https://github.com/reem/rust-move-acceptor" +[dependencies.typeable] +git = "https://github.com/reem/rust-typeable" + [dev-dependencies.curl] git = "https://github.com/carllerche/curl-rust" diff --git a/src/lib.rs b/src/lib.rs index a4d26cba..a78979ce 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,6 +11,7 @@ extern crate url; extern crate "unsafe-any" as uany; extern crate "move-acceptor" as macceptor; extern crate intertwine; +extern crate typeable; pub use std::io::net::ip::{SocketAddr, IpAddr, Ipv4Addr, Ipv6Addr, Port}; pub use mimewrapper::mime; From fdcd25356da1571fe8501bb8594c1c123a484e67 Mon Sep 17 00:00:00 2001 From: Jonathan Reem Date: Mon, 15 Sep 2014 19:21:21 -0700 Subject: [PATCH 2/3] Refactor Header representation to not store the raw representation This disallows reparsing, but since that can be a significant source of errors I think this is actually beneficial. This also refactors to avoid storing the TypeId, though that is less of a gain. --- src/header/mod.rs | 97 +++++++++++++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 42 deletions(-) diff --git a/src/header/mod.rs b/src/header/mod.rs index 587c6ec1..5f2e026c 100644 --- a/src/header/mod.rs +++ b/src/header/mod.rs @@ -15,6 +15,7 @@ use std::string::raw; use std::collections::hashmap::{HashMap, Entries}; use uany::UncheckedAnyDowncast; +use typeable::Typeable; use http::read_header; use {HttpResult}; @@ -23,7 +24,7 @@ use {HttpResult}; pub mod common; /// A trait for any object that will represent a header field and value. -pub trait Header: 'static { +pub trait Header: Typeable { /// Returns the name of the header field this belongs to. /// /// The market `Option` is to hint to the type system which implementation @@ -41,7 +42,18 @@ pub trait Header: 'static { fn fmt_header(&self, fmt: &mut fmt::Formatter) -> fmt::Result; } -impl<'a> UncheckedAnyDowncast<'a> for &'a Header + 'a { +#[doc(hidden)] +trait Is { + fn is(self) -> bool; +} + +impl<'a> Is for &'a Header { + fn is(self) -> bool { + self.get_type() == TypeId::of::() + } +} + +impl<'a> UncheckedAnyDowncast<'a> for &'a Header { #[inline] unsafe fn downcast_ref_unchecked(self) -> &'a T { let to: TraitObject = transmute_copy(&self); @@ -81,8 +93,12 @@ impl Headers { let name = unsafe { raw::from_utf8(name) }.into_ascii_lower(); - let item = headers.data.find_or_insert(Owned(name), raw(vec![])); - item.raw.push(value); + let item = headers.data.find_or_insert(Owned(name), Raw(vec![])); + match *item { + Raw(ref mut raw) => raw.push(value), + // Unreachable + _ => {} + }; }, None => break, } @@ -94,11 +110,7 @@ impl Headers { /// /// The field is determined by the type of the value being set. pub fn set(&mut self, value: H) { - self.data.insert(Slice(header_name::()), Item { - raw: vec![], - tid: Some(TypeId::of::()), - typed: Some(box value as Box
) - }); + self.data.insert(Slice(header_name::()), Typed(box value as Box
)); } /// Get a clone of the header field's value, if it exists. @@ -128,8 +140,11 @@ impl Headers { /// let raw_content_type = unsafe { headers.get_raw("content-type") }; /// ``` pub unsafe fn get_raw(&self, name: &'static str) -> Option<&[Vec]> { - self.data.find(&Slice(name)).map(|item| { - item.raw.as_slice() + self.data.find(&Slice(name)).and_then(|item| { + match *item { + Raw(ref raw) => Some(raw.as_slice()), + _ => None + } }) } @@ -137,28 +152,35 @@ impl Headers { pub fn get_ref(&mut self) -> Option<&H> { self.data.find_mut(&Slice(header_name::())).and_then(|item| { debug!("get_ref, name={}, val={}", header_name::(), item); - let expected_tid = TypeId::of::(); - let header = match item.tid { - Some(tid) if tid == expected_tid => return Some(item), - _ => match Header::parse_header(item.raw.as_slice()) { + let header = match *item { + // Huge borrowck hack here, should be refactored to just return here. + Typed(ref typed) if typed.is::() => None, + // Typed, wrong type + Typed(_) => return None, + Raw(ref raw) => match Header::parse_header(raw.as_slice()) { Some::(h) => { - h + Some(h) }, None => return None }, }; - item.typed = Some(box header as Box
); - item.tid = Some(expected_tid); - Some(item) + + match header { + Some(header) => { + *item = Typed(box header as Box
); + Some(item) + }, + None => { + Some(item) + } + } }).and_then(|item| { debug!("downcasting {}", item); - let ret = match item.typed { - Some(ref val) => { - unsafe { - Some(val.downcast_ref_unchecked()) - } + let ret = match *item { + Typed(ref val) => { + unsafe { Some(val.downcast_ref_unchecked()) } }, - None => unreachable!() + _ => unreachable!() }; ret }) @@ -181,7 +203,7 @@ impl Headers { /// Removes a header from the map, if one existed. /// Returns true if a header has been removed. pub fn remove(&mut self) -> bool { - self.data.pop_equiv(&Header::header_name(None::)).is_some() + self.data.remove(&Slice(Header::header_name(None::))) } /// Returns an iterator over the header fields. @@ -238,26 +260,17 @@ impl Mutable for Headers { } } -struct Item { - raw: Vec>, - tid: Option, - typed: Option>, -} - -fn raw(parts: Vec>) -> Item { - Item { - raw: parts, - tid: None, - typed: None, - } +enum Item { + Raw(Vec>), + Typed(Box
) } impl fmt::Show for Item { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - match self.typed { - Some(ref h) => h.fmt_header(fmt), - None => { - for part in self.raw.iter() { + match *self { + Typed(ref h) => h.fmt_header(fmt), + Raw(ref raw) => { + for part in raw.iter() { try!(fmt.write(part.as_slice())); } Ok(()) From 7065a3f946a7b1963cf4c97766fbba64c25a7bd1 Mon Sep 17 00:00:00 2001 From: Jonathan Reem Date: Tue, 16 Sep 2014 16:31:05 -0700 Subject: [PATCH 3/3] Disallow parsing as multiple types to prevent transient errors. --- src/header/mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/header/mod.rs b/src/header/mod.rs index 5f2e026c..f1c45b6b 100644 --- a/src/header/mod.rs +++ b/src/header/mod.rs @@ -333,8 +333,7 @@ mod tests { #[test] fn test_different_structs_for_same_header() { let mut headers = Headers::from_raw(&mut mem("Content-Length: 10\r\n\r\n")).unwrap(); - let ContentLength(num) = headers.get::().unwrap(); - let CrazyLength(_, crazy_num) = headers.get::().unwrap(); - assert_eq!(num, crazy_num); + let ContentLength(_) = headers.get::().unwrap(); + assert!(headers.get::().is_none()); } }