From 4e0e01aa5cb50bee5680ec2bcfd54553e06d6a42 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Tue, 11 Jul 2017 14:01:25 -0700 Subject: [PATCH 1/4] More tests --- tests/client_request.rs | 99 ++++++++++++++++++++++++++++++++++------- 1 file changed, 82 insertions(+), 17 deletions(-) diff --git a/tests/client_request.rs b/tests/client_request.rs index 6557135..f93bd7f 100644 --- a/tests/client_request.rs +++ b/tests/client_request.rs @@ -5,11 +5,11 @@ extern crate mock_io; extern crate env_logger; extern crate bytes; -use h2::client; -use http::request; -use bytes::Bytes; +use h2::{client, Frame}; +use http::{request, status}; use futures::*; +use bytes::Bytes; // TODO: move into another file macro_rules! assert_user_err { @@ -32,7 +32,7 @@ fn handshake() { .write(SETTINGS_ACK) .build(); - let mut h2 = client::handshake(mock) + let h2 = client::handshake(mock) .wait().unwrap(); // At this point, the connection should be closed @@ -55,7 +55,7 @@ fn get_with_204_response() { .read(&[0, 0, 1, 1, 5, 0, 0, 0, 1, 0x89]) .build(); - let mut h2 = client::handshake(mock) + let h2 = client::handshake(mock) .wait().unwrap(); // Send the request @@ -66,11 +66,18 @@ fn get_with_204_response() { // Get the response let (resp, h2) = h2.into_future().wait().unwrap(); + match resp.unwrap() { + Frame::Headers { headers, .. } => { + assert_eq!(headers.status, status::NO_CONTENT); + } + _ => panic!("unexpected frame"), + } + + // No more frames assert!(Stream::wait(h2).next().is_none()); } #[test] -#[ignore] fn get_with_200_response() { let _ = ::env_logger::init(); @@ -78,15 +85,18 @@ fn get_with_200_response() { .handshake() // Write GET / .write(&[ - 0, 0, 0x10, 1, 5, 0, 0, 0, 1, 0x82, 0x87, 0x41, 0x8B, 0x9D, 0x29, - 0xAC, 0x4B, 0x8F, 0xA8, 0xE9, 0x19, 0x97, 0x21, 0xE9, 0x84, + 0, 0, 16, 1, 5, 0, 0, 0, 1, 130, 135, 65, 139, 157, 41, 172, 75, + 143, 168, 233, 25, 151, 33, 233, 132 ]) .write(SETTINGS_ACK) // Read response - .read(&[0, 0, 1, 1, 5, 0, 0, 0, 1, 0x89]) + .read(&[ + 0, 0, 1, 1, 4, 0, 0, 0, 1, 136, 0, 0, 5, 0, 0, 0, 0, 0, 1, 104, 101, + 108, 108, 111, 0, 0, 5, 0, 1, 0, 0, 0, 1, 119, 111, 114, 108, 100, + ]) .build(); - let mut h2 = client::handshake(mock) + let h2 = client::handshake(mock) .wait().unwrap(); // Send the request @@ -94,9 +104,40 @@ fn get_with_200_response() { request.uri = "https://http2.akamai.com/".parse().unwrap(); let h2 = h2.send_request(1.into(), request, true).wait().unwrap(); - // Get the response + // Get the response headers let (resp, h2) = h2.into_future().wait().unwrap(); + match resp.unwrap() { + Frame::Headers { headers, .. } => { + assert_eq!(headers.status, status::OK); + } + _ => panic!("unexpected frame"), + } + + // Get the response body + let (data, h2) = h2.into_future().wait().unwrap(); + + match data.unwrap() { + Frame::Data { id, data, end_of_stream } => { + assert_eq!(id, 1.into()); + assert_eq!(data, &b"hello"[..]); + assert!(!end_of_stream); + } + _ => panic!("unexpected frame"), + } + + // Get the response body + let (data, h2) = h2.into_future().wait().unwrap(); + + match data.unwrap() { + Frame::Data { id, data, end_of_stream } => { + assert_eq!(id, 1.into()); + assert_eq!(data, &b"world"[..]); + assert!(end_of_stream); + } + _ => panic!("unexpected frame"), + } + assert!(Stream::wait(h2).next().is_none()); } @@ -135,10 +176,16 @@ fn request_with_server_stream_id() { } #[test] -#[ignore] -fn send_data_without_headers() { +fn send_headers_twice_with_same_stream_id() { + let _ = ::env_logger::init(); + let mock = mock_io::Builder::new() .handshake() + // Write GET / + .write(&[ + 0, 0, 0x10, 1, 5, 0, 0, 0, 1, 0x82, 0x87, 0x41, 0x8B, 0x9D, 0x29, + 0xAC, 0x4B, 0x8F, 0xA8, 0xE9, 0x19, 0x97, 0x21, 0xE9, 0x84, + ]) .build(); let h2 = client::handshake(mock) @@ -147,11 +194,29 @@ fn send_data_without_headers() { // Send the request let mut request = request::Head::default(); request.uri = "https://http2.akamai.com/".parse().unwrap(); + let h2 = h2.send_request(1.into(), request, true).wait().unwrap(); - /* - let err = h2.send_request(2.into(), request, true).wait().unwrap_err(); - assert_user_err!(err, InvalidStreamId); - */ + // Send another request with the same stream ID + let mut request = request::Head::default(); + request.uri = "https://http2.akamai.com/".parse().unwrap(); + let err = h2.send_request(1.into(), request, true).wait().unwrap_err(); + + assert_user_err!(err, UnexpectedFrameType); +} + +#[test] +fn send_data_without_headers() { + let mock = mock_io::Builder::new() + .handshake() + .build(); + + let h2 = client::handshake(mock) + .wait().unwrap(); + + let err = h2.send_data(1.into(), Bytes::from_static(b"hello world"), true) + .wait().unwrap_err(); + + assert_user_err!(err, InactiveStreamId); } #[test] From fab9fa8ed2d3b987c5e604ff54bcb4033b287594 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Tue, 11 Jul 2017 14:28:40 -0700 Subject: [PATCH 2/4] More tests --- examples/client.rs | 10 +++- examples/server.rs | 12 ++--- tests/client_request.rs | 107 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 119 insertions(+), 10 deletions(-) diff --git a/examples/client.rs b/examples/client.rs index 130d9c4..7e79c51 100644 --- a/examples/client.rs +++ b/examples/client.rs @@ -8,7 +8,7 @@ extern crate env_logger; use h2::client; -use http::request; +use http::*; use futures::*; @@ -34,11 +34,19 @@ pub fn main() { println!("sending request"); let mut request = request::Head::default(); + request.method = method::POST; request.uri = "https://http2.akamai.com/".parse().unwrap(); // request.version = version::H2; conn.send_request(1.into(), request, true) }) + /* + .then(|res| { + let conn = res.unwrap(); + + conn.send_data(1.into(), "hello".into(), true) + }) + */ .then(|res| { let conn = res.unwrap(); // Get the next message diff --git a/examples/server.rs b/examples/server.rs index bcd1f54..d826595 100644 --- a/examples/server.rs +++ b/examples/server.rs @@ -38,6 +38,12 @@ pub fn main() { // Receive a request conn.into_future() + .then(|res| { + let (frame, conn) = res.unwrap(); + println!("Zomg frame; {:?}", frame); + + conn.into_future() + }) .then(|res| { let (frame, conn) = res.unwrap(); println!("Zomg frame; {:?}", frame); @@ -47,12 +53,6 @@ pub fn main() { conn.send_response(1.into(), response, false) }) - .then(|res| { - let conn = res.unwrap(); - println!("... sending data frame"); - - conn.send_data(1.into(), "hello".into(), false) - }) .then(|res| { let conn = res.unwrap(); println!("... sending next frame"); diff --git a/tests/client_request.rs b/tests/client_request.rs index f93bd7f..47f198a 100644 --- a/tests/client_request.rs +++ b/tests/client_request.rs @@ -6,7 +6,7 @@ extern crate env_logger; extern crate bytes; use h2::{client, Frame}; -use http::{request, status}; +use http::*; use futures::*; use bytes::Bytes; @@ -158,6 +158,68 @@ fn request_with_zero_stream_id() { assert_user_err!(err, InvalidStreamId); } +#[test] +fn post_with_200_response() { + let _ = ::env_logger::init(); + + let mock = mock_io::Builder::new() + .handshake() + .write(&[ + // POST / + 0, 0, 16, 1, 4, 0, 0, 0, 1, 131, 135, 65, 139, 157, 41, + 172, 75, 143, 168, 233, 25, 151, 33, 233, 132, + ]) + .write(&[ + // DATA + 0, 0, 5, 0, 1, 0, 0, 0, 1, 104, 101, 108, 108, 111, + ]) + .write(SETTINGS_ACK) + // Read response + .read(&[ + // HEADERS + 0, 0, 1, 1, 4, 0, 0, 0, 1, 136, + // DATA + 0, 0, 5, 0, 1, 0, 0, 0, 1, 119, 111, 114, 108, 100 + ]) + .build(); + + let h2 = client::handshake(mock) + .wait().unwrap(); + + // Send the request + let mut request = request::Head::default(); + request.method = method::POST; + request.uri = "https://http2.akamai.com/".parse().unwrap(); + let h2 = h2.send_request(1.into(), request, false).wait().unwrap(); + + // Send the data + let h2 = h2.send_data(1.into(), "hello".into(), true).wait().unwrap(); + + // Get the response headers + let (resp, h2) = h2.into_future().wait().unwrap(); + + match resp.unwrap() { + Frame::Headers { headers, .. } => { + assert_eq!(headers.status, status::OK); + } + _ => panic!("unexpected frame"), + } + + // Get the response body + let (data, h2) = h2.into_future().wait().unwrap(); + + match data.unwrap() { + Frame::Data { id, data, end_of_stream } => { + assert_eq!(id, 1.into()); + assert_eq!(data, &b"world"[..]); + assert!(end_of_stream); + } + _ => panic!("unexpected frame"), + } + + assert!(Stream::wait(h2).next().is_none()); +} + #[test] fn request_with_server_stream_id() { let mock = mock_io::Builder::new() @@ -220,8 +282,31 @@ fn send_data_without_headers() { } #[test] -#[ignore] fn send_data_after_headers_eos() { + let _ = ::env_logger::init(); + + let mock = mock_io::Builder::new() + .handshake() + // Write GET / + .write(&[ + // GET /, no EOS + 0, 0, 16, 1, 5, 0, 0, 0, 1, 131, 135, 65, 139, 157, 41, 172, + 75, 143, 168, 233, 25, 151, 33, 233, 132 + ]) + .build(); + + let h2 = client::handshake(mock) + .wait().unwrap(); + + // Send the request + let mut request = request::Head::default(); + request.method = method::POST; + request.uri = "https://http2.akamai.com/".parse().unwrap(); + let h2 = h2.send_request(1.into(), request, true).wait().unwrap(); + + // Send the data + let err = h2.send_data(1.into(), "hello".into(), true).wait().unwrap_err(); + assert_user_err!(err, UnexpectedFrameType); } #[test] @@ -235,8 +320,24 @@ fn request_with_h1_version() { } #[test] -#[ignore] fn invalid_client_stream_id() { + let _ = ::env_logger::init(); + + for &id in &[0, 2] { + let mock = mock_io::Builder::new() + .handshake() + .build(); + + let h2 = client::handshake(mock) + .wait().unwrap(); + + // Send the request + let mut request = request::Head::default(); + request.uri = "https://http2.akamai.com/".parse().unwrap(); + let err = h2.send_request(id.into(), request, true).wait().unwrap_err(); + + assert_user_err!(err, InvalidStreamId); + } } #[test] From 36a1c6f045970c7473db0bb67ff8b1d7ae84cb9b Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Tue, 11 Jul 2017 20:50:41 -0700 Subject: [PATCH 3/4] More tests --- src/client.rs | 2 +- src/error.rs | 4 ++++ src/proto/connection.rs | 14 +++++++++++++- src/server.rs | 2 +- tests/client_request.rs | 37 ++++++++++++++++++++++++++++++++++++- 5 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/client.rs b/src/client.rs index bea78b3..8574e3c 100644 --- a/src/client.rs +++ b/src/client.rs @@ -57,7 +57,7 @@ impl Peer for Client { } fn is_valid_remote_stream_id(id: StreamId) -> bool { - id.is_server_initiated() + false } fn convert_send_message( diff --git a/src/error.rs b/src/error.rs index 606c3d9..4b82bb4 100644 --- a/src/error.rs +++ b/src/error.rs @@ -56,6 +56,9 @@ pub enum User { /// The stream is not currently expecting a frame of this type. UnexpectedFrameType, + /// The connection state is corrupt and the connection should be dropped. + Corrupt, + // TODO: reserve additional variants } @@ -93,6 +96,7 @@ macro_rules! user_desc { InvalidStreamId => concat!($prefix, "invalid stream ID"), InactiveStreamId => concat!($prefix, "inactive stream ID"), UnexpectedFrameType => concat!($prefix, "unexpected frame type"), + Corrupt => concat!($prefix, "connection state corrupt"), } }); } diff --git a/src/proto/connection.rs b/src/proto/connection.rs index f480ebf..ca93c49 100644 --- a/src/proto/connection.rs +++ b/src/proto/connection.rs @@ -23,6 +23,8 @@ use std::hash::BuildHasherDefault; pub struct Connection { inner: proto::Inner, streams: StreamMap, + // Set to `true` as long as the connection is in a valid state. + active: bool, peer: PhantomData<(P, B)>, } @@ -36,6 +38,7 @@ pub fn new(transport: proto::Inner) -> Connection Connection { inner: transport, streams: StreamMap::default(), + active: true, peer: PhantomData, } } @@ -108,6 +111,10 @@ impl Stream for Connection trace!("Connection::poll"); + if !self.active { + return Err(error::User::Corrupt.into()); + } + let frame = match try!(self.inner.poll()) { Async::Ready(f) => f, Async::NotReady => { @@ -135,7 +142,8 @@ impl Stream for Connection // connections should not be factored. if !P::is_valid_remote_stream_id(stream_id) { - unimplemented!(); + self.active = false; + return Err(error::Reason::ProtocolError.into()); } } @@ -179,6 +187,10 @@ impl Sink for Connection fn start_send(&mut self, item: Self::SinkItem) -> StartSend { + if !self.active { + return Err(error::User::Corrupt.into()); + } + // First ensure that the upstream can process a new item if !try!(self.poll_ready()).is_ready() { return Ok(AsyncSink::NotReady(item)); diff --git a/src/server.rs b/src/server.rs index fe40040..ff8411d 100644 --- a/src/server.rs +++ b/src/server.rs @@ -111,7 +111,7 @@ impl Peer for Server { type Poll = http::request::Head; fn is_valid_local_stream_id(id: StreamId) -> bool { - id.is_server_initiated() + false } fn is_valid_remote_stream_id(id: StreamId) -> bool { diff --git a/tests/client_request.rs b/tests/client_request.rs index 47f198a..6baea71 100644 --- a/tests/client_request.rs +++ b/tests/client_request.rs @@ -23,6 +23,17 @@ macro_rules! assert_user_err { }}; } +macro_rules! assert_proto_err { + ($actual:expr, $err:ident) => {{ + use h2::error::{ConnectionError, Reason}; + + match $actual { + ConnectionError::Proto(e) => assert_eq!(e, Reason::$err), + _ => panic!("unexpected connection error type"), + } + }}; +} + #[test] fn handshake() { let _ = ::env_logger::init(); @@ -341,8 +352,32 @@ fn invalid_client_stream_id() { } #[test] -#[ignore] fn invalid_server_stream_id() { + let _ = ::env_logger::init(); + + let mock = mock_io::Builder::new() + .handshake() + // Write GET / + .write(&[ + 0, 0, 0x10, 1, 5, 0, 0, 0, 1, 0x82, 0x87, 0x41, 0x8B, 0x9D, 0x29, + 0xAC, 0x4B, 0x8F, 0xA8, 0xE9, 0x19, 0x97, 0x21, 0xE9, 0x84, + ]) + .write(SETTINGS_ACK) + // Read response + .read(&[0, 0, 1, 1, 5, 0, 0, 0, 2, 137]) + .build(); + + let h2 = client::handshake(mock) + .wait().unwrap(); + + // Send the request + let mut request = request::Head::default(); + request.uri = "https://http2.akamai.com/".parse().unwrap(); + let h2 = h2.send_request(1.into(), request, true).wait().unwrap(); + + // Get the response + let (err, _) = h2.into_future().wait().unwrap_err(); + assert_proto_err!(err, ProtocolError); } #[test] From c061777663547e72262545703db8c6f56066a3f6 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 12 Jul 2017 14:42:08 -0700 Subject: [PATCH 4/4] Add large body test --- tests/client_request.rs | 64 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/client_request.rs b/tests/client_request.rs index 6baea71..6bd7817 100644 --- a/tests/client_request.rs +++ b/tests/client_request.rs @@ -152,6 +152,70 @@ fn get_with_200_response() { assert!(Stream::wait(h2).next().is_none()); } +#[test] +#[ignore] +fn post_with_large_body() { + let _ = ::env_logger::init(); + + let mock = mock_io::Builder::new() + .handshake() + .write(&[ + // POST / + 0, 0, 16, 1, 4, 0, 0, 0, 1, 131, 135, 65, 139, 157, 41, + 172, 75, 143, 168, 233, 25, 151, 33, 233, 132, + ]) + .write(&[ + // DATA + 0, 0, 5, 0, 1, 0, 0, 0, 1, 104, 101, 108, 108, 111, + ]) + .write(SETTINGS_ACK) + // Read response + .read(&[ + // HEADERS + 0, 0, 1, 1, 4, 0, 0, 0, 1, 136, + // DATA + 0, 0, 5, 0, 1, 0, 0, 0, 1, 119, 111, 114, 108, 100 + ]) + .build(); + + let h2 = client::handshake(mock) + .wait().unwrap(); + + // Send the request + let mut request = request::Head::default(); + request.method = method::POST; + request.uri = "https://http2.akamai.com/".parse().unwrap(); + let h2 = h2.send_request(1.into(), request, false).wait().unwrap(); + + // Send the data + let b = [0; 300]; + let h2 = h2.send_data(1.into(), (&b[..]).into(), true).wait().unwrap(); + + // Get the response headers + let (resp, h2) = h2.into_future().wait().unwrap(); + + match resp.unwrap() { + Frame::Headers { headers, .. } => { + assert_eq!(headers.status, status::OK); + } + _ => panic!("unexpected frame"), + } + + // Get the response body + let (data, h2) = h2.into_future().wait().unwrap(); + + match data.unwrap() { + Frame::Data { id, data, end_of_stream } => { + assert_eq!(id, 1.into()); + assert_eq!(data, &b"world"[..]); + assert!(end_of_stream); + } + _ => panic!("unexpected frame"), + } + + assert!(Stream::wait(h2).next().is_none()); +} + #[test] fn request_with_zero_stream_id() { let mock = mock_io::Builder::new()