Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
38 changes: 31 additions & 7 deletions src/client/hyper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<http::Uri, ClientError> {
let sanitized = sanitize_uri(location);
if let Ok(uri) = sanitized.parse::<http::Uri>()
&& uri.scheme().is_some()
{
return Ok(uri);
return Ok(normalize_empty_path(uri));
}

let scheme = current.scheme_str().unwrap_or("https");
Expand All @@ -1549,9 +1570,12 @@ fn resolve_redirect(current: &http::Uri, location: &str) -> Result<http::Uri, Cl
})?;

let absolute = format!("{}://{}{}", scheme, authority, sanitized);
absolute.parse().map_err(|e: http::uri::InvalidUri| {
ClientError::invalid_url(format!("invalid redirect URL '{}': {}", absolute, e))
})
absolute
.parse()
.map(normalize_empty_path)
.map_err(|e: http::uri::InvalidUri| {
ClientError::invalid_url(format!("invalid redirect URL '{}': {}", absolute, e))
})
}

// ── HttpClient implementation ─────────────────────────────────────
Expand Down Expand Up @@ -1645,9 +1669,9 @@ impl HyperClient {
let start = Instant::now();

let sanitized_url = sanitize_uri(&config.url);
let mut uri: http::Uri = sanitized_url.parse().map_err(|e: http::uri::InvalidUri| {
ClientError::invalid_url(format!("invalid URL: {}", e))
})?;
let mut uri: http::Uri = sanitized_url.parse().map(normalize_empty_path).map_err(
|e: http::uri::InvalidUri| ClientError::invalid_url(format!("invalid URL: {}", e)),
)?;

debug_record(log, v, 1, &format!("-> {} {}", config.method(), uri));
if let Some(proxy) = config.effective_proxy(uri.host().unwrap_or("")) {
Expand Down
110 changes: 110 additions & 0 deletions tests/h2_path.rs
Original file line number Diff line number Diff line change
@@ -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<Mutex<Vec<String>>>) {
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);
}
19 changes: 17 additions & 2 deletions tests/tls_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -89,6 +89,21 @@ fn generate_self_signed(config: &TlsServerConfig) -> (PKey<openssl::pkey::Privat
}

fn build_acceptor(config: &TlsServerConfig) -> 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();

Expand Down Expand Up @@ -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)
Expand Down
Loading