From 88163ad6a56a26b5ef05753a0f409bf3dca2fc51 Mon Sep 17 00:00:00 2001 From: Daiki Mizukami Date: Tue, 9 Mar 2021 16:14:13 +0900 Subject: [PATCH] feat: bring back `TryFrom` implementations This is mostly a revert of commit 4ce5e6a3a33c6548f885972db6f23450be18e133, but this does not bring back the error variants suffixed with `Bytes` (db9b1b9a76ce9c0b366fc6fff433ca4ca00571d1). 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 . --- benches/src/header_value.rs | 10 +++--- src/header/name.rs | 23 +++++++++---- src/header/value.rs | 66 ++++++++++++++++++++++++++----------- src/uri/authority.rs | 47 +++++++++++++++++++++----- src/uri/mod.rs | 45 ++++++++++++++++++++----- src/uri/path.rs | 27 +++++++++++---- src/uri/scheme.rs | 51 ++++++++++++++++++++++++++++ 7 files changed, 215 insertions(+), 54 deletions(-) diff --git a/benches/src/header_value.rs b/benches/src/header_value.rs index 2e1004e9..e68c6d7e 100644 --- a/benches/src/header_value.rs +++ b/benches/src/header_value.rs @@ -2,6 +2,8 @@ extern crate test; +use std::convert::TryFrom; + use bytes::Bytes; use http::HeaderValue; use test::Bencher; @@ -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(); }); } @@ -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(); }); } @@ -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()); }); } @@ -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()); }); } diff --git a/src/header/name.rs b/src/header/name.rs index 3d563f4e..1cbe8bfc 100644 --- a/src/header/name.rs +++ b/src/header/name.rs @@ -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); @@ -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 { @@ -1372,6 +1368,13 @@ impl From for Bytes { } } +impl From for Bytes { + #[inline] + fn from(name: HeaderName) -> Bytes { + name.inner.into() + } +} + impl<'a> TryFrom<&'a str> for HeaderName { type Error = InvalidHeaderName; #[inline] @@ -1414,6 +1417,14 @@ impl TryFrom> for HeaderName { } } +impl TryFrom for HeaderName { + type Error = InvalidHeaderName; + #[inline] + fn try_from(bytes: Bytes) -> Result { + Self::from_bytes(bytes.as_ref()) + } +} + #[doc(hidden)] impl From for HeaderName { fn from(src: StandardHeader) -> HeaderName { diff --git a/src/header/value.rs b/src/header/value.rs index 4813f6fd..a749c5e4 100644 --- a/src/header/value.rs +++ b/src/header/value.rs @@ -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()) @@ -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(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(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::try_from_generic(src, std::convert::identity) } @@ -414,7 +426,7 @@ impl From for HeaderValue { #[inline] fn from(h: HeaderName) -> HeaderValue { HeaderValue { - inner: h.into_bytes(), + inner: h.into(), is_sensitive: false, } } @@ -532,6 +544,13 @@ impl FromStr for HeaderValue { } } +impl From for Bytes { + #[inline] + fn from(value: HeaderValue) -> Bytes { + value.inner + } +} + impl<'a> From<&'a HeaderValue> for HeaderValue { #[inline] fn from(t: &'a HeaderValue) -> Self { @@ -570,7 +589,7 @@ impl TryFrom for HeaderValue { #[inline] fn try_from(t: String) -> Result { - HeaderValue::from_shared(t.into()) + HeaderValue::try_from(Bytes::from(t)) } } @@ -579,7 +598,16 @@ impl TryFrom> for HeaderValue { #[inline] fn try_from(vec: Vec) -> Result { - HeaderValue::from_shared(vec.into()) + HeaderValue::try_from(Bytes::from(vec)) + } +} + +impl TryFrom for HeaderValue { + type Error = InvalidHeaderValue; + + #[inline] + fn try_from(bytes: Bytes) -> Result { + HeaderValue::from_shared(bytes) } } diff --git a/src/uri/authority.rs b/src/uri/authority.rs index 07aa6795..6b29e757 100644 --- a/src/uri/authority.rs +++ b/src/uri/authority.rs @@ -21,8 +21,7 @@ impl Authority { } } - // Not public while `bytes` is unstable. - pub(super) fn from_shared(s: Bytes) -> Result { + fn from_shared(s: Bytes) -> Result { // Precondition on create_authority: trivially satisfied by the // identity closure create_authority(s, |s| s) @@ -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") } @@ -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()) @@ -263,8 +262,31 @@ impl Authority { } } -// Purposefully not public while `bytes` is unstable. -// impl TryFrom for Authority +impl TryFrom 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 { + Authority::from_shared(bytes) + } +} impl AsRef for Authority { fn as_ref(&self) -> &str { @@ -451,7 +473,7 @@ impl TryFrom> for Authority { #[inline] fn try_from(vec: Vec) -> Result { - Authority::from_shared(vec.into()) + Authority::try_from(Bytes::from(vec)) } } @@ -460,7 +482,7 @@ impl TryFrom for Authority { #[inline] fn try_from(t: String) -> Result { - Authority::from_shared(t.into()) + Authority::try_from(Bytes::from(t)) } } @@ -472,6 +494,13 @@ impl FromStr for Authority { } } +impl From 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()) @@ -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); } diff --git a/src/uri/mod.rs b/src/uri/mod.rs index 767f0743..d8f51fb1 100644 --- a/src/uri/mod.rs +++ b/src/uri/mod.rs @@ -282,13 +282,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 { use self::ErrorKind::*; @@ -316,7 +315,7 @@ impl Uri { }); } _ => { - let authority = Authority::from_shared(s)?; + let authority = Authority::try_from(s)?; return Ok(Uri { scheme: Scheme::empty(), @@ -332,7 +331,7 @@ impl Uri { return Ok(Uri { scheme: Scheme::empty(), authority: Authority::empty(), - path_and_query: PathAndQuery::from_shared(s)?, + path_and_query: PathAndQuery::try_from(s)?, }); } @@ -359,7 +358,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), } @@ -705,12 +704,40 @@ impl Uri { } } +impl TryFrom 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(t: Bytes) -> Result { + Uri::from_shared(t) + } +} + impl<'a> TryFrom<&'a [u8]> for Uri { type Error = InvalidUri; #[inline] fn try_from(t: &'a [u8]) -> Result { - Uri::from_shared(Bytes::copy_from_slice(t)) + Uri::try_from(Bytes::copy_from_slice(t)) } } @@ -737,7 +764,7 @@ impl TryFrom for Uri { #[inline] fn try_from(t: String) -> Result { - Uri::from_shared(Bytes::from(t)) + Uri::try_from(Bytes::from(t)) } } @@ -746,7 +773,7 @@ impl TryFrom> for Uri { #[inline] fn try_from(vec: Vec) -> Result { - Uri::from_shared(Bytes::from(vec)) + Uri::try_from(Bytes::from(vec)) } } @@ -875,7 +902,7 @@ fn parse_full(mut s: Bytes) -> Result { Ok(Uri { scheme: scheme.into(), authority, - path_and_query: PathAndQuery::from_shared(s)?, + path_and_query: PathAndQuery::try_from(s)?, }) } diff --git a/src/uri/path.rs b/src/uri/path.rs index 6a2f1e1e..f33d334c 100644 --- a/src/uri/path.rs +++ b/src/uri/path.rs @@ -17,8 +17,7 @@ pub struct PathAndQuery { const NONE: u16 = u16::MAX; impl PathAndQuery { - // Not public while `bytes` is unstable. - pub(super) fn from_shared(mut src: Bytes) -> Result { + fn from_shared(mut src: Bytes) -> Result { let mut query = NONE; let mut fragment = None; @@ -127,7 +126,7 @@ impl PathAndQuery { pub fn from_static(src: &'static str) -> Self { let src = Bytes::from_static(src.as_bytes()); - PathAndQuery::from_shared(src).unwrap() + PathAndQuery::try_from(src).unwrap() } /// Attempt to convert a `Bytes` buffer to a `PathAndQuery`. @@ -139,7 +138,7 @@ impl PathAndQuery { T: AsRef<[u8]> + 'static, { if_downcast_into!(T, Bytes, src, { - return PathAndQuery::from_shared(src); + return PathAndQuery::try_from(src); }); PathAndQuery::try_from(src.as_ref()) @@ -278,11 +277,19 @@ impl PathAndQuery { } } +impl TryFrom for PathAndQuery { + type Error = InvalidUri; + #[inline] + fn try_from(bytes: Bytes) -> Result { + PathAndQuery::from_shared(bytes) + } +} + impl<'a> TryFrom<&'a [u8]> for PathAndQuery { type Error = InvalidUri; #[inline] fn try_from(s: &'a [u8]) -> Result { - PathAndQuery::from_shared(Bytes::copy_from_slice(s)) + PathAndQuery::try_from(Bytes::copy_from_slice(s)) } } @@ -298,7 +305,7 @@ impl TryFrom> for PathAndQuery { type Error = InvalidUri; #[inline] fn try_from(vec: Vec) -> Result { - PathAndQuery::from_shared(vec.into()) + PathAndQuery::try_from(Bytes::from(vec)) } } @@ -306,7 +313,7 @@ impl TryFrom for PathAndQuery { type Error = InvalidUri; #[inline] fn try_from(s: String) -> Result { - PathAndQuery::from_shared(s.into()) + PathAndQuery::try_from(Bytes::from(s)) } } @@ -326,6 +333,12 @@ impl FromStr for PathAndQuery { } } +impl From for Bytes { + fn from(src: PathAndQuery) -> Bytes { + src.data.into() + } +} + impl fmt::Debug for PathAndQuery { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(self, f) diff --git a/src/uri/scheme.rs b/src/uri/scheme.rs index dbcc8c3f..3700fe9c 100644 --- a/src/uri/scheme.rs +++ b/src/uri/scheme.rs @@ -67,6 +67,42 @@ impl Scheme { } } +impl TryFrom for Scheme { + type Error = InvalidUri; + + /// Attempt to convert a `Scheme` 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"); + /// let scheme = Scheme::try_from(bytes).unwrap(); + /// + /// assert_eq!(scheme.as_str(), "http"); + /// # } + /// ``` + fn try_from(s: Bytes) -> Result { + use self::Scheme2::*; + + match Scheme2::parse_exact(&s[..])? { + None => Err(ErrorKind::InvalidScheme.into()), + Standard(p) => Ok(Standard(p).into()), + Other(_) => { + let b = unsafe { ByteStr::from_utf8_unchecked(s) }; + Ok(Other(Box::new(b)).into()) + } + } + } +} + impl<'a> TryFrom<&'a [u8]> for Scheme { type Error = InvalidUri; #[inline] @@ -105,6 +141,21 @@ impl FromStr for Scheme { } } +impl From for Bytes { + #[inline] + fn from(src: Scheme) -> Self { + use self::Protocol::*; + use self::Scheme2::*; + + match src.inner { + None => Bytes::new(), + Standard(Http) => Bytes::from_static(b"http"), + Standard(Https) => Bytes::from_static(b"https"), + Other(v) => (*v).into(), + } + } +} + impl fmt::Debug for Scheme { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Debug::fmt(self.as_str(), f)