fix(http1): detect TE: trailers caselessly and with other values (hyperium#4152)

Author: seanmonstar

src/headers.rs

@@ -42,6 +42,26 @@ fn connection_has(value: &HeaderValue, needle: &str) -> bool {
     false
 }
 
+#[cfg(feature = "http1")]
+pub(super) fn te_is_trailers(headers: &http::HeaderMap) -> bool {
+    header_value_list_has(headers.get_all(http::header::TE).into_iter(), "trailers")
+}
+
+#[cfg(feature = "http1")]
+fn header_value_list_has(values: http::header::ValueIter<'_, HeaderValue>, needle: &str) -> bool {
+    for value in values {
+        if let Ok(line) = value.to_str() {
+            for token in line.split(',') {
+                if token.trim().eq_ignore_ascii_case(needle) {
+                    return true;
+                }
+            }
+        }
+    }
+
+    false
+}
+
 #[cfg(all(feature = "http1", feature = "server"))]
 pub(super) fn content_length_parse(value: &HeaderValue) -> Option<u64> {
     from_digits(value.as_bytes())
@@ -166,3 +186,35 @@ pub(super) fn add_chunked(mut entry: http::header::OccupiedEntry<'_, HeaderValue
 
     entry.insert(HeaderValue::from_static(CHUNKED));
 }
+
+#[cfg(all(test, feature = "http1"))]
+mod tests {
+    use super::te_is_trailers;
+    use http::header::{HeaderValue, TE};
+    use http::HeaderMap;
+
+    #[test]
+    fn te_is_trailers_accepts_comma_separated_values() {
+        let mut headers = HeaderMap::new();
+        headers.insert(TE, HeaderValue::from_static("gzip, Trailers"));
+
+        assert!(te_is_trailers(&headers));
+    }
+
+    #[test]
+    fn te_is_trailers_accepts_multiple_header_lines() {
+        let mut headers = HeaderMap::new();
+        headers.append(TE, HeaderValue::from_static("gzip"));
+        headers.append(TE, HeaderValue::from_static("trailers"));
+
+        assert!(te_is_trailers(&headers));
+    }
+
+    #[test]
+    fn te_is_trailers_rejects_missing_trailers_token() {
+        let mut headers = HeaderMap::new();
+        headers.insert(TE, HeaderValue::from_static("gzip"));
+
+        assert!(!te_is_trailers(&headers));
+    }
+}

src/proto/h1/conn.rs

@@ -11,7 +11,7 @@ use std::time::Duration;
 use crate::rt::{Read, Write};
 use bytes::{Buf, Bytes};
 use futures_core::ready;
-use http::header::{HeaderValue, CONNECTION, TE};
+use http::header::{HeaderValue, CONNECTION};
 use http::{HeaderMap, Method, Version};
 use http_body::Frame;
 use httparse::ParserConfig;
@@ -325,11 +325,7 @@ where
             ));
         }
 
-        self.state.allow_trailer_fields = msg
-            .head
-            .headers
-            .get(TE)
-            .map_or(false, |te_header| te_header == "trailers");
+        self.state.allow_trailer_fields = headers::te_is_trailers(&msg.head.headers);
 
         Poll::Ready(Some(Ok((msg.head, msg.decode, wants))))
     }

tests/server.rs

@@ -3335,6 +3335,79 @@ fn http1_trailer_fields_not_allowed() {
     assert_eq!(body, expected_body);
 }
 
+#[test]
+fn http1_trailer_fields_allowed_with_comma_separated_te() {
+    let body = futures_util::stream::once(async move { Ok("hello".into()) });
+    let mut headers = HeaderMap::new();
+    headers.insert("chunky-trailer", "header data".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: gzip, 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 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_allowed_with_multiple_te_headers() {
+    let body = futures_util::stream::once(async move { Ok("hello".into()) });
+    let mut headers = HeaderMap::new();
+    headers.insert("chunky-trailer", "header data".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: gzip\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 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_recv_fields() {
     let server = serve();