From 640db36dfa1ac1dbb0f4c289c3b5ed4ae8edbee0 Mon Sep 17 00:00:00 2001
From: Yuchen Wu <yuchen@cloudflare.com>
Date: Mon, 11 Nov 2024 14:04:44 -0800
Subject: [PATCH] fix: HEADERS frame with non-zero content-length and
 END_STREAM is malformed (#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.
---
 src/frame/headers.rs                   |  4 +++
 src/proto/streams/recv.rs              | 12 +++++++
 tests/h2-tests/tests/client_request.rs | 43 ++++++++++++++++++++++++++
 3 files changed, 59 insertions(+)

diff --git a/src/frame/headers.rs b/src/frame/headers.rs
index 8411a3aa..a0f282e3 100644
--- a/src/frame/headers.rs
+++ b/src/frame/headers.rs
@@ -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()
diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs
index a70527e2..d8572d00 100644
--- a/src/proto/streams/recv.rs
+++ b/src/proto/streams/recv.rs
@@ -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());
+                }
             }
         }
 
diff --git a/tests/h2-tests/tests/client_request.rs b/tests/h2-tests/tests/client_request.rs
index e914d484..8c991839 100644
--- a/tests/h2-tests/tests/client_request.rs
+++ b/tests/h2-tests/tests/client_request.rs
@@ -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!();