From 2f7be9d62e45205429e0694c4e705baeeea02b35 Mon Sep 17 00:00:00 2001 From: liquidsec Date: Thu, 24 Sep 2026 13:43:04 -0400 Subject: [PATCH] give a URL with no path the path / before sending it A URL like https://crt.sh?q=example.com went out over HTTP/2 with :path set to ?q=example.com, because hyper builds :path from the URI's path-and-query as written. Servers answer that with a 400. The request URL and redirect targets now get a leading / when they have a host but no path, the same as curl and browsers. Fixes #111 --- Cargo.lock | 7 +++ Cargo.toml | 2 + src/client/hyper.rs | 38 ++++++++++++--- tests/h2_path.rs | 110 ++++++++++++++++++++++++++++++++++++++++++++ tests/tls_server.rs | 19 +++++++- 5 files changed, 167 insertions(+), 9 deletions(-) create mode 100644 tests/h2_path.rs diff --git a/Cargo.lock b/Cargo.lock index fc635ff..3e3891c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -423,6 +423,12 @@ version = "1.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6dbf3de79e51f3d586ab4cb9d5c3e2c14aa28ed23d180cf89b4df0454a69cc87" +[[package]] +name = "httpdate" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9" + [[package]] name = "hyper" version = "1.11.1" @@ -437,6 +443,7 @@ dependencies = [ "http", "http-body", "httparse", + "httpdate", "itoa", "pin-project-lite", "smallvec", diff --git a/Cargo.toml b/Cargo.toml index 072a839..692ea64 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -70,3 +70,5 @@ pyo3-async-runtimes = { version = "0.27", features = ["attributes", "tokio-runti [dev-dependencies] # TLS test server — uses our custom OpenSSL to serve with weak ciphers tokio-test = "0.4" +# h2 test server, for checking what the client puts on the wire over HTTP/2 +hyper = { version = "1", features = ["server"] } diff --git a/src/client/hyper.rs b/src/client/hyper.rs index a8a31c9..81c32de 100644 --- a/src/client/hyper.rs +++ b/src/client/hyper.rs @@ -1532,12 +1532,33 @@ fn is_redirect(status: u16) -> bool { matches!(status, 301 | 302 | 303 | 307 | 308) } +/// Give an absolute URL with no path (`https://host?q=1`) the path `/`, as +/// curl and browsers do. hyper builds the HTTP/2 `:path` straight from the +/// URI's path-and-query, so without this it sends `?q=1` and servers answer +/// 400. +fn normalize_empty_path(uri: http::Uri) -> http::Uri { + // `Uri::path()` already reports "/" for an empty path, so look at the raw + // path-and-query, which is what hyper puts on the wire. + let raw = uri.path_and_query().map(|pq| pq.as_str()).unwrap_or(""); + if uri.authority().is_none() || raw.starts_with('/') { + return uri; + } + let mut parts = uri.clone().into_parts(); + match format!("/{}", raw).parse() { + Ok(pq) => { + parts.path_and_query = Some(pq); + http::Uri::from_parts(parts).unwrap_or(uri) + } + Err(_) => uri, + } +} + fn resolve_redirect(current: &http::Uri, location: &str) -> Result { let sanitized = sanitize_uri(location); if let Ok(uri) = sanitized.parse::() && uri.scheme().is_some() { - return Ok(uri); + return Ok(normalize_empty_path(uri)); } let scheme = current.scheme_str().unwrap_or("https"); @@ -1549,9 +1570,12 @@ fn resolve_redirect(current: &http::Uri, location: &str) -> Result {} {}", config.method(), uri)); if let Some(proxy) = config.effective_proxy(uri.host().unwrap_or("")) { diff --git a/tests/h2_path.rs b/tests/h2_path.rs new file mode 100644 index 0000000..895c762 --- /dev/null +++ b/tests/h2_path.rs @@ -0,0 +1,110 @@ +//! What the client sends as `:path` over HTTP/2. +//! +//! hyper builds `:path` from the request URI's path-and-query as written, so +//! a URL with no path (`https://host?q=1`) has to be given one before it gets +//! there. Servers reject `:path: ?q=1` with a 400. + +#[allow(dead_code)] +mod tls_server; + +use blasthttp::client::HttpClient; +use blasthttp::client::hyper::HyperClient; +use blasthttp::config::RequestConfig; +use http_body_util::Full; +use hyper::body::Bytes; +use hyper_util::rt::{TokioExecutor, TokioIo}; +use std::net::SocketAddr; +use std::sync::{Arc, Mutex}; +use tls_server::{TlsServerConfig, build_h2_acceptor}; +use tokio::net::TcpListener; + +/// Serve HTTP/2 over TLS until the test ends, recording every `:path` that +/// arrives. Like a strict server, answer 400 to one that doesn't start with +/// `/`. `/redirect` sends the client to a URL on this server with no path. +async fn spawn_h2_server() -> (SocketAddr, Arc>>) { + let acceptor = build_h2_acceptor(&TlsServerConfig::default()); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let paths = Arc::new(Mutex::new(Vec::new())); + let seen = paths.clone(); + + tokio::spawn(async move { + loop { + let Ok((tcp, _)) = listener.accept().await else { + return; + }; + let ssl = openssl::ssl::Ssl::new(acceptor.context()).unwrap(); + let mut tls = tokio_openssl::SslStream::new(ssl, tcp).unwrap(); + if std::pin::Pin::new(&mut tls).accept().await.is_err() { + continue; + } + let seen = seen.clone(); + tokio::spawn(async move { + let service = hyper::service::service_fn(move |req: hyper::Request<_>| { + let path = req + .uri() + .path_and_query() + .map(|pq| pq.as_str().to_string()) + .unwrap_or_default(); + seen.lock().unwrap().push(path.clone()); + let resp = if !path.starts_with('/') { + hyper::Response::builder().status(400) + } else if path == "/redirect" { + hyper::Response::builder() + .status(302) + .header("location", format!("https://{}?from=redirect", addr)) + } else { + hyper::Response::builder().status(200) + }; + async move { + Ok::<_, std::convert::Infallible>( + resp.body(Full::new(Bytes::from_static(b"ok"))).unwrap(), + ) + } + }); + let _ = hyper::server::conn::http2::Builder::new(TokioExecutor::new()) + .serve_connection(TokioIo::new(tls), service) + .await; + }); + } + }); + + (addr, paths) +} + +fn h2_config(url: String) -> RequestConfig { + let mut config = RequestConfig::new(url); + config.verify_certs = Some(false); // self-signed cert + config.timeout_seconds = Some(5); + config.alpn_protocols = Some(vec!["h2".to_string()]); + config +} + +#[tokio::test] +async fn url_with_no_path_is_sent_with_a_leading_slash() { + let (addr, paths) = spawn_h2_server().await; + let config = h2_config(format!("https://{}?q=%25.example.com", addr)); + + let resp = HyperClient::new() + .send(&config) + .await + .expect("request failed"); + + assert_eq!(*paths.lock().unwrap(), vec!["/?q=%25.example.com"]); + assert_eq!(resp.status, 200); +} + +#[tokio::test] +async fn redirect_to_a_url_with_no_path_is_sent_with_a_leading_slash() { + let (addr, paths) = spawn_h2_server().await; + let mut config = h2_config(format!("https://{}/redirect", addr)); + config.follow_redirects = Some(true); + + let resp = HyperClient::new() + .send(&config) + .await + .expect("request failed"); + + assert_eq!(*paths.lock().unwrap(), vec!["/redirect", "/?from=redirect"]); + assert_eq!(resp.status, 200); +} diff --git a/tests/tls_server.rs b/tests/tls_server.rs index 5988139..ecf5f5c 100644 --- a/tests/tls_server.rs +++ b/tests/tls_server.rs @@ -7,7 +7,7 @@ use openssl::bn::BigNum; use openssl::hash::MessageDigest; use openssl::pkey::PKey; use openssl::rsa::Rsa; -use openssl::ssl::{SslAcceptor, SslMethod, SslOptions, SslVersion}; +use openssl::ssl::{AlpnError, SslAcceptor, SslAcceptorBuilder, SslMethod, SslOptions, SslVersion}; use openssl::x509::X509; use openssl::x509::extension::SubjectAlternativeName; use std::net::SocketAddr; @@ -89,6 +89,21 @@ fn generate_self_signed(config: &TlsServerConfig) -> (PKey SslAcceptor { + acceptor_builder(config).build() +} + +/// An acceptor that only speaks HTTP/2, for tests that serve with hyper's +/// h2 server instead of the hardcoded HTTP/1.1 response above. +#[allow(dead_code)] +pub fn build_h2_acceptor(config: &TlsServerConfig) -> SslAcceptor { + let mut builder = acceptor_builder(config); + builder.set_alpn_select_callback(|_, client| { + openssl::ssl::select_next_proto(b"\x02h2", client).ok_or(AlpnError::NOACK) + }); + builder.build() +} + +fn acceptor_builder(config: &TlsServerConfig) -> SslAcceptorBuilder { // Load the legacy provider so the server can use weak ciphers too load_legacy_provider(); @@ -133,7 +148,7 @@ fn build_acceptor(config: &TlsServerConfig) -> SslAcceptor { builder.set_max_proto_version(Some(max_ver)).unwrap(); } - builder.build() + builder } // Load legacy provider for the test server (same as our client does)