diff --git a/src/proto/h1/encode.rs b/src/proto/h1/encode.rs index 2df0c396b7..ff8f21fdf5 100644 --- a/src/proto/h1/encode.rs +++ b/src/proto/h1/encode.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::HashSet; use std::fmt; use std::io::IoSlice; @@ -9,7 +9,7 @@ use http::{ AUTHORIZATION, CACHE_CONTROL, CONTENT_ENCODING, CONTENT_LENGTH, CONTENT_RANGE, CONTENT_TYPE, HOST, MAX_FORWARDS, SET_COOKIE, TE, TRAILER, TRANSFER_ENCODING, }, - HeaderMap, HeaderName, HeaderValue, + HeaderMap, HeaderName, }; use super::io::WriteBuf; @@ -35,7 +35,7 @@ pub(crate) struct NotEof(u64); #[derive(Debug, PartialEq, Clone)] enum Kind { /// An Encoder for when Transfer-Encoding includes `chunked`. - Chunked(Option>), + Chunked(Option>), /// An Encoder for when Content-Length is set. /// /// Enforces that the body is not longer than the Content-Length header. @@ -77,7 +77,7 @@ impl Encoder { Encoder::new(Kind::CloseDelimited) } - pub(crate) fn into_chunked_with_trailing_fields(self, trailers: Vec) -> Encoder { + pub(crate) fn into_chunked_with_trailing_fields(self, trailers: Vec) -> Encoder { match self.kind { Kind::Chunked(_) => Encoder { kind: Kind::Chunked(Some(trailers)), @@ -168,7 +168,7 @@ impl Encoder { trace!("encoding trailers"); match &self.kind { Kind::Chunked(Some(allowed_trailer_fields)) => { - let allowed_trailer_field_map = allowed_trailer_field_map(allowed_trailer_fields); + let allowed_set: HashSet<&HeaderName> = allowed_trailer_fields.iter().collect(); let mut cur_name = None; let mut allowed_trailers = HeaderMap::new(); @@ -179,7 +179,7 @@ impl Encoder { } let name = cur_name.as_ref().expect("current header name"); - if allowed_trailer_field_map.contains_key(name.as_str()) { + if allowed_set.contains(name) { if is_valid_trailer_field(name) { allowed_trailers.insert(name, value); } else { @@ -279,22 +279,6 @@ fn is_valid_trailer_field(name: &HeaderName) -> bool { ) } -fn allowed_trailer_field_map(allowed_trailer_fields: &Vec) -> HashMap { - let mut trailer_map = HashMap::new(); - - for header_value in allowed_trailer_fields { - if let Ok(header_str) = header_value.to_str() { - let items: Vec<&str> = header_str.split(',').map(|item| item.trim()).collect(); - - for item in items { - trailer_map.entry(item.to_string()).or_insert(()); - } - } - } - - trailer_map -} - impl Buf for EncodedBuf where B: Buf, @@ -532,7 +516,7 @@ mod tests { #[test] fn chunked_with_valid_trailers() { let encoder = Encoder::chunked(); - let trailers = vec![HeaderValue::from_static("chunky-trailer")]; + let trailers = vec![HeaderName::from_static("chunky-trailer")]; let encoder = encoder.into_chunked_with_trailing_fields(trailers); let headers = HeaderMap::from_iter(vec![ @@ -557,8 +541,8 @@ mod tests { fn chunked_with_multiple_trailer_headers() { let encoder = Encoder::chunked(); let trailers = vec![ - HeaderValue::from_static("chunky-trailer"), - HeaderValue::from_static("chunky-trailer-2"), + HeaderName::from_static("chunky-trailer"), + HeaderName::from_static("chunky-trailer-2"), ]; let encoder = encoder.into_chunked_with_trailing_fields(trailers); @@ -606,8 +590,7 @@ mod tests { fn chunked_with_invalid_trailers() { let encoder = Encoder::chunked(); - let trailers = format!( - "{},{},{},{},{},{},{},{},{},{},{},{}", + let trailers = vec![ AUTHORIZATION, CACHE_CONTROL, CONTENT_ENCODING, @@ -620,8 +603,7 @@ mod tests { TRAILER, TRANSFER_ENCODING, TE, - ); - let trailers = vec![HeaderValue::from_str(&trailers).unwrap()]; + ]; let encoder = encoder.into_chunked_with_trailing_fields(trailers); let mut headers = HeaderMap::new(); @@ -644,7 +626,7 @@ mod tests { #[test] fn chunked_with_title_case_headers() { let encoder = Encoder::chunked(); - let trailers = vec![HeaderValue::from_static("chunky-trailer")]; + let trailers = vec![HeaderName::from_static("chunky-trailer")]; let encoder = encoder.into_chunked_with_trailing_fields(trailers); let headers = HeaderMap::from_iter(vec![( @@ -657,4 +639,34 @@ mod tests { dst.put(buf1); assert_eq!(dst, b"0\r\nChunky-Trailer: header data\r\n\r\n"); } + + #[test] + fn chunked_trailers_case_insensitive_matching() { + // Regression test for issue #4010: HTTP/1.1 trailers are case-sensitive + // + // Previously, the Trailer header values were stored as HeaderValue (preserving case) + // and compared against HeaderName (which is always lowercase). This caused trailers + // declared as "Chunky-Trailer" to not match actual trailers sent as "chunky-trailer". + // + // The fix converts Trailer header values to HeaderName during parsing, which + // normalizes the case and enables proper case-insensitive matching. + // + // Note: HeaderName::from_static() requires lowercase input. In real usage, + // HeaderName::from_bytes() is used to parse the Trailer header value, which + // normalizes mixed-case input like "Chunky-Trailer" to "chunky-trailer". + let encoder = Encoder::chunked(); + let trailers = vec![HeaderName::from_static("chunky-trailer")]; + let encoder = encoder.into_chunked_with_trailing_fields(trailers); + + // The actual trailer being sent + let headers = HeaderMap::from_iter(vec![( + HeaderName::from_static("chunky-trailer"), + HeaderValue::from_static("trailer value"), + )]); + + let buf = encoder.encode_trailers::<&[u8]>(headers, false).unwrap(); + let mut dst = Vec::new(); + dst.put(buf); + assert_eq!(dst, b"0\r\nchunky-trailer: trailer value\r\n\r\n"); + } } diff --git a/src/proto/h1/role.rs b/src/proto/h1/role.rs index 1c8f1fbcf7..f92092e5a9 100644 --- a/src/proto/h1/role.rs +++ b/src/proto/h1/role.rs @@ -650,7 +650,7 @@ impl Server { }; let mut encoder = Encoder::length(0); - let mut allowed_trailer_fields: Option> = None; + let mut allowed_trailer_fields: Option> = None; let mut wrote_date = false; let mut cur_name = None; let mut is_name_written = false; @@ -860,12 +860,22 @@ impl Server { extend(dst, value.as_bytes()); } - match allowed_trailer_fields { - Some(ref mut allowed_trailer_fields) => { - allowed_trailer_fields.push(value); - } - None => { - allowed_trailer_fields = Some(vec![value]); + // Parse the Trailer header value into HeaderNames. + // The value may contain comma-separated names. + // HeaderName normalizes to lowercase for case-insensitive matching. + if let Ok(value_str) = value.to_str() { + let names: Vec = value_str + .split(',') + .filter_map(|s| HeaderName::from_bytes(s.trim().as_bytes()).ok()) + .collect(); + + match allowed_trailer_fields { + Some(ref mut fields) => { + fields.extend(names); + } + None => { + allowed_trailer_fields = Some(names); + } } } @@ -1389,8 +1399,16 @@ impl Client { let encoder = encoder.map(|enc| { if enc.is_chunked() { - let allowed_trailer_fields: Vec = - headers.get_all(header::TRAILER).iter().cloned().collect(); + // Parse Trailer header values into HeaderNames. + // Each Trailer header value may contain comma-separated names. + // HeaderName normalizes to lowercase, enabling case-insensitive matching. + let allowed_trailer_fields: Vec = headers + .get_all(header::TRAILER) + .iter() + .filter_map(|hv| hv.to_str().ok()) + .flat_map(|s| s.split(',')) + .filter_map(|s| HeaderName::from_bytes(s.trim().as_bytes()).ok()) + .collect(); if !allowed_trailer_fields.is_empty() { return enc.into_chunked_with_trailing_fields(allowed_trailer_fields); diff --git a/tests/client.rs b/tests/client.rs index d5a0e4e005..5f976b1792 100644 --- a/tests/client.rs +++ b/tests/client.rs @@ -704,6 +704,44 @@ test! { body: None, } +test! { + name: client_post_req_body_chunked_with_trailer_titlecase, + + server: + expected: "\ + POST / HTTP/1.1\r\n\ + trailer: Chunky-Trailer\r\n\ + host: {addr}\r\n\ + transfer-encoding: chunked\r\n\ + \r\n\ + 5\r\n\ + hello\r\n\ + 0\r\n\ + chunky-trailer: header data\r\n\ + \r\n\ + ", + reply: REPLY_OK, + + client: + request: { + method: POST, + url: "http://{addr}/", + headers: { + "trailer" => "Chunky-Trailer", + }, + body_stream_with_trailers: ( + (futures_util::stream::once(async { Ok::<_, Infallible>(Bytes::from("hello"))})), + HeaderMap::from_iter(vec![( + HeaderName::from_static("chunky-trailer"), + HeaderValue::from_static("header data") + )].into_iter())), + }, + response: + status: OK, + headers: {}, + body: None, +} + test! { name: client_res_body_chunked_with_trailer, @@ -740,6 +778,42 @@ test! { }, } +test! { + name: client_res_body_chunked_with_trailer_titlecase, + + server: + expected: "GET / HTTP/1.1\r\nte: trailers\r\nhost: {addr}\r\n\r\n", + reply: "\ + HTTP/1.1 200 OK\r\n\ + transfer-encoding: chunked\r\n\ + trailer: Chunky-Trailer\r\n\ + \r\n\ + 5\r\n\ + hello\r\n\ + 0\r\n\ + chunky-trailer: header data\r\n\ + \r\n\ + ", + + client: + request: { + method: GET, + url: "http://{addr}/", + headers: { + "te" => "trailers", + }, + }, + response: + status: OK, + headers: { + "Transfer-Encoding" => "chunked", + }, + body: &b"hello"[..], + trailers: { + "chunky-trailer" => "header data", + }, +} + test! { name: client_res_body_chunked_with_pathological_trailers, diff --git a/tests/server.rs b/tests/server.rs index 485a10e049..1cd0eba570 100644 --- a/tests/server.rs +++ b/tests/server.rs @@ -2850,6 +2850,51 @@ fn http1_trailer_send_fields() { assert_eq!(body, expected_body); } +#[test] +fn http1_trailer_send_fields_titlecase() { + let body = futures_util::stream::once(async move { Ok("hello".into()) }); + let mut headers = HeaderMap::new(); + headers.insert("chunky-trailer", "header data".parse().unwrap()); + // Invalid trailer field that should not be sent + headers.insert("Host", "www.example.com".parse().unwrap()); + // Not specified in Trailer header, so should not be sent + headers.insert("foo", "bar".parse().unwrap()); + + let server = serve(); + server + .reply() + .header("transfer-encoding", "chunked") + .header("trailer", "Chunky-Trailer") + .body_stream_with_trailers(body, headers); + let mut req = connect(server.addr()); + req.write_all( + b"\ + GET / HTTP/1.1\r\n\ + Host: example.domain\r\n\ + Connection: keep-alive\r\n\ + TE: trailers\r\n\ + \r\n\ + ", + ) + .expect("writing"); + + let chunky_trailer_chunk = b"\r\nchunky-trailer: header data\r\n\r\n"; + let res = read_until(&mut req, |buf| buf.ends_with(chunky_trailer_chunk)).expect("reading"); + let sres = s(&res); + + let expected_head = + "HTTP/1.1 200 OK\r\ntransfer-encoding: chunked\r\ntrailer: Chunky-Trailer\r\n"; + assert_eq!(&sres[..expected_head.len()], expected_head); + + // skip the date header + let date_fragment = "GMT\r\n\r\n"; + let pos = sres.find(date_fragment).expect("find GMT"); + let body = &sres[pos + date_fragment.len()..]; + + let expected_body = "5\r\nhello\r\n0\r\nchunky-trailer: header data\r\n\r\n"; + assert_eq!(body, expected_body); +} + #[test] fn http1_trailer_fields_not_allowed() { let body = futures_util::stream::once(async move { Ok("hello".into()) });