Skip to content

Commit

Permalink
Add from_maybe_shared constructors
Browse files Browse the repository at this point in the history
These constructors don't expose the internal `Bytes` type, but instead
will try to downcast the argument to prevent a copy. If the types don't
match up (a user provides an older version of `Bytes`), the value will
just be copied.

Adds:

- `HeaderValue::from_maybe_shared`
- `HeaderValue::from_maybe_shared_unchecked`
- `Uri::from_maybe_shared`
- `Authority::from_maybe_shared`
- `PathAndQuery::from_maybe_shared`
  • Loading branch information
seanmonstar committed Dec 2, 2019
1 parent efa7fe0 commit 566878e
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 117 deletions.
8 changes: 4 additions & 4 deletions benches/header_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn from_shared_short(b: &mut Bencher) {
b.bytes = SHORT.len() as u64;
let bytes = Bytes::from_static(SHORT);
b.iter(|| {
HeaderValue::from_shared(bytes.clone()).unwrap();
HeaderValue::from_maybe_shared(bytes.clone()).unwrap();
});
}

Expand All @@ -23,7 +23,7 @@ fn from_shared_long(b: &mut Bencher) {
b.bytes = LONG.len() as u64;
let bytes = Bytes::from_static(LONG);
b.iter(|| {
HeaderValue::from_shared(bytes.clone()).unwrap();
HeaderValue::from_maybe_shared(bytes.clone()).unwrap();
});
}

Expand All @@ -32,7 +32,7 @@ fn from_shared_unchecked_short(b: &mut Bencher) {
b.bytes = SHORT.len() as u64;
let bytes = Bytes::from_static(SHORT);
b.iter(|| unsafe {
HeaderValue::from_shared_unchecked(bytes.clone());
HeaderValue::from_maybe_shared_unchecked(bytes.clone());
});
}

