From 8b0102dc9aee2fdeac14c137000a09c15ba66361 Mon Sep 17 00:00:00 2001
From: cfal <cfal@users.noreply.github.com>
Date: Sat, 17 Oct 2026 16:17:26 +0000
Subject: [PATCH] Reject raw request-path fragments (#986)

PathAndQuery strips fragment suffixes before validating them, allowing malformed HTTP/2 request targets to reach the application as different paths.

Reject literal # in the decoded :path before URI conversion. Preserve percent-encoded hashes in both path and query.

The raw-frame regression fails on unpatched master and passes with this fix, requiring wire PROTOCOL_ERROR resets and successful encoded-hash sibling requests on the same connection.
---
 src/server.rs                                 |  5 ++
 .../h2-tests/tests/request_path_fragments.rs  | 46 +++++++++++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 tests/h2-tests/tests/request_path_fragments.rs

diff --git a/src/server.rs b/src/server.rs
index 931661a1..a042f77c 100644
--- a/src/server.rs
+++ b/src/server.rs
@@ -1745,6 +1745,11 @@ impl proto::Peer for Peer {
                 malformed!("malformed headers: missing path");
             }
 
+            // PathAndQuery would silently discard a fragment, including invalid suffix bytes.
+            if path.as_bytes().contains(&b'#') {
+                malformed!("malformed headers: fragment in :path ({:?})", path);
+            }
+
             let maybe_path = uri::PathAndQuery::from_maybe_shared(path.clone().into_inner());
             parts.path_and_query = Some(maybe_path.or_else(|why| {
                 malformed!("malformed headers: malformed path ({:?}): {}", path, why,)
diff --git a/tests/h2-tests/tests/request_path_fragments.rs b/tests/h2-tests/tests/request_path_fragments.rs
new file mode 100644
index 00000000..6a0cbd13
--- /dev/null
+++ b/tests/h2-tests/tests/request_path_fragments.rs
@@ -0,0 +1,46 @@
+use futures::StreamExt;
+use h2_support::prelude::*;
+
+#[tokio::test]
+async fn raw_path_fragments_are_rejected_but_encoded_hashes_survive() {
+    for path in [
+        "/admin#fragment",
+        "/admin?query#fragment",
+        "/admin#\r\ninvalid",
+        "/admin?query#\r\ninvalid",
+    ] {
+        let (io, mut peer) = mock::new();
+        let mut headers: frame::Headers = frames::headers(1)
+            .request("GET", "https://example.com/")
+            .eos()
+            .into();
+        headers.pseudo_mut().path = Some(util::byte_str(path));
+        let peer = async move {
+            peer.assert_server_handshake().await;
+            peer.send_frame(headers).await;
+            peer.recv_frame(frames::reset(1).protocol_error()).await;
+            for (id, path) in [(3, "/admin%23fragment"), (5, "/admin?query=%23fragment")] {
+                peer.send_frame(
+                    frames::headers(id)
+                        .request("GET", format!("https://example.com{}", path))
+                        .eos(),
+                )
+                .await;
+                peer.recv_frame(frames::headers(id).response(200).eos())
+                    .await;
+            }
+        };
+        let server = async move {
+            let mut connection = server::handshake(io).await.unwrap();
+            for path in ["/admin%23fragment", "/admin?query=%23fragment"] {
+                let (request, mut response) = connection.next().await.unwrap().unwrap();
+                assert_eq!(request.uri().path_and_query().unwrap().as_str(), path);
+                response.send_response(Response::new(()), true).unwrap();
+            }
+            assert!(connection.next().await.is_none());
+        };
+        tokio::time::timeout(Duration::from_secs(5), join(peer, server))
+            .await
+            .unwrap();
+    }
+}