Fix stream-id double-accounting bug (#112)

Both Recv::open and Rev::recv_headers check new stream ids against the
previously stream id. The second such check fails.

Now, only Recv::open performs stream id checks.

Fixes #110
This commit is contained in:
Oliver Gould
2017-09-26 10:42:12 -07:00
committed by Carl Lerche
parent 8911ee2a4b
commit 0b289fd55d
4 changed files with 95 additions and 10 deletions

View File

@@ -137,13 +137,6 @@ where
let is_initial = stream.state.recv_open(frame.is_end_stream())?;
if is_initial {
let next_id = self.next_stream_id()?;
if frame.stream_id() >= next_id {
self.next_stream_id = frame.stream_id().next_id();
} else {
return Err(RecvError::Connection(ProtocolError));
}
// TODO: be smarter about this logic
if frame.stream_id() > self.last_processed_id {
self.last_processed_id = frame.stream_id();

View File

@@ -116,10 +116,9 @@ impl State {
return Ok(());
}
/// Open the receive have of the stream, this action is taken when a HEADERS
/// frame is received.
/// Opens the receive-half of the stream when a HEADERS frame is received.
///
/// Returns true if this transitions the state to Open
/// Returns true if this transitions the state to Open.
pub fn recv_open(&mut self, eos: bool) -> Result<bool, RecvError> {
let remote = Peer::Streaming;
let mut initial = false;

View File

@@ -22,6 +22,39 @@ fn read_preface_in_multiple_frames() {
assert!(Stream::wait(h2).next().is_none());
}
#[test]
fn serve_request() {
let _ = ::env_logger::init();
let (io, client) = mock::new();
let client = client
.assert_server_handshake()
.unwrap()
.recv_settings()
.send_frame(
frames::headers(1)
.request("GET", "https://example.com/")
.eos(),
)
.recv_frame(frames::headers(1).response(200).eos())
.close();
let srv = Server::handshake(io).expect("handshake").and_then(|srv| {
srv.into_future().unwrap().and_then(|(reqstream, srv)| {
let (req, mut stream) = reqstream.unwrap();
assert_eq!(req.method(), &http::Method::GET);
let rsp = http::Response::builder().status(200).body(()).unwrap();
stream.send_response(rsp, true).unwrap();
srv.into_future().unwrap()
})
});
srv.join(client).wait().expect("wait");
}
#[test]
#[ignore]
fn accept_with_pending_connections_after_socket_close() {}

View File

@@ -162,6 +162,66 @@ impl Handle {
Box::new(ret)
}
/// Perform the H2 handshake
pub fn assert_server_handshake(
self,
) -> Box<Future<Item = (frame::Settings, Self), Error = h2::Error>> {
self.assert_server_handshake_with_settings(frame::Settings::default())
}
/// Perform the H2 handshake
pub fn assert_server_handshake_with_settings<T>(
mut self,
settings: T,
) -> Box<Future<Item = (frame::Settings, Self), Error = h2::Error>>
where
T: Into<frame::Settings>,
{
self.write_preface();
let settings = settings.into();
self.send(settings.into()).unwrap();
let ret = self.into_future()
.unwrap()
.map(|(frame, mut me)| {
match frame {
Some(Frame::Settings(settings)) => {
// Send the ACK
let ack = frame::Settings::ack();
// TODO: Don't unwrap?
me.send(ack.into()).unwrap();
(settings, me)
},
Some(frame) => {
panic!("unexpected frame; frame={:?}", frame);
},
None => {
panic!("unexpected EOF");
},
}
})
.then(|res| {
let (settings, me) = res.unwrap();
me.into_future()
.map_err(|e| panic!("error: {:?}", e))
.map(|(frame, me)| {
let f = assert_settings!(frame.unwrap());
// Is ACK
assert!(f.is_ack());
(settings, me)
})
});
Box::new(ret)
}
}
impl Stream for Handle {