Skip to content

Commit

Permalink
feat: bring back TryFrom<Bytes> implementations
Browse files Browse the repository at this point in the history
This is mostly a revert of commit
4ce5e6a, but this does not bring back
the error variants suffixed with `Bytes`
(db9b1b9).

This also adds `HeaderValue::from_shared_unchecked` constructor, which
corresponds to the `from_maybe_shared_unchecked` constructor, and
replaces usage of the internal `from_shared` associated functions with
`try_from`.

Closes <#459>.
  • Loading branch information
tesaguri committed Oct 26, 2024
1 parent 761d36a commit 88163ad
Show file tree
Hide file tree
Showing 7 changed files with 215 additions and 54 deletions.
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
66 changes: 47 additions & 19 deletions src/header/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ impl HeaderValue {
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,33 +206,45 @@ 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,
}
}
}

/// Convert a `Bytes` directly into a `HeaderValue` without validating.
///
/// This function does NOT validate that illegal bytes are not contained
/// within the buffer.
///
/// ## Panics
/// In a debug build this will panic if `src` is not valid UTF-8.
///
/// ## 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,
{
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 from_shared(src: Bytes) -> Result<HeaderValue, InvalidHeaderValue> {
HeaderValue::try_from_generic(src, std::convert::identity)
}
Expand Down Expand Up @@ -414,7 +426,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 +544,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 +589,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 +598,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::from_shared(bytes)
}
}

Expand Down
47 changes: 38 additions & 9 deletions src/uri/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ impl Authority {
}
}

// Not public while `bytes` is unstable.
pub(super) fn from_shared(s: Bytes) -> Result<Self, InvalidUri> {
fn from_shared(s: Bytes) -> Result<Self, InvalidUri> {
// Precondition on create_authority: trivially satisfied by the
// identity closure
create_authority(s, |s| s)
Expand All @@ -46,7 +45,7 @@ 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")
}

Expand All @@ -59,7 +58,7 @@ impl Authority {
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 +262,31 @@ 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> {
Authority::from_shared(bytes)
}
}

impl AsRef<str> for Authority {
fn as_ref(&self) -> &str {
Expand Down Expand Up @@ -451,7 +473,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 +482,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 +494,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 +697,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

0 comments on commit 88163ad

Please sign in to comment.