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..3b374a7 100644 --- a/spansy/src/http/span.rs +++ b/spansy/src/http/span.rs @@ -213,6 +213,41 @@ 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, 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, 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, +) -> Result, 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 { // The presence of a message body in a request is signaled by a Content-Length @@ -220,20 +255,18 @@ fn request_body_len(request: &Request) -> Result { // 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::() - .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 { _ => {} } - 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")); + } }