Simplified Transfer-Encoding Error Handling

spansy/CHANGELOG.md

@@ -5,3 +5,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+
+### Fixed
+- Improve handling of Transfer-Encoding errors

spansy/src/http/span.rs

@@ -213,27 +213,60 @@ fn from_header(src: &Bytes, header: &httparse::Header) -> Header {
     }
 }
 
+/// Gets the values of a header in a Request as a list of strings.
+fn get_header_values_request<'a>(
+    request: &'a Request,
+    header_name: &'a str,
+) -> Result<Vec<&'a str>, ParseError> {
+    get_header_values_from_iter(header_name, request.headers_with_name(header_name))
+}
+
+/// Gets the values of a header in a Response as a list of strings.
+fn get_header_values_response<'a>(
+    response: &'a Response,
+    header_name: &'a str,
+) -> Result<Vec<&'a str>, ParseError> {
+    get_header_values_from_iter(header_name, response.headers_with_name(header_name))
+}
+
+/// Gets the values of a header field from an Iterator as a list of strings.
+/// Returns a ParseError if any of the values are not valid UTF-8.
+fn get_header_values_from_iter<'a>(
+    header_name: &'a str,
+    headers: impl Iterator<Item = &'a Header>,
+) -> Result<Vec<&'a str>, ParseError> {
+    headers
+        .map(|h| {
+            std::str::from_utf8(h.value.0.as_bytes())
+                .map(|v| v.trim())
+                .map_err(|err| {
+                    ParseError(format!(
+                        "Invalid UTF-8 when parsing {header_name} header value: {err}"
+                    ))
+                })
+        })
+        .collect()
+}
+
 /// Calculates the length of the request body according to RFC 9112, section 6.
 fn request_body_len(request: &Request) -> Result<usize, ParseError> {
     // The presence of a message body in a request is signaled by a Content-Length
     // or Transfer-Encoding header field.
 
     // If a message is received with both a Transfer-Encoding and a Content-Length header field,
     // the Transfer-Encoding overrides the Content-Length
-    if request
-        .headers_with_name("Transfer-Encoding")
-        .next()
-        .is_some()
-    {
-        Err(ParseError(
-            "Transfer-Encoding not supported yet".to_string(),
-        ))
+    let transfer_encodings: Vec<&str> = get_header_values_request(request, "Transfer-Encoding")?;
+    if transfer_encodings.len() > 0 && transfer_encodings.iter().all(|v| *v != "identity") {
+        let bad_values: String = transfer_encodings.join(", ");
+        Err(ParseError(format!(
+            "Transfer-Encoding other than identity not supported yet {bad_values}"
+        )))
     } else if let Some(h) = request.headers_with_name("Content-Length").next() {
         // If a valid Content-Length header field is present without Transfer-Encoding, its decimal value
         // defines the expected message body length in octets.
         std::str::from_utf8(h.value.0.as_bytes())?
             .parse::<usize>()
-            .map_err(|err| ParseError(format!("failed to parse Content-Length value: {err}")))
+            .map_err(|err| ParseError(format!("Failed to parse Content-Length value: {err}")))
     } else {
         // If this is a request message and none of the above are true, then the message body length is zero
         Ok(0)
@@ -256,14 +289,13 @@ fn response_body_len(response: &Response) -> Result<usize, ParseError> {
         _ => {}
     }
 
-    if response
-        .headers_with_name("Transfer-Encoding")
-        .next()
-        .is_some()
-    {
-        Err(ParseError(
-            "Transfer-Encoding not supported yet".to_string(),
-        ))
+    let transfer_encodings: Vec<&str> = get_header_values_response(response, "Transfer-Encoding")?;
+    if transfer_encodings.len() > 0 && transfer_encodings.iter().all(|v| *v != "identity") {
+        let bad_values = transfer_encodings.join(", ");
+
+        Err(ParseError(format!(
+            "Transfer-Encoding other than identity not supported yet: {bad_values}"
+        )))
     } else if let Some(h) = response.headers_with_name("Content-Length").next() {
         // If a valid Content-Length header field is present without Transfer-Encoding, its decimal value
         // defines the expected message body length in octets.
@@ -363,6 +395,46 @@ mod tests {
                         Content-Length: 14\r\n\r\n\
                         {\"foo\": \"bar\"}";
 
+    const TEST_REQUEST_TRANSFER_ENCODING_IDENTITY: &[u8] = b"\
+                        POST / HTTP/1.1\r\n\
+                        Transfer-Encoding: identity\r\n\
+                        Content-Length: 12\r\n\r\n\
+                        Hello World!";
+
+    const TEST_RESPONSE_TRANSFER_ENCODING_IDENTITY: &[u8] = b"\
+                        HTTP/1.1 200 OK\r\n\
+                        Transfer-Encoding: identity\r\n\
+                        Content-Length: 12\r\n\r\n\
+                        Hello World!";
+
+    const TEST_REQUEST_TRANSFER_ENCODING_CHUNKED: &[u8] = b"\
+                        POST / HTTP/1.1\r\n\
+                        Transfer-Encoding: chunked\r\n\r\n\
+                        a\r\n\
+                        Hello World!\r\n\
+                        0\r\n\r\n";
+
+    const TEST_RESPONSE_TRANSFER_ENCODING_CHUNKED: &[u8] = b"\
+                        HTTP/1.1 200 OK\r\n\
+                        Transfer-Encoding: chunked\r\n\r\n\
+                        a\r\n\
+                        Hello World!\r\n\
+                        0\r\n\r\n";
+
+    const TEST_REQUEST_TRANSFER_ENCODING_MULTIPLE: &[u8] = b"\
+                        POST / HTTP/1.1\r\n\
+                        Transfer-Encoding: chunked, identity\r\n\r\n\
+                        a\r\n\
+                        Hello World!\r\n\
+                        0\r\n\r\n";
+
+    const TEST_RESPONSE_TRANSFER_ENCODING_MULTIPLE: &[u8] = b"\
+                        HTTP/1.1 200 OK\r\n\
+                        Transfer-Encoding: chunked, identity\r\n\r\n\
+                        a\r\n\
+                        Hello World!\r\n\
+                        0\r\n\r\n";
+
     #[test]
     fn test_parse_request() {
         let req = parse_request(TEST_REQUEST).unwrap();
@@ -496,4 +568,52 @@ mod tests {
 
         assert_eq!(value.span(), "{\"foo\": \"bar\"}");
     }
+
+    #[test]
+    fn test_parse_request_transfer_encoding_identity() {
+        let req = parse_request(TEST_REQUEST_TRANSFER_ENCODING_IDENTITY).unwrap();
+        assert_eq!(req.body.unwrap().span(), b"Hello World!".as_slice());
+    }
+
+    #[test]
+    fn test_parse_response_transfer_encoding_identity() {
+        let res = parse_response(TEST_RESPONSE_TRANSFER_ENCODING_IDENTITY).unwrap();
+        assert_eq!(res.body.unwrap().span(), b"Hello World!".as_slice());
+    }
+
+    #[test]
+    fn test_parse_request_transfer_encoding_chunked() {
+        let err = parse_request(TEST_REQUEST_TRANSFER_ENCODING_CHUNKED).unwrap_err();
+        assert!(matches!(err, ParseError(_)));
+        assert!(err
+            .to_string()
+            .contains("Transfer-Encoding other than identity not supported yet"));
+    }
+
+    #[test]
+    fn test_parse_response_transfer_encoding_chunked() {
+        let err = parse_response(TEST_RESPONSE_TRANSFER_ENCODING_CHUNKED).unwrap_err();
+        assert!(matches!(err, ParseError(_)));
+        assert!(err
+            .to_string()
+            .contains("Transfer-Encoding other than identity not supported yet"));
+    }
+
+    #[test]
+    fn test_parse_request_transfer_encoding_multiple() {
+        let err = parse_request(TEST_REQUEST_TRANSFER_ENCODING_MULTIPLE).unwrap_err();
+        assert!(matches!(err, ParseError(_)));
+        assert!(err
+            .to_string()
+            .contains("Transfer-Encoding other than identity not supported yet"));
+    }
+
+    #[test]
+    fn test_parse_response_transfer_encoding_multiple() {
+        let err = parse_response(TEST_RESPONSE_TRANSFER_ENCODING_MULTIPLE).unwrap_err();
+        assert!(matches!(err, ParseError(_)));
+        assert!(err
+            .to_string()
+            .contains("Transfer-Encoding other than identity not supported yet"));
+    }
 }