Expand All @@ -41,6 +41,6 @@ fn from_shared_unchecked_long(b: &mut Bencher) {
b.bytes = LONG.len() as u64;
let bytes = Bytes::from_static(LONG);
b.iter(|| unsafe {
HeaderValue::from_shared_unchecked(bytes.clone());
HeaderValue::from_maybe_shared_unchecked(bytes.clone());
});
}
17 changes: 17 additions & 0 deletions src/convert.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
macro_rules! if_downcast_into {
($in_ty:ty, $out_ty:ty, $val:ident, $body:expr) => ({
if std::any::TypeId::of::<$in_ty>() == std::any::TypeId::of::<$out_ty>() {
// Store the value in an `Option` so we can `take`
// it after casting to `&mut dyn Any`.
let mut slot = Some($val);
// Re-write the `$val` ident with the downcasted value.
let $val = (&mut slot as &mut dyn std::any::Any)
.downcast_mut::<Option<$out_ty>>()
.unwrap()
.take()
.unwrap();
// Run the $body in scope of the replaced val.
$body
}
})
}
45 changes: 30 additions & 15 deletions src/header/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,39 +149,54 @@ impl HeaderValue {

/// Attempt to convert a `Bytes` buffer to a `HeaderValue`.
///
/// If the argument contains invalid header value bytes, an error is
/// returned. Only byte values between 32 and 255 (inclusive) are permitted,
/// excluding byte 127 (DEL).
///
/// This function is intended to be replaced in the future by a `TryFrom`
/// implementation once the trait is stabilized in std.
#[inline]
fn from_shared(src: Bytes) -> Result<HeaderValue, InvalidHeaderValue> {
HeaderValue::try_from_generic(src, std::convert::identity)
/// This will try to prevent a copy if the type passed is the type used
/// internally, and will copy the data if it is not.
pub fn from_maybe_shared<T>(src: T) -> Result<HeaderValue, InvalidHeaderValue>
where
T: AsRef<[u8]> + 'static,
{
if_downcast_into!(T, Bytes, src, {
return HeaderValue::from_shared(src);
});

HeaderValue::from_bytes(src.as_ref())
}

/*
/// Convert a `Bytes` directly into a `HeaderValue` without validating.
///
/// This function does NOT validate that illegal bytes are not contained
/// within the buffer.
#[inline]
pub unsafe fn from_shared_unchecked(src: Bytes) -> HeaderValue {
pub unsafe fn from_maybe_shared_unchecked<T>(src: T) -> HeaderValue
where
T: AsRef<[u8]> + 'static,
{
if cfg!(debug_assertions) {
match HeaderValue::from_shared(src) {
match HeaderValue::from_maybe_shared(src) {
Ok(val) => val,
Err(_err) => {
panic!("HeaderValue::from_shared_unchecked() with invalid bytes");
panic!("HeaderValue::from_maybe_shared_unchecked() with invalid bytes");
}
}
} else {

if_downcast_into!(T, Bytes, src, {
return HeaderValue {
inner: src,
is_sensitive: false,
};
});

let src = Bytes::copy_from_slice(src.as_ref());
HeaderValue {
inner: src,
is_sensitive: false,
}
}
}
*/

fn from_shared(src: Bytes) -> Result<HeaderValue, InvalidHeaderValue> {
HeaderValue::try_from_generic(src, std::convert::identity)
}

fn try_from_generic<T: AsRef<[u8]>, F: FnOnce(T) -> Bytes>(src: T, into: F) -> Result<HeaderValue, InvalidHeaderValue> {
for &b in src.as_ref() {
Expand Down
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ extern crate doc_comment;
#[cfg(test)]
doctest!("../README.md");

#[macro_use]
mod convert;

pub mod header;
pub mod method;
pub mod request;
Expand Down
74 changes: 19 additions & 55 deletions src/uri/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,26 +21,7 @@ impl Authority {
}
}

/// Attempt to convert an `Authority` from `Bytes`.
///
/// This function has been replaced by `TryFrom` implementation.
///
/// # Examples
///
/// ```
/// # extern crate http;
/// # use http::uri::*;
/// extern crate bytes;
///
/// use bytes::Bytes;
///
/// # pub fn main() {
/// let bytes = Bytes::from("example.com");
/// let authority = Authority::from_shared(bytes).unwrap();
///
/// assert_eq!(authority.host(), "example.com");
/// # }
/// ```
// Not public while `bytes` is unstable.
pub(super) fn from_shared(s: Bytes) -> Result<Self, InvalidUri> {
let authority_end = Authority::parse_non_empty(&s[..])?;

Expand Down Expand Up @@ -85,6 +66,22 @@ impl Authority {
}
}


/// Attempt to convert a `Bytes` buffer to a `Authority`.
///
/// This will try to prevent a copy if the type passed is the type used
/// internally, and will copy the data if it is not.
pub fn from_maybe_shared<T>(src: T) -> Result<Self, InvalidUri>
where
T: AsRef<[u8]> + 'static,
{
if_downcast_into!(T, Bytes, src, {
return Authority::from_shared(src);
});

Authority::try_from(src.as_ref())
}

// Note: this may return an *empty* Authority. You might want `parse_non_empty`.
pub(super) fn parse(s: &[u8]) -> Result<usize, InvalidUri> {
let mut colon_cnt = 0;
Expand Down Expand Up @@ -267,41 +264,8 @@ impl Authority {
}
}

/*
impl TryFrom<Bytes> for Authority {
type Error = InvalidUri;
/// Attempt to convert an `Authority` from `Bytes`.
///
/// # Examples
///
/// ```
/// # extern crate http;
/// # use http::uri::*;
/// extern crate bytes;
///
/// use std::convert::TryFrom;
/// use bytes::Bytes;
///
/// # pub fn main() {
/// let bytes = Bytes::from("example.com");
/// let authority = Authority::try_from(bytes).unwrap();
///
/// assert_eq!(authority.host(), "example.com");
/// # }
/// ```
fn try_from(s: Bytes) -> Result<Self, Self::Error> {
let authority_end = Authority::parse_non_empty(&s[..])?;
if authority_end != s.len() {
return Err(ErrorKind::InvalidUriChar.into());
}
Ok(Authority {
data: unsafe { ByteStr::from_utf8_unchecked(s) },
})
}
}
*/
// Purposefully not public while `bytes` is unstable.
// impl TryFrom<Bytes> for Authority

impl AsRef<str> for Authority {
fn as_ref(&self) -> &str {
Expand Down
46 changes: 25 additions & 21 deletions src/uri/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,27 +235,22 @@ impl Uri {
})
}

/// Attempt to convert a `Uri` from `Bytes`
/// Attempt to convert a `Bytes` buffer to a `Uri`.
///
/// This function has been replaced by `TryFrom` implementation.
///
/// # Examples
///
/// ```
/// # extern crate http;
/// # use http::uri::*;
/// extern crate bytes;
///
/// use bytes::Bytes;
///
/// # pub fn main() {
/// let bytes = Bytes::from("http://example.com/foo");
/// let uri = Uri::from_shared(bytes).unwrap();
///
/// assert_eq!(uri.host().unwrap(), "example.com");
/// assert_eq!(uri.path(), "/foo");
/// # }
/// ```
/// This will try to prevent a copy if the type passed is the type used
/// internally, and will copy the data if it is not.
pub fn from_maybe_shared<T>(src: T) -> Result<Self, InvalidUri>
where
T: AsRef<[u8]> + 'static,
{
if_downcast_into!(T, Bytes, src, {
return Uri::from_shared(src);
});

Uri::try_from(src.as_ref())
}

// Not public while `bytes` is unstable.
fn from_shared(s: Bytes) -> Result<Uri, InvalidUri> {
use self::ErrorKind::*;

Expand Down Expand Up @@ -674,6 +669,15 @@ impl Uri {
}
}

impl<'a> TryFrom<&'a [u8]> for Uri {
type Error = InvalidUri;

#[inline]
fn try_from(t: &'a [u8]) -> Result<Self, Self::Error> {
Uri::from_shared(Bytes::copy_from_slice(t))
}
}

impl<'a> TryFrom<&'a str> for Uri {
type Error = InvalidUri;

Expand Down Expand Up @@ -846,7 +850,7 @@ impl FromStr for Uri {

#[inline]
fn from_str(s: &str) -> Result<Uri, InvalidUri> {
Uri::from_shared(Bytes::copy_from_slice(s.as_bytes()))
Uri::try_from(s.as_bytes())
}
}

Expand Down
38 changes: 16 additions & 22 deletions src/uri/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,7 @@ pub struct PathAndQuery {
const NONE: u16 = ::std::u16::MAX;

impl PathAndQuery {
/// Attempt to convert a `PathAndQuery` from `Bytes`.
///
/// This function will be replaced by a `TryFrom` implementation once the
/// trait lands in stable.
///
/// # Examples
///
/// ```
/// # extern crate http;
/// # use http::uri::*;
/// extern crate bytes;
///
/// use bytes::Bytes;
///
/// # pub fn main() {
/// let bytes = Bytes::from("/hello?world");
/// let path_and_query = PathAndQuery::from_shared(bytes).unwrap();
///
/// assert_eq!(path_and_query.path(), "/hello");
/// assert_eq!(path_and_query.query(), Some("world"));
/// # }
/// ```
// Not public while `bytes` is unstable.
pub(super) fn from_shared(mut src: Bytes) -> Result<Self, InvalidUri> {
let mut query = NONE;
let mut fragment = None;
Expand Down Expand Up @@ -144,6 +123,21 @@ impl PathAndQuery {
PathAndQuery::from_shared(src).unwrap()
}

/// Attempt to convert a `Bytes` buffer to a `PathAndQuery`.
///
/// This will try to prevent a copy if the type passed is the type used
/// internally, and will copy the data if it is not.
pub fn from_maybe_shared<T>(src: T) -> Result<Self, InvalidUri>
where
T: AsRef<[u8]> + 'static,
{
if_downcast_into!(T, Bytes, src, {
return PathAndQuery::from_shared(src);
});

PathAndQuery::try_from(src.as_ref())
}

pub(super) fn empty() -> Self {
PathAndQuery {
data: ByteStr::new(),
Expand Down

0 comments on commit 566878e

Please sign in to comment.