Improve handling of Transfer-Encoding errors

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

@@ -218,33 +218,48 @@ 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 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()
+        .map(|h| {
+            std::str::from_utf8(h.value.0.as_bytes())
+                .unwrap_or("")
+                .split(',')
+                .any(|v| v.trim() != "identity")
+        })
+        .unwrap_or(false)
     {
-        Err(ParseError(
-            "Transfer-Encoding not supported yet".to_string(),
-        ))
+        let bad_values = request
+            .headers_with_name("Transfer-Encoding")
+            .map(|h| std::str::from_utf8(h.value.0.as_bytes()).unwrap_or("{invalid utf-8}"))
+            .collect::<Vec<_>>()
+            .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.
+        // 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}")))
     } else {
-        // If this is a request message and none of the above are true, then the message body length is zero
+        // If this is a request message and none of the above are true, then the message
+        // body length is zero
         Ok(0)
     }
 }
 
 /// Calculates the length of the response body according to RFC 9112, section 6.
 fn response_body_len(response: &Response) -> Result<usize, ParseError> {
-    // Any response to a HEAD request and any response with a 1xx (Informational), 204 (No Content), or 304 (Not Modified)
-    // status code is always terminated by the first empty line after the header fields, regardless of the header fields
-    // present in the message, and thus cannot contain a message body or trailer section.
+    // Any response to a HEAD request and any response with a 1xx (Informational),
+    // 204 (No Content), or 304 (Not Modified) status code is always terminated
+    // by the first empty line after the header fields, regardless of the header
+    // fields present in the message, and thus cannot contain a message body or
+    // trailer section.
     match response
         .status
         .code
@@ -259,22 +274,36 @@ fn response_body_len(response: &Response) -> Result<usize, ParseError> {
     if response
         .headers_with_name("Transfer-Encoding")
         .next()
-        .is_some()
+        .map(|h| {
+            std::str::from_utf8(h.value.0.as_bytes())
+                .unwrap_or("")
+                .split(',')
+                .any(|v| v.trim() != "identity")
+        })
+        .unwrap_or(false)
     {
-        Err(ParseError(
-            "Transfer-Encoding not supported yet".to_string(),
-        ))
+        let bad_values = response
+            .headers_with_name("Transfer-Encoding")
+            .map(|h| std::str::from_utf8(h.value.0.as_bytes()).unwrap_or("{invalid utf-8}"))
+            .collect::<Vec<_>>()
+            .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.
+        // 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}")))
     } else {
-        // If this is a response message and none of the above are true, then there is no way to
-        // determine the length of the message body except by reading it until the connection is closed.
+        // If this is a response message and none of the above are true, then there is
+        // no way to determine the length of the message body except by reading
+        // it until the connection is closed.
 
-        // We currently consider this an error because we have no outer context information.
+        // We currently consider this an error because we have no outer context
+        // information.
         Err(ParseError(
             "A response with a body must contain either a Content-Length or Transfer-Encoding header".to_string(),
         ))
@@ -363,6 +392,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 +565,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"));
+    }
 }