diff --git a/spansy/CHANGELOG.md b/spansy/CHANGELOG.md index ef88a6b..6c5e52c 100644 --- a/spansy/CHANGELOG.md +++ b/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 diff --git a/spansy/src/http/span.rs b/spansy/src/http/span.rs index a93b40e..becbb20 100644 --- a/spansy/src/http/span.rs +++ b/spansy/src/http/span.rs @@ -218,33 +218,48 @@ fn request_body_len(request: &Request) -> Result { // 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::>() + .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::() .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 { - // 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 { 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::>() + .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::() .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")); + } }