Skip to content

Commit

Permalink
fix: HEADERS frame with non-zero content-length and END_STREAM is mal…
Browse files Browse the repository at this point in the history
…formed (#813)

Before this change, content-length underflow is only checked when
receiving date frames. The underflow error was never triggered if
data frames are never received.

This change adds similar check for headers frames.
  • Loading branch information
eaufavor authored Nov 11, 2024
1 parent 87c9be0 commit 640db36
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/frame/headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,10 @@ impl Headers {
&mut self.header_block.pseudo
}

pub(crate) fn pseudo(&self) -> &Pseudo {
&self.header_block.pseudo
}

/// Whether it has status 1xx
pub(crate) fn is_informational(&self) -> bool {
self.header_block.pseudo.is_informational()
Expand Down
12 changes: 12 additions & 0 deletions src/proto/streams/recv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,18 @@ impl Recv {
};

stream.content_length = ContentLength::Remaining(content_length);
// END_STREAM on headers frame with non-zero content-length is malformed.
// https://datatracker.ietf.org/doc/html/rfc9113#section-8.1.1
if frame.is_end_stream()
&& content_length > 0
&& frame
.pseudo()
.status
.map_or(true, |status| status != 204 && status != 304)
{
proto_err!(stream: "recv_headers with END_STREAM: content-length is not zero; stream={:?};", stream.id);
return Err(Error::library_reset(stream.id, Reason::PROTOCOL_ERROR).into());
}
}
}

Expand Down
43 changes: 43 additions & 0 deletions tests/h2-tests/tests/client_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1351,6 +1351,49 @@ async fn allow_empty_data_for_head() {
join(srv, h2).await;
}

#[tokio::test]
async fn reject_none_zero_content_length_header_with_end_stream() {
h2_support::trace_init!();
let (io, mut srv) = mock::new();

let srv = async move {
let settings = srv.assert_client_handshake().await;
assert_default_settings!(settings);
srv.recv_frame(
frames::headers(1)
.request("GET", "https://example.com/")
.eos(),
)
.await;
srv.send_frame(
frames::headers(1)
.response(200)
.field("content-length", 100)
.eos(),
)
.await;
};

let h2 = async move {
let (mut client, h2) = client::Builder::new()
.handshake::<_, Bytes>(io)
.await
.unwrap();
tokio::spawn(async {
h2.await.expect("connection failed");
});
let request = Request::builder()
.method(Method::GET)
.uri("https://example.com/")
.body(())
.unwrap();
let (response, _) = client.send_request(request, true).unwrap();
let _ = response.await.unwrap_err();
};

join(srv, h2).await;
}

#[tokio::test]
async fn early_hints() {
h2_support::trace_init!();
Expand Down

0 comments on commit 640db36

Please sign in to comment.