diff --git a/crates/genie-common/src/probe.rs b/crates/genie-common/src/probe.rs index 7815f24e..e34ec9f9 100644 --- a/crates/genie-common/src/probe.rs +++ b/crates/genie-common/src/probe.rs @@ -266,10 +266,16 @@ async fn read_http_body( ) -> Result { let mut body = buf.split_off(header_end); - if headers - .lines() - .any(|line| line.eq_ignore_ascii_case("transfer-encoding: chunked")) - { + // Fail loudly rather than handing back an encoded payload as if it were + // text: removing chunk framing from a `gzip, chunked` body still leaves + // gzip bytes, which the lossy UTF-8 conversion below would silently mangle + // into a plausible-looking String no caller can detect as wrong. + let undecodable = undecodable_codings(headers); + if !undecodable.is_empty() { + anyhow::bail!("unsupported transfer coding: {}", undecodable.join(", ")); + } + + if is_chunked(headers) { let decoded = read_chunked_body(stream, &mut body, read_timeout, max_response_bytes).await?; return Ok(String::from_utf8_lossy(&decoded).trim().to_string()); @@ -411,6 +417,65 @@ async fn read_with_timeout( .context("failed to read HTTP response") } +/// True when the response body is chunk-framed. +/// +/// Parsed the same way as [`parse_content_length`] rather than by matching the +/// whole header line: RFC 7230 §3.2 allows zero or more spaces after the colon, +/// and §3.3.1 permits a codings list (`gzip, chunked`) whose final entry is +/// what frames the body. An exact-string match missed both, and since a chunked +/// response carries no `Content-Length`, the miss fell through to the +/// read-to-EOF path and handed the caller the raw chunk-size lines as content. +/// The transfer codings applied to the body, in the order they were applied. +/// +/// Repeated `Transfer-Encoding` fields are combined in the order received, as +/// if their values were one comma-separated list (RFC 9110 §5.3), so +/// `Transfer-Encoding: chunked` followed by `Transfer-Encoding: gzip` yields +/// `[chunked, gzip]` — not two independent "is it chunked?" answers. +fn transfer_codings(headers: &str) -> Vec { + let mut codings = Vec::new(); + for line in headers.lines() { + let Some((name, value)) = line.split_once(':') else { + continue; + }; + if name.trim().eq_ignore_ascii_case("transfer-encoding") { + codings.extend( + value + .split(',') + .map(|coding| coding.trim().to_ascii_lowercase()) + .filter(|coding| !coding.is_empty()), + ); + } + } + codings +} + +/// True when chunk framing delimits the body. +/// +/// Only the *final* coding decides this (RFC 9112 §6.1): when chunked is not +/// last the message is close-delimited instead, so a scan for "is chunked +/// present anywhere" would try to chunk-parse a body that is not chunk-framed. +fn is_chunked(headers: &str) -> bool { + transfer_codings(headers) + .last() + .is_some_and(|last| last == "chunked") +} + +/// Codings this reader cannot undo, outermost first. +/// +/// Chunk framing is the one coding [`read_http_body`] removes, and only when it +/// is final. Anything else — `gzip`, `deflate`, `br`, `compress` — leaves the +/// payload encoded, and since the body is handed back as a `String` through a +/// lossy UTF-8 conversion, decoding it later is not possible. `identity` is a +/// no-op and needs no decoder. +fn undecodable_codings(headers: &str) -> Vec { + let mut codings = transfer_codings(headers); + if codings.last().is_some_and(|last| last == "chunked") { + codings.pop(); + } + codings.retain(|coding| coding != "identity"); + codings +} + fn parse_content_length(headers: &str) -> Option { headers.lines().find_map(|line| { let (name, value) = line.split_once(':')?; @@ -719,6 +784,84 @@ mod tests { assert_eq!(decoded, b"hello world"); } + async fn get_body_with_headers(header_block: &str, body: &str) -> Result { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let response = format!("HTTP/1.1 200 OK\r\n{header_block}\r\n\r\n{body}"); + let server = tokio::spawn(async move { + let (mut stream, _) = listener.accept().await.unwrap(); + let mut buf = [0u8; 1024]; + let _ = stream.read(&mut buf).await.unwrap(); + stream.write_all(response.as_bytes()).await.unwrap(); + }); + let out = probe_http_get_body( + &addr.to_string(), + "/", + false, + ProbeTimeouts { + connect: Duration::from_secs(2), + read: Duration::from_secs(2), + }, + ) + .await; + server.abort(); + out + } + + const CHUNKED_HELLO_WORLD: &str = "5\r\nhello\r\n6\r\n world\r\n0\r\n\r\n"; + + #[tokio::test] + async fn chunked_body_decoded_for_noncanonical_transfer_encoding() { + // RFC 9112 allows zero or more spaces after the colon. The old + // exact-string match on "transfer-encoding: chunked" missed these, so + // the raw chunk-size lines came back as the body. + for header in ["Transfer-Encoding:chunked", "Transfer-Encoding: chunked"] { + let body = get_body_with_headers(header, CHUNKED_HELLO_WORLD) + .await + .unwrap(); + assert_eq!(body, "hello world", "header was {header:?}"); + } + } + + #[test] + fn repeated_transfer_encoding_fields_combine_in_order() { + // RFC 9110 §5.3: repeated fields combine as one ordered list, and only + // the final coding frames the body (RFC 9112 §6.1). Scanning each line + // independently would call the first case chunked. + assert!(!is_chunked( + "Transfer-Encoding: chunked\r\nTransfer-Encoding: gzip\r\n" + )); + assert!(is_chunked( + "Transfer-Encoding: gzip\r\nTransfer-Encoding: chunked\r\n" + )); + assert_eq!( + transfer_codings("Transfer-Encoding: chunked\r\nTransfer-Encoding: gzip\r\n"), + vec!["chunked".to_string(), "gzip".to_string()], + ); + assert!(!is_chunked("Content-Length: 5\r\n")); + } + + #[tokio::test] + async fn undecodable_transfer_coding_is_rejected_not_mangled() { + // Stripping chunk framing off a `gzip, chunked` body leaves gzip bytes; + // the lossy UTF-8 conversion would turn those into a plausible-looking + // String no caller could tell was wrong. Fail instead. + let err = get_body_with_headers("Transfer-Encoding: gzip, chunked", CHUNKED_HELLO_WORLD) + .await + .unwrap_err(); + assert!( + err.to_string().contains("unsupported transfer coding"), + "got: {err}" + ); + + // `identity` is a no-op, so it must not trip the same guard. + let body = + get_body_with_headers("Transfer-Encoding: identity, chunked", CHUNKED_HELLO_WORLD) + .await + .unwrap(); + assert_eq!(body, "hello world"); + } + #[tokio::test] async fn read_chunked_body_rejects_oversize_chunk_header() { // A chunk-size line of `usize::MAX` (16 hex Fs on a 64-bit target) used to