Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: bring back TryFrom<Bytes> implementations #470

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions benches/src/header_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

extern crate test;

use std::convert::TryFrom;

use bytes::Bytes;
use http::HeaderValue;
use test::Bencher;
Expand All @@ -14,7 +16,7 @@ fn from_shared_short(b: &mut Bencher) {
b.bytes = SHORT.len() as u64;
let bytes = Bytes::from_static(SHORT);
b.iter(|| {
HeaderValue::from_maybe_shared(bytes.clone()).unwrap();
HeaderValue::try_from(bytes.clone()).unwrap();
});
}

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

Expand All @@ -32,7 +34,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_maybe_shared_unchecked(bytes.clone());
HeaderValue::from_shared_unchecked(bytes.clone());
});
}

Expand All @@ -41,6 +43,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_maybe_shared_unchecked(bytes.clone());
HeaderValue::from_shared_unchecked(bytes.clone());
});
}
23 changes: 17 additions & 6 deletions src/header/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,14 @@ macro_rules! standard_headers {
let std = HeaderName::from(std);
// Test lower case
let bytes: Bytes =
HeaderName::from_bytes(name_bytes).unwrap().inner.into();
HeaderName::from_bytes(name_bytes).unwrap().into();
assert_eq!(bytes, name);
assert_eq!(HeaderName::from_bytes(name_bytes).unwrap(), std);

// Test upper case
let upper = name.to_uppercase();
let bytes: Bytes =
HeaderName::from_bytes(upper.as_bytes()).unwrap().inner.into();
HeaderName::from_bytes(upper.as_bytes()).unwrap().into();
assert_eq!(bytes, name_bytes);
assert_eq!(HeaderName::from_bytes(upper.as_bytes()).unwrap(),
std);
Expand Down Expand Up @@ -1296,10 +1296,6 @@ impl HeaderName {
Repr::Custom(ref v) => &v.0,
}
}

pub(super) fn into_bytes(self) -> Bytes {
self.inner.into()
}
}

impl FromStr for HeaderName {
Expand Down Expand Up @@ -1372,6 +1368,13 @@ impl From<Custom> for Bytes {
}
}

impl From<HeaderName> for Bytes {
#[inline]
fn from(name: HeaderName) -> Bytes {
name.inner.into()
}
}

impl<'a> TryFrom<&'a str> for HeaderName {
type Error = InvalidHeaderName;
#[inline]
Expand Down Expand Up @@ -1414,6 +1417,14 @@ impl TryFrom<Vec<u8>> for HeaderName {
}
}

impl TryFrom<Bytes> for HeaderName {
type Error = InvalidHeaderName;
#[inline]
fn try_from(bytes: Bytes) -> Result<Self, Self::Error> {
Self::from_bytes(bytes.as_ref())
}
}

#[doc(hidden)]
impl From<StandardHeader> for HeaderName {
fn from(src: StandardHeader) -> HeaderName {
Expand Down
63 changes: 38 additions & 25 deletions src/header/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,16 +180,14 @@ impl HeaderValue {
HeaderValue::try_from_generic(src, Bytes::copy_from_slice)
}

/// Attempt to convert a `Bytes` buffer to a `HeaderValue`.
///
/// 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.
#[deprecated = "Use TryFrom::<Bytes>::try_from instead"]
#[doc(hidden)]
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);
return HeaderValue::try_from(src);
});

