diff --git a/Cargo.toml b/Cargo.toml index 59aba0a..07c6a9d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ futures = "0.1" tokio-io = "0.1.3" tokio-timer = "0.1" bytes = "0.4" -http = { git = "https://github.com/carllerche/http", branch = "uri-try-from-parts" } +http = { git = "https://github.com/carllerche/http", branch = "lower-case-header-name-parsing" } byteorder = "1.0" log = "0.3.8" fnv = "1.0.5" diff --git a/src/client.rs b/src/client.rs index b3b52b2..055de34 100644 --- a/src/client.rs +++ b/src/client.rs @@ -2,7 +2,6 @@ use {frame, HeaderMap, ConnectionError}; use Body; use frame::StreamId; use proto::{self, Connection, WindowSize}; -use error::Reason::*; use http::{Request, Response}; use futures::{Future, Poll, Sink, Async, AsyncSink}; @@ -224,7 +223,5 @@ impl proto::Peer for Peer { fn convert_poll_message(headers: frame::Headers) -> Result { headers.into_response() - // TODO: Is this always a protocol error? - .map_err(|_| ProtocolError.into()) } } diff --git a/src/error.rs b/src/error.rs index 815402e..7d98d24 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,3 +1,4 @@ +use http; use std::{error, fmt, io}; /// The error type for HTTP/2 operations @@ -162,6 +163,13 @@ impl From for io::Error { } } +impl From for ConnectionError { + fn from(_: http::Error) -> Self { + // TODO: Should this always be a protocol error? + Reason::ProtocolError.into() + } +} + impl fmt::Display for ConnectionError { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { use self::ConnectionError::*; diff --git a/src/frame/headers.rs b/src/frame/headers.rs index d90a094..96f551c 100644 --- a/src/frame/headers.rs +++ b/src/frame/headers.rs @@ -1,9 +1,11 @@ use super::{StreamId, StreamDependency}; +use ConnectionError; use hpack; use frame::{self, Frame, Head, Kind, Error}; use HeaderMap; +use error::Reason::*; -use http::{self, version, uri, Request, Response, Method, StatusCode, Uri}; +use http::{version, uri, Request, Response, Method, StatusCode, Uri}; use http::header::{self, HeaderName, HeaderValue}; use bytes::{BytesMut, Bytes}; @@ -183,11 +185,14 @@ impl Headers { decoder: &mut hpack::Decoder) -> Result<(), Error> { + let mut reg = false; let mut err = false; macro_rules! set_pseudo { ($field:ident, $val:expr) => {{ - if self.pseudo.$field.is_some() { + if reg { + err = true; + } else if self.pseudo.$field.is_some() { err = true; } else { self.pseudo.$field = Some($val); @@ -207,6 +212,7 @@ impl Headers { match header { Field { name, value } => { + reg = true; self.fields.append(name, value); } Authority(v) => set_pseudo!(authority, v), @@ -254,7 +260,7 @@ impl Headers { self.flags.set_end_stream() } - pub fn into_response(self) -> http::Result> { + pub fn into_response(self) -> Result, ConnectionError> { let mut b = Response::builder(); if let Some(status) = self.pseudo.status { @@ -267,33 +273,36 @@ impl Headers { Ok(response) } - pub fn into_request(self) -> http::Result> { + pub fn into_request(self) -> Result, ConnectionError> { let mut b = Request::builder(); - // TODO: should we distinguish between HTTP_2 and HTTP_2C? - // carllerche/http#42 b.version(version::HTTP_2); if let Some(method) = self.pseudo.method { b.method(method); } + // Specifying :status for a request is a protocol error + if self.pseudo.status.is_some() { + return Err(ProtocolError.into()); + } + // Convert the URI let mut parts = uri::Parts::default(); if let Some(scheme) = self.pseudo.scheme { // TODO: Don't unwrap - parts.scheme = Some(uri::Scheme::try_from_shared(scheme.into_inner()).unwrap()); + parts.scheme = Some(uri::Scheme::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_inner()).unwrap()); + parts.authority = Some(uri::Authority::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_inner()).unwrap()); + parts.path_and_query = Some(uri::PathAndQuery::from_shared(path.into_inner()).unwrap()); } b.uri(parts); @@ -400,7 +409,7 @@ impl Pseudo { unsafe { String::from_utf8_unchecked(src) } } - let path = parts.origin_form + let path = parts.path_and_query .map(|v| v.into()) .unwrap_or_else(|| Bytes::from_static(b"/")); diff --git a/src/hpack/encoder.rs b/src/hpack/encoder.rs index 9c597fd..8e6d8e9 100644 --- a/src/hpack/encoder.rs +++ b/src/hpack/encoder.rs @@ -544,7 +544,7 @@ mod test { use http::header::HeaderValue; let name = "my-password".parse().unwrap(); - let mut value = HeaderValue::try_from_bytes(b"12345").unwrap(); + let mut value = HeaderValue::from_bytes(b"12345").unwrap(); value.set_sensitive(true); let header = Header::Field { name: Some(name), value: value }; @@ -561,7 +561,7 @@ mod test { // Now, try to encode a sensitive header w/ a name in the static table let name = "authorization".parse().unwrap(); - let mut value = HeaderValue::try_from_bytes(b"12345").unwrap(); + let mut value = HeaderValue::from_bytes(b"12345").unwrap(); value.set_sensitive(true); let header = Header::Field { name: Some(name), value: value }; @@ -579,7 +579,7 @@ mod test { let _ = encode(&mut encoder, vec![self::header("my-password", "not-so-secret")]); let name = "my-password".parse().unwrap(); - let mut value = HeaderValue::try_from_bytes(b"12345").unwrap(); + let mut value = HeaderValue::from_bytes(b"12345").unwrap(); value.set_sensitive(true); let header = Header::Field { name: Some(name), value: value }; @@ -746,11 +746,11 @@ mod test { let res = encode(&mut encoder, vec![ Header::Field { name: Some("hello".parse().unwrap()), - value: HeaderValue::try_from_bytes(b"world").unwrap(), + value: HeaderValue::from_bytes(b"world").unwrap(), }, Header::Field { name: None, - value: HeaderValue::try_from_bytes(b"zomg").unwrap(), + value: HeaderValue::from_bytes(b"zomg").unwrap(), }, ]); @@ -772,11 +772,11 @@ mod test { let mut input = vec![ Header::Field { name: Some("hello".parse().unwrap()), - value: HeaderValue::try_from_bytes(b"world").unwrap(), + value: HeaderValue::from_bytes(b"world").unwrap(), }, Header::Field { name: None, - value: HeaderValue::try_from_bytes(b"zomg").unwrap(), + value: HeaderValue::from_bytes(b"zomg").unwrap(), }, ].into_iter(); @@ -822,7 +822,7 @@ mod test { use http::header::{HeaderName, HeaderValue}; let name = HeaderName::from_bytes(name.as_bytes()).unwrap(); - let value = HeaderValue::try_from_bytes(val.as_bytes()).unwrap(); + let value = HeaderValue::from_bytes(val.as_bytes()).unwrap(); Header::Field { name: Some(name), value: value } } diff --git a/src/hpack/header.rs b/src/hpack/header.rs index e35a4a5..8ab1244 100644 --- a/src/hpack/header.rs +++ b/src/hpack/header.rs @@ -81,8 +81,9 @@ impl Header { } } } else { - let name = try!(HeaderName::from_bytes(&name)); - let value = try!(HeaderValue::try_from_bytes(&value)); + // HTTP/2 requires lower case header names + let name = try!(HeaderName::from_lowercase(&name)); + let value = try!(HeaderValue::from_bytes(&value)); Ok(Header::Field { name: name, value: value }) } @@ -228,7 +229,7 @@ impl<'a> Name<'a> { Name::Field(name) => { Ok(Header::Field { name: name.clone(), - value: try!(HeaderValue::try_from_bytes(&*value)), + value: try!(HeaderValue::from_bytes(&*value)), }) } Name::Authority => { diff --git a/src/hpack/test/fuzz.rs b/src/hpack/test/fuzz.rs index 8751530..6a6aa94 100644 --- a/src/hpack/test/fuzz.rs +++ b/src/hpack/test/fuzz.rs @@ -330,7 +330,7 @@ fn gen_header_name(g: &mut StdRng) -> HeaderName { fn gen_header_value(g: &mut StdRng) -> HeaderValue { let value = gen_string(g, 0, 70); - HeaderValue::try_from_bytes(value.as_bytes()).unwrap() + HeaderValue::from_bytes(value.as_bytes()).unwrap() } fn gen_string(g: &mut StdRng, min: usize, max: usize) -> String { diff --git a/src/server.rs b/src/server.rs index 9919a89..36b49c9 100644 --- a/src/server.rs +++ b/src/server.rs @@ -370,7 +370,5 @@ impl proto::Peer for Peer { -> Result { headers.into_request() - // TODO: Is this always a protocol error? - .map_err(|_| ProtocolError.into()) } }