Skip to content

Commit

Permalink
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 replaces usage of the internal `from_shared` associated
functions with `try_from`.

Reference: <#459>
  • Loading branch information
tesaguri committed Mar 9, 2021
1 parent 5b90e38 commit 000c9dd
Show file tree
Hide file tree
Showing 7 changed files with 290 additions and 153 deletions.
6 changes: 4 additions & 2 deletions benches/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 Down
23 changes: 17 additions & 6 deletions src/header/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,14 @@ macro_rules! standard_headers {
// Test lower case
let name_bytes = name.as_bytes();
let bytes: Bytes =
HeaderName::from_bytes(name_bytes).unwrap().inner.into();
HeaderName::from_bytes(name_bytes).unwrap().into();
assert_eq!(bytes, name_bytes);
assert_eq!(HeaderName::from_bytes(name_bytes).unwrap(), std);

// Test upper case
let upper = name.to_uppercase().to_string();
let bytes: Bytes =
HeaderName::from_bytes(upper.as_bytes()).unwrap().inner.into();
HeaderName::from_bytes(upper.as_bytes()).unwrap().into();
assert_eq!(bytes, name.as_bytes());
assert_eq!(HeaderName::from_bytes(upper.as_bytes()).unwrap(),
std);
Expand Down Expand Up @@ -1800,10 +1800,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 @@ -1876,6 +1872,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 All @@ -1900,6 +1903,14 @@ impl<'a> TryFrom<&'a [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
18 changes: 17 additions & 1 deletion src/header/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,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 @@ -490,6 +490,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 @@ -541,6 +548,15 @@ impl TryFrom<Vec<u8>> for HeaderValue {
}
}

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

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

#[cfg(test)]
mod try_from_header_name_tests {
use super::*;
Expand Down
49 changes: 37 additions & 12 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 clousre
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,7 +39,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 +52,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 @@ -259,8 +252,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(s: Bytes) -> Result<Self, Self::Error> {
// Precondition on create_authority: trivially satisfied by the
// identity clousre
create_authority(s, |s| s)
}
}

impl AsRef<str> for Authority {
fn as_ref(&self) -> &str {
Expand Down Expand Up @@ -450,6 +468,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 @@ -640,7 +665,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()))
let err = Authority::try_from(Bytes::from_static([0xc0u8].as_ref()))
.unwrap_err();
assert_eq!(err.0, ErrorKind::InvalidUriChar);
}
Expand Down
135 changes: 79 additions & 56 deletions src/uri/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,63 +250,12 @@ impl Uri {
T: AsRef<[u8]> + 'static,
{
if_downcast_into!(T, Bytes, src, {
return Uri::from_shared(src);
return Uri::try_from(src);
});

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

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

if s.len() > MAX_LEN {
return Err(TooLong.into());
}

match s.len() {
0 => {
return Err(Empty.into());
}
1 => match s[0] {
b'/' => {
return Ok(Uri {
scheme: Scheme::empty(),
authority: Authority::empty(),
path_and_query: PathAndQuery::slash(),
});
}
b'*' => {
return Ok(Uri {
scheme: Scheme::empty(),
authority: Authority::empty(),
path_and_query: PathAndQuery::star(),
});
}
_ => {
let authority = Authority::from_shared(s)?;

return Ok(Uri {
scheme: Scheme::empty(),
authority: authority,
path_and_query: PathAndQuery::empty(),
});
}
},
_ => {}
}

if s[0] == b'/' {
return Ok(Uri {
scheme: Scheme::empty(),
authority: Authority::empty(),
path_and_query: PathAndQuery::from_shared(s)?,
});
}

parse_full(s)
}

/// Convert a `Uri` from a static string.
///
/// This function will not perform any copying, however the string is
Expand All @@ -327,7 +276,7 @@ impl Uri {
/// ```
pub fn from_static(src: &'static str) -> Self {
let s = Bytes::from_static(src.as_bytes());
match Uri::from_shared(s) {
match Uri::try_from(s) {
Ok(uri) => uri,
Err(e) => panic!("static str is not valid URI: {}", e),
}
Expand Down Expand Up @@ -675,12 +624,86 @@ impl Uri {
}
}

impl TryFrom<Bytes> for Uri {
type Error = InvalidUri;

/// Attempt to convert a `Uri` 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("http://example.com/foo");
/// let uri = Uri::try_from(bytes).unwrap();
///
/// assert_eq!(uri.host().unwrap(), "example.com");
/// assert_eq!(uri.path(), "/foo");
/// # }
/// ```
fn try_from(s: Bytes) -> Result<Uri, Self::Error> {
use self::ErrorKind::*;

if s.len() > MAX_LEN {
return Err(TooLong.into());
}

match s.len() {
0 => {
return Err(Empty.into());
}
1 => match s[0] {
b'/' => {
return Ok(Uri {
scheme: Scheme::empty(),
authority: Authority::empty(),
path_and_query: PathAndQuery::slash(),
});
}
b'*' => {
return Ok(Uri {
scheme: Scheme::empty(),
authority: Authority::empty(),
path_and_query: PathAndQuery::star(),
});
}
_ => {
let authority = Authority::try_from(s)?;

return Ok(Uri {
scheme: Scheme::empty(),
authority: authority,
path_and_query: PathAndQuery::empty(),
});
}
},
_ => {}
}

if s[0] == b'/' {
return Ok(Uri {
scheme: Scheme::empty(),
authority: Authority::empty(),
path_and_query: PathAndQuery::try_from(s)?,
});
}

parse_full(s)
}
}

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))
Uri::try_from(Bytes::copy_from_slice(t))
}
}

Expand All @@ -707,7 +730,7 @@ impl TryFrom<String> for Uri {

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

Expand Down Expand Up @@ -847,7 +870,7 @@ fn parse_full(mut s: Bytes) -> Result<Uri, InvalidUri> {
Ok(Uri {
scheme: scheme.into(),
authority: authority,
path_and_query: PathAndQuery::from_shared(s)?,
path_and_query: PathAndQuery::try_from(s)?,
})
}

Expand Down
Loading

0 comments on commit 000c9dd

Please sign in to comment.