From a203365d797fd8c4774e8a409c724f1ec161cf40 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 26 Jul 2017 13:20:26 -0700 Subject: [PATCH] Switch to string crate (#6) --- Cargo.toml | 1 + src/client.rs | 15 +--------- src/frame/headers.rs | 51 +++++++++++++++++++++++--------- src/hpack/decoder.rs | 22 ++++++++------ src/hpack/header.rs | 21 +++++++------- src/hpack/test/fuzz.rs | 13 ++++++--- src/lib.rs | 4 +-- src/util/byte_str.rs | 66 ------------------------------------------ src/util/mod.rs | 1 - 9 files changed, 75 insertions(+), 119 deletions(-) delete mode 100644 src/util/byte_str.rs delete mode 100644 src/util/mod.rs diff --git a/Cargo.toml b/Cargo.toml index 91d1565..827804c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,7 @@ byteorder = "1.0" log = "0.3.8" fnv = "1.0.5" ordermap = "0.2.10" +string = { git = "https://github.com/carllerche/string" } [dev-dependencies] mock-io = { git = "https://github.com/carllerche/mock-io" } diff --git a/src/client.rs b/src/client.rs index c522451..dbd1289 100644 --- a/src/client.rs +++ b/src/client.rs @@ -78,20 +78,7 @@ impl Peer for Client { // Build the set pseudo header set. All requests will include `method` // and `path`. - let mut pseudo = frame::Pseudo::request(method, uri.path().into()); - - // If the URI includes a scheme component, add it to the pseudo headers - // - // TODO: Scheme must be set... - if let Some(scheme) = uri.scheme() { - pseudo.set_scheme(scheme.into()); - } - - // If the URI includes an authority component, add it to the pseudo - // headers - if let Some(authority) = uri.authority() { - pseudo.set_authority(authority.into()); - } + let pseudo = frame::Pseudo::request(method, uri); // Create the HEADERS frame let mut frame = frame::Headers::new(id, pseudo, headers); diff --git a/src/frame/headers.rs b/src/frame/headers.rs index 48aa18b..b80de90 100644 --- a/src/frame/headers.rs +++ b/src/frame/headers.rs @@ -1,13 +1,13 @@ use super::StreamId; use hpack; use frame::{self, Frame, Head, Kind, Error}; -use util::byte_str::ByteStr; -use http::{request, response, version, uri, Method, StatusCode}; +use http::{request, response, version, uri, Method, StatusCode, Uri}; use http::header::{self, HeaderMap, HeaderName, HeaderValue}; use bytes::{BytesMut, Bytes}; use byteorder::{BigEndian, ByteOrder}; +use string::String; use std::io::Cursor; @@ -84,9 +84,9 @@ pub struct StreamDependency { pub struct Pseudo { // Request method: Option, - scheme: Option, - authority: Option, - path: Option, + scheme: Option>, + authority: Option>, + path: Option>, // Response status: Option, @@ -225,17 +225,17 @@ impl Headers { if let Some(scheme) = self.pseudo.scheme { // TODO: Don't unwrap - parts.scheme = Some(uri::Scheme::try_from_shared(scheme.into()).unwrap()); + parts.scheme = Some(uri::Scheme::try_from_shared(scheme.into_inner()).unwrap()); } if let Some(authority) = self.pseudo.authority { // TODO: Don't unwrap - parts.authority = Some(uri::Authority::try_from_shared(authority.into()).unwrap()); + parts.authority = Some(uri::Authority::try_from_shared(authority.into_inner()).unwrap()); } if let Some(path) = self.pseudo.path { // TODO: Don't unwrap - parts.origin_form = Some(uri::OriginForm::try_from_shared(path.into()).unwrap()); + parts.origin_form = Some(uri::OriginForm::try_from_shared(path.into_inner()).unwrap()); } request.uri = parts.into(); @@ -297,14 +297,39 @@ impl From for Frame { // ===== impl Pseudo ===== impl Pseudo { - pub fn request(method: Method, path: ByteStr) -> Self { - Pseudo { + pub fn request(method: Method, uri: Uri) -> Self { + let parts = uri::Parts::from(uri); + + fn to_string(src: Bytes) -> String { + unsafe { String::from_utf8_unchecked(src) } + } + + let path = parts.origin_form + .map(|v| v.into()) + .unwrap_or_else(|| Bytes::from_static(b"/")); + + let mut pseudo = Pseudo { method: Some(method), scheme: None, authority: None, - path: Some(path), + path: Some(to_string(path)), status: None, + }; + + // If the URI includes a scheme component, add it to the pseudo headers + // + // TODO: Scheme must be set... + if let Some(scheme) = parts.scheme { + pseudo.set_scheme(to_string(scheme.into())); } + + // If the URI includes an authority component, add it to the pseudo + // headers + if let Some(authority) = parts.authority { + pseudo.set_authority(to_string(authority.into())); + } + + pseudo } pub fn response(status: StatusCode) -> Self { @@ -317,11 +342,11 @@ impl Pseudo { } } - pub fn set_scheme(&mut self, scheme: ByteStr) { + pub fn set_scheme(&mut self, scheme: String) { self.scheme = Some(scheme); } - pub fn set_authority(&mut self, authority: ByteStr) { + pub fn set_authority(&mut self, authority: String) { self.authority = Some(authority); } } diff --git a/src/hpack/decoder.rs b/src/hpack/decoder.rs index 4c5b2c0..d6cde3b 100644 --- a/src/hpack/decoder.rs +++ b/src/hpack/decoder.rs @@ -1,13 +1,14 @@ use super::{huffman, Header}; use frame; -use util::byte_str::FromUtf8Error; use http::{method, header, status}; use bytes::{Buf, Bytes, BytesMut}; +use string::String; use std::cmp; use std::io::Cursor; use std::collections::VecDeque; +use std::str::Utf8Error; /// Decodes headers using HPACK #[derive(Debug)] @@ -487,8 +488,8 @@ impl Table { // ===== impl DecoderError ===== -impl From for DecoderError { - fn from(_: FromUtf8Error) -> DecoderError { +impl From for DecoderError { + fn from(_: Utf8Error) -> DecoderError { // TODO: Better error? DecoderError::InvalidUtf8 } @@ -532,16 +533,15 @@ impl From for frame::Error { pub fn get_static(idx: usize) -> Header { use http::{status, method, header}; use http::header::HeaderValue; - use util::byte_str::ByteStr; match idx { - 1 => Header::Authority(ByteStr::from_static("")), + 1 => Header::Authority(from_static("")), 2 => Header::Method(method::GET), 3 => Header::Method(method::POST), - 4 => Header::Path(ByteStr::from_static("/")), - 5 => Header::Path(ByteStr::from_static("/index.html")), - 6 => Header::Scheme(ByteStr::from_static("http")), - 7 => Header::Scheme(ByteStr::from_static("https")), + 4 => Header::Path(from_static("/")), + 5 => Header::Path(from_static("/index.html")), + 6 => Header::Scheme(from_static("http")), + 7 => Header::Scheme(from_static("https")), 8 => Header::Status(status::OK), 9 => Header::Status(status::NO_CONTENT), 10 => Header::Status(status::PARTIAL_CONTENT), @@ -740,3 +740,7 @@ pub fn get_static(idx: usize) -> Header { _ => unreachable!(), } } + +fn from_static(s: &'static str) -> String { + unsafe { String::from_utf8_unchecked(Bytes::from_static(s.as_bytes())) } +} diff --git a/src/hpack/header.rs b/src/hpack/header.rs index 66bf08e..e35a4a5 100644 --- a/src/hpack/header.rs +++ b/src/hpack/header.rs @@ -1,9 +1,9 @@ use super::DecoderError; -use util::byte_str::ByteStr; use http::{Method, StatusCode}; use http::header::{HeaderName, HeaderValue}; use bytes::Bytes; +use string::{String, TryFrom}; /// HTTP/2.0 Header #[derive(Debug, Clone, Eq, PartialEq)] @@ -12,10 +12,11 @@ pub enum Header { name: T, value: HeaderValue, }, - Authority(ByteStr), + // TODO: Change these types to `http::uri` types. + Authority(String), Method(Method), - Scheme(ByteStr), - Path(ByteStr), + Scheme(String), + Path(String), Status(StatusCode), } @@ -56,7 +57,7 @@ impl Header { if name[0] == b':' { match &name[1..] { b"authority" => { - let value = try!(ByteStr::from_utf8(value)); + let value = try!(String::try_from(value)); Ok(Header::Authority(value)) } b"method" => { @@ -64,11 +65,11 @@ impl Header { Ok(Header::Method(method)) } b"scheme" => { - let value = try!(ByteStr::from_utf8(value)); + let value = try!(String::try_from(value)); Ok(Header::Scheme(value)) } b"path" => { - let value = try!(ByteStr::from_utf8(value)); + let value = try!(String::try_from(value)); Ok(Header::Path(value)) } b"status" => { @@ -231,16 +232,16 @@ impl<'a> Name<'a> { }) } Name::Authority => { - Ok(Header::Authority(try!(ByteStr::from_utf8(value)))) + Ok(Header::Authority(try!(String::try_from(value)))) } Name::Method => { Ok(Header::Method(try!(Method::from_bytes(&*value)))) } Name::Scheme => { - Ok(Header::Scheme(try!(ByteStr::from_utf8(value)))) + Ok(Header::Scheme(try!(String::try_from(value)))) } Name::Path => { - Ok(Header::Path(try!(ByteStr::from_utf8(value)))) + Ok(Header::Path(try!(String::try_from(value)))) } Name::Status => { match StatusCode::from_bytes(&value) { diff --git a/src/hpack/test/fuzz.rs b/src/hpack/test/fuzz.rs index ecf9fa1..80adbec 100644 --- a/src/hpack/test/fuzz.rs +++ b/src/hpack/test/fuzz.rs @@ -6,7 +6,7 @@ use hpack::{Header, Decoder, Encoder, Encode}; use http::header::{HeaderName, HeaderValue}; -use self::bytes::BytesMut; +use self::bytes::{BytesMut, Bytes}; use self::quickcheck::{QuickCheck, Arbitrary, Gen, TestResult}; use self::rand::{StdRng, Rng, SeedableRng}; @@ -183,7 +183,7 @@ fn gen_header(g: &mut StdRng) -> Header> { match g.next_u32() % 5 { 0 => { let value = gen_string(g, 4, 20); - Header::Authority(value.into()) + Header::Authority(to_shared(value)) } 1 => { let method = match g.next_u32() % 6 { @@ -212,7 +212,7 @@ fn gen_header(g: &mut StdRng) -> Header> { _ => unreachable!(), }; - Header::Scheme(value.into()) + Header::Scheme(to_shared(value.to_string())) } 3 => { let value = match g.next_u32() % 100 { @@ -221,7 +221,7 @@ fn gen_header(g: &mut StdRng) -> Header> { _ => gen_string(g, 2, 20), }; - Header::Path(value.into()) + Header::Path(to_shared(value)) } 4 => { let status = (g.gen::() % 500) + 100; @@ -346,3 +346,8 @@ fn gen_string(g: &mut StdRng, min: usize, max: usize) -> String { String::from_utf8(bytes).unwrap() } + +fn to_shared(src: String) -> ::string::String { + let b: Bytes = src.into(); + unsafe { ::string::String::from_utf8_unchecked(b) } +} diff --git a/src/lib.rs b/src/lib.rs index 646cdc3..e625f78 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,6 +25,8 @@ extern crate byteorder; #[macro_use] extern crate log; +extern crate string; + pub mod client; pub mod error; mod hpack; @@ -32,8 +34,6 @@ mod proto; mod frame; pub mod server; -mod util; - pub use error::{ConnectionError, Reason}; pub use frame::StreamId; pub use proto::Connection; diff --git a/src/util/byte_str.rs b/src/util/byte_str.rs deleted file mode 100644 index 7cd4292..0000000 --- a/src/util/byte_str.rs +++ /dev/null @@ -1,66 +0,0 @@ -use bytes::Bytes; - -use std::{ops, str}; -use std::str::Utf8Error; - -#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] -pub struct ByteStr { - bytes: Bytes, -} - -#[derive(Debug)] -pub struct FromUtf8Error { - err: Utf8Error, -} - -impl ByteStr { - #[inline] - pub fn from_static(val: &'static str) -> ByteStr { - ByteStr { bytes: Bytes::from_static(val.as_bytes()) } - } - - #[inline] - pub unsafe fn from_utf8_unchecked(bytes: Bytes) -> ByteStr { - ByteStr { bytes: bytes } - } - - pub fn from_utf8(bytes: Bytes) -> Result { - if let Err(e) = str::from_utf8(&bytes[..]) { - return Err(FromUtf8Error { - err: e, - }); - } - - Ok(ByteStr { bytes: bytes }) - } -} - -impl ops::Deref for ByteStr { - type Target = str; - - #[inline] - fn deref(&self) -> &str { - let b: &[u8] = self.bytes.as_ref(); - unsafe { str::from_utf8_unchecked(b) } - } -} - -impl From for ByteStr { - #[inline] - fn from(src: String) -> ByteStr { - ByteStr { bytes: Bytes::from(src) } - } -} - -impl<'a> From<&'a str> for ByteStr { - #[inline] - fn from(src: &'a str) -> ByteStr { - ByteStr { bytes: Bytes::from(src) } - } -} - -impl From for Bytes { - fn from(src: ByteStr) -> Self { - src.bytes - } -} diff --git a/src/util/mod.rs b/src/util/mod.rs deleted file mode 100644 index 8b8e497..0000000 --- a/src/util/mod.rs +++ /dev/null @@ -1 +0,0 @@ -pub mod byte_str;