HeaderValue::from_bytes(src.as_ref())
Expand All @@ -206,35 +204,34 @@ impl HeaderValue {
/// ## Safety
/// `src` must contain valid UTF-8. In a release build it is undefined
/// behaviour to call this with `src` that is not valid UTF-8.
pub unsafe fn from_maybe_shared_unchecked<T>(src: T) -> HeaderValue
where
T: AsRef<[u8]> + 'static,
{
pub unsafe fn from_shared_unchecked(bytes: Bytes) -> HeaderValue {
if cfg!(debug_assertions) {
match HeaderValue::from_maybe_shared(src) {
match HeaderValue::try_from(bytes) {
Ok(val) => val,
Err(_err) => {
panic!("HeaderValue::from_maybe_shared_unchecked() with invalid bytes");
panic!("HeaderValue::from_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,
inner: bytes,
is_sensitive: false,
}
}
}

fn from_shared(src: Bytes) -> Result<HeaderValue, InvalidHeaderValue> {
HeaderValue::try_from_generic(src, std::convert::identity)
#[deprecated = "Use from_shared_unchecked instead"]
#[doc(hidden)]
pub unsafe fn from_maybe_shared_unchecked<T>(src: T) -> HeaderValue
where
T: AsRef<[u8]> + 'static,
{
if_downcast_into!(T, Bytes, src, {
return HeaderValue::from_shared_unchecked(src);
});

let src = Bytes::copy_from_slice(src.as_ref());
HeaderValue::from_shared_unchecked(src)
}

fn try_from_generic<T: AsRef<[u8]>, F: FnOnce(T) -> Bytes>(
Expand Down Expand Up @@ -414,7 +411,7 @@ impl From<HeaderName> for HeaderValue {
#[inline]
fn from(h: HeaderName) -> HeaderValue {
HeaderValue {
inner: h.into_bytes(),
inner: h.into(),
is_sensitive: false,
}
}
Expand Down Expand Up @@ -532,6 +529,13 @@ impl FromStr for HeaderValue {
}
}

impl From<HeaderValue> for Bytes {
#[inline]
fn from(value: HeaderValue) -> Bytes {
value.inner
}
}

impl<'a> From<&'a HeaderValue> for HeaderValue {
#[inline]
fn from(t: &'a HeaderValue) -> Self {
Expand Down Expand Up @@ -570,7 +574,7 @@ impl TryFrom<String> for HeaderValue {

#[inline]
fn try_from(t: String) -> Result<Self, Self::Error> {
HeaderValue::from_shared(t.into())
HeaderValue::try_from(Bytes::from(t))
}
}

Expand All @@ -579,7 +583,16 @@ impl TryFrom<Vec<u8>> for HeaderValue {

#[inline]
fn try_from(vec: Vec<u8>) -> Result<Self, Self::Error> {
HeaderValue::from_shared(vec.into())
HeaderValue::try_from(Bytes::from(vec))
}
}

impl TryFrom<Bytes> for HeaderValue {
type Error = InvalidHeaderValue;

#[inline]
fn try_from(bytes: Bytes) -> Result<Self, Self::Error> {
HeaderValue::try_from_generic(bytes, std::convert::identity)
}
}

Expand Down
59 changes: 41 additions & 18 deletions src/uri/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,6 @@ impl Authority {
}
}

// Not public while `bytes` is unstable.
pub(super) fn from_shared(s: Bytes) -> Result<Self, InvalidUri> {
// Precondition on create_authority: trivially satisfied by the
// identity closure
create_authority(s, |s| s)
}

/// Attempt to convert an `Authority` from a static string.
///
/// This function will not perform any copying, and the string will be
Expand All @@ -46,20 +39,18 @@ impl Authority {
/// assert_eq!(authority.host(), "example.com");
/// ```
pub fn from_static(src: &'static str) -> Self {
Authority::from_shared(Bytes::from_static(src.as_bytes()))
Authority::try_from(Bytes::from_static(src.as_bytes()))
.expect("static str is not valid 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.
#[deprecated = "Use TryFrom::<Bytes>::try_from instead"]
#[doc(hidden)]
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);
return Authority::try_from(src);
});

Authority::try_from(src.as_ref())
Expand Down Expand Up @@ -263,8 +254,33 @@ impl Authority {
}
}

// Purposefully not public while `bytes` is unstable.
// impl TryFrom<Bytes> for 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(bytes: Bytes) -> Result<Self, Self::Error> {
// Precondition on create_authority: trivially satisfied by the
// identity closure
create_authority(bytes, |s| s)
}
}

impl AsRef<str> for Authority {
fn as_ref(&self) -> &str {
Expand Down Expand Up @@ -451,7 +467,7 @@ impl TryFrom<Vec<u8>> for Authority {

#[inline]
fn try_from(vec: Vec<u8>) -> Result<Self, Self::Error> {
Authority::from_shared(vec.into())
Authority::try_from(Bytes::from(vec))
}
}

Expand All @@ -460,7 +476,7 @@ impl TryFrom<String> for Authority {

#[inline]
fn try_from(t: String) -> Result<Self, Self::Error> {
Authority::from_shared(t.into())
Authority::try_from(Bytes::from(t))
}
}

Expand All @@ -472,6 +488,13 @@ impl FromStr for Authority {
}
}

impl From<Authority> for Bytes {
#[inline]
fn from(src: Authority) -> Bytes {
src.data.into()
}
}

impl fmt::Debug for Authority {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str(self.as_str())
Expand Down Expand Up @@ -668,7 +691,7 @@ mod tests {
let err = Authority::try_from([0xc0u8].as_ref()).unwrap_err();
assert_eq!(err.0, ErrorKind::InvalidUriChar);

let err = Authority::from_shared(Bytes::from_static([0xc0u8].as_ref())).unwrap_err();
let err = Authority::try_from(Bytes::from_static([0xc0u8].as_ref())).unwrap_err();
assert_eq!(err.0, ErrorKind::InvalidUriChar);
}

Expand Down
Loading