diff --git a/src/client.rs b/src/client.rs index c1c733e..fdab127 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1,12 +1,12 @@ //! HTTP2 client side. use {SendStream, RecvStream, ReleaseCapacity}; -use codec::{Codec, RecvError}; +use codec::{Codec, RecvError, SendError, UserError}; use frame::{Headers, Pseudo, Reason, Settings, StreamId}; use proto; use bytes::{Bytes, IntoBuf}; use futures::{Async, Future, MapErr, Poll}; -use http::{Request, Response}; +use http::{uri, Request, Response, Method, Version}; use tokio_io::{AsyncRead, AsyncWrite}; use tokio_io::io::WriteAll; @@ -46,7 +46,11 @@ pub struct ResponseFuture { /// Build a Client. #[derive(Clone, Debug)] pub struct Builder { + /// Initial `Settings` frame to send as part of the handshake. settings: Settings, + + /// The stream ID of the first (lowest) stream. Subsequent streams will use + /// monotonically increasing stream IDs. stream_id: StreamId, } @@ -359,20 +363,12 @@ impl Future for ResponseFuture { // ===== impl Peer ===== -impl proto::Peer for Peer { - type Send = Request<()>; - type Poll = Response<()>; - - - fn dyn() -> proto::DynPeer { - proto::DynPeer::Client - } - - fn is_server() -> bool { - false - } - - fn convert_send_message(id: StreamId, request: Self::Send, end_of_stream: bool) -> Headers { +impl Peer { + pub fn convert_send_message( + id: StreamId, + request: Request<()>, + end_of_stream: bool) -> Result + { use http::request::Parts; let ( @@ -380,14 +376,45 @@ impl proto::Peer for Peer { method, uri, headers, + version, .. }, _, ) = request.into_parts(); + let is_connect = method == Method::CONNECT; + // Build the set pseudo header set. All requests will include `method` // and `path`. - let pseudo = Pseudo::request(method, uri); + let mut pseudo = Pseudo::request(method, uri); + + if pseudo.scheme.is_none() { + // If the scheme is not set, then there are a two options. + // + // 1) Authority is not set. In this case, a request was issued with + // a relative URI. This is permitted **only** when forwarding + // HTTP 1.x requests. If the HTTP version is set to 2.0, then + // this is an error. + // + // 2) Authority is set, then the HTTP method *must* be CONNECT. + // + // It is not possible to have a scheme but not an authority set (the + // `http` crate does not allow it). + // + if pseudo.authority.is_none() { + if version == Version::HTTP_2 { + return Err(UserError::MissingUriSchemeAndAuthority.into()); + } else { + // This is acceptable as per the above comment. However, + // HTTP/2.0 requires that a scheme is set. Since we are + // forwarding an HTTP 1.1 request, the scheme is set to + // "http". + pseudo.set_scheme(uri::Scheme::HTTP); + } + } else if !is_connect { + // TODO: Error + } + } // Create the HEADERS frame let mut frame = Headers::new(id, pseudo, headers); @@ -396,7 +423,19 @@ impl proto::Peer for Peer { frame.set_end_stream() } - frame + Ok(frame) + } +} + +impl proto::Peer for Peer { + type Poll = Response<()>; + + fn dyn() -> proto::DynPeer { + proto::DynPeer::Client + } + + fn is_server() -> bool { + false } fn convert_poll_message(headers: Headers) -> Result { diff --git a/src/codec/error.rs b/src/codec/error.rs index 8d5d098..76ac619 100644 --- a/src/codec/error.rs +++ b/src/codec/error.rs @@ -47,6 +47,9 @@ pub enum UserError { /// Illegal headers, such as connection-specific headers. MalformedHeaders, + + /// Request submitted with relative URI. + MissingUriSchemeAndAuthority, } // ===== impl RecvError ===== @@ -125,6 +128,7 @@ impl error::Error for UserError { ReleaseCapacityTooBig => "release capacity too big", OverflowedStreamId => "stream ID overflowed", MalformedHeaders => "malformed headers", + MissingUriSchemeAndAuthority => "request URI missing scheme and authority", } } } diff --git a/src/frame/headers.rs b/src/frame/headers.rs index 8cd27e2..06cb53f 100644 --- a/src/frame/headers.rs +++ b/src/frame/headers.rs @@ -63,6 +63,7 @@ pub struct Continuation { headers: Iter, } +// TODO: These fields shouldn't be `pub` #[derive(Debug, Default, Eq, PartialEq)] pub struct Pseudo { // Request @@ -405,10 +406,6 @@ impl 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 .path_and_query .map(|v| v.into()) @@ -426,7 +423,7 @@ impl Pseudo { // // TODO: Scheme must be set... if let Some(scheme) = parts.scheme { - pseudo.set_scheme(to_string(scheme.into())); + pseudo.set_scheme(scheme); } // If the URI includes an authority component, add it to the pseudo @@ -448,8 +445,8 @@ impl Pseudo { } } - pub fn set_scheme(&mut self, scheme: String) { - self.scheme = Some(scheme); + pub fn set_scheme(&mut self, scheme: uri::Scheme) { + self.scheme = Some(to_string(scheme.into())); } pub fn set_authority(&mut self, authority: String) { @@ -457,6 +454,10 @@ impl Pseudo { } } +fn to_string(src: Bytes) -> String { + unsafe { String::from_utf8_unchecked(src) } +} + // ===== impl Iter ===== impl Iterator for Iter { diff --git a/src/proto/peer.rs b/src/proto/peer.rs index 7f1c2c9..1603e52 100644 --- a/src/proto/peer.rs +++ b/src/proto/peer.rs @@ -8,9 +8,6 @@ use std::fmt; /// Either a Client or a Server pub trait Peer { - /// Message type sent into the transport - type Send; - /// Message type polled from the transport type Poll: fmt::Debug; @@ -18,8 +15,6 @@ pub trait Peer { fn is_server() -> bool; - fn convert_send_message(id: StreamId, headers: Self::Send, end_of_stream: bool) -> Headers; - fn convert_poll_message(headers: Headers) -> Result; fn is_local_init(id: StreamId) -> bool { diff --git a/src/proto/streams/send.rs b/src/proto/streams/send.rs index 431e3e6..a7af550 100644 --- a/src/proto/streams/send.rs +++ b/src/proto/streams/send.rs @@ -74,8 +74,6 @@ impl Send { } } - - let end_stream = frame.is_end_stream(); // Update the state diff --git a/src/proto/streams/streams.rs b/src/proto/streams/streams.rs index 38e2d82..9ec3f0a 100644 --- a/src/proto/streams/streams.rs +++ b/src/proto/streams/streams.rs @@ -494,7 +494,8 @@ where } // Convert the message - let headers = client::Peer::convert_send_message(stream_id, request, end_of_stream); + let headers = client::Peer::convert_send_message( + stream_id, request, end_of_stream)?; let mut stream = me.store.insert(stream.id, stream); diff --git a/src/server.rs b/src/server.rs index 15337c9..298f6d2 100644 --- a/src/server.rs +++ b/src/server.rs @@ -375,23 +375,12 @@ impl fmt::Debug for Handshake } } -impl proto::Peer for Peer { - type Send = Response<()>; - type Poll = Request<()>; - - fn is_server() -> bool { - true - } - - fn dyn() -> proto::DynPeer { - proto::DynPeer::Server - } - - fn convert_send_message( +impl Peer { + pub fn convert_send_message( id: StreamId, - response: Self::Send, - end_of_stream: bool, - ) -> frame::Headers { + response: Response<()>, + end_of_stream: bool) -> frame::Headers + { use http::response::Parts; // Extract the components of the HTTP request @@ -417,6 +406,18 @@ impl proto::Peer for Peer { frame } +} + +impl proto::Peer for Peer { + type Poll = Request<()>; + + fn is_server() -> bool { + true + } + + fn dyn() -> proto::DynPeer { + proto::DynPeer::Server + } fn convert_poll_message(headers: frame::Headers) -> Result { use http::{uri, Version}; diff --git a/tests/client_request.rs b/tests/client_request.rs index b8ae4d7..7ab7b51 100644 --- a/tests/client_request.rs +++ b/tests/client_request.rs @@ -121,7 +121,6 @@ fn request_stream_id_overflows() { .body(()) .unwrap(); - // second cannot use the next stream id, it's over let poll_err = client.poll_ready().unwrap_err(); @@ -279,8 +278,71 @@ fn request_over_max_concurrent_streams_errors() { } #[test] -#[ignore] -fn request_without_scheme() {} +fn http_11_request_without_scheme_or_authority() { + let _ = ::env_logger::init(); + let (io, srv) = mock::new(); + + let srv = srv.assert_client_handshake() + .unwrap() + .recv_settings() + .recv_frame( + frames::headers(1) + .request("GET", "/") + .scheme("http") + .eos(), + ) + .send_frame(frames::headers(1).response(200).eos()) + .close(); + + let h2 = Client::handshake(io) + .expect("handshake") + .and_then(|(mut client, h2)| { + // we send a simple req here just to drive the connection so we can + // receive the server settings. + let request = Request::builder() + .method(Method::GET) + .uri("/") + .body(()) + .unwrap(); + + // first request is allowed + let (response, _) = client.send_request(request, true).unwrap(); + h2.drive(response) + .map(move |(h2, _)| (client, h2)) + }); + + h2.join(srv).wait().expect("wait"); +} + +#[test] +fn http_2_request_without_scheme_or_authority() { + let _ = ::env_logger::init(); + let (io, srv) = mock::new(); + + let srv = srv.assert_client_handshake() + .unwrap() + .recv_settings() + .close(); + + let h2 = Client::handshake(io) + .expect("handshake") + .and_then(|(mut client, h2)| { + // we send a simple req here just to drive the connection so we can + // receive the server settings. + let request = Request::builder() + .version(Version::HTTP_2) + .method(Method::GET) + .uri("/") + .body(()) + .unwrap(); + + // first request is allowed + assert!(client.send_request(request, true).is_err()); + h2.expect("h2") + }); + + h2.join(srv).wait().expect("wait"); +} #[test] #[ignore] diff --git a/tests/support/frames.rs b/tests/support/frames.rs index f8ca36e..3a25bc3 100644 --- a/tests/support/frames.rs +++ b/tests/support/frames.rs @@ -137,6 +137,16 @@ impl Mock { Mock(frame) } + pub fn scheme(self, value: &str) -> Self + { + let (id, mut pseudo, fields) = self.into_parts(); + let value = value.parse().unwrap(); + + pseudo.set_scheme(value); + + Mock(frame::Headers::new(id, pseudo, fields)) + } + pub fn eos(mut self) -> Self { self.0.set_end_stream(); self diff --git a/tests/support/prelude.rs b/tests/support/prelude.rs index 23004ec..a72a57b 100644 --- a/tests/support/prelude.rs +++ b/tests/support/prelude.rs @@ -32,7 +32,7 @@ pub use self::futures::{Future, IntoFuture, Sink, Stream}; pub use super::future_ext::{FutureExt, Unwrap}; // Re-export HTTP types -pub use self::http::{HeaderMap, Method, Request, Response, StatusCode}; +pub use self::http::{HeaderMap, Method, Request, Response, StatusCode, Version}; pub use self::bytes::{Buf, BufMut, Bytes, BytesMut, IntoBuf};