From b86c64d49090641569f2f4b0af16f14959109462 Mon Sep 17 00:00:00 2001 From: "Claude (sandbox)" Date: Sun, 9 Aug 2026 08:02:04 +0000 Subject: [PATCH] feat(parser): strict Transfer-Encoding framing; unknown coding -> 501 The TE arm set chunked whenever the token appeared anywhere in the value, so 'chunked, gzip' (chunked not final) was accepted and an unknown coding like 'bogus' was treated as no-body (h1spec #18/#19 -> 404). Collect the ordered coding list across all TE headers and decide post-loop: TE on HTTP/1.0 or TE+Content-Length -> 400 (the CL check now covers ANY TE, not just chunked, closing the old TE:unknown + CL smuggling gap); chunked present but not final -> 400; any coding other than chunked -> 501 via a new UnknownTransferCoding variant (emit_error_response gains the 501 arm); only a sole final chunked sets the flag. Tests cover each branch. Note: the dead Unsupported/411 variant is left as-is (separate cleanup). --- src/conn_actor.rs | 2 + src/parser.rs | 106 ++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 96 insertions(+), 12 deletions(-) diff --git a/src/conn_actor.rs b/src/conn_actor.rs index 3c1b3a2..87b3c7e 100644 --- a/src/conn_actor.rs +++ b/src/conn_actor.rs @@ -773,6 +773,8 @@ fn emit_error_response(fd: RawFd, err: &ParseError, deadline: Instant) { b"HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", ParseError::Unsupported => b"HTTP/1.1 411 Length Required\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", + ParseError::UnknownTransferCoding => + b"HTTP/1.1 501 Not Implemented\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", // Incomplete and Malformed both lead here; Incomplete shouldn't // appear (read_head loops on it). _ => diff --git a/src/parser.rs b/src/parser.rs index 364c878..7b8a13e 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -9,8 +9,10 @@ //! - No body header — empty body. //! - `Transfer-Encoding: chunked` (HTTP/1.1) — flagged in `ParsedHead`; //! the connection actor decodes incrementally (`read_chunked_body`). -//! Chunked + Content-Length together, or chunked on HTTP/1.0, is -//! Malformed (request-smuggling ambiguity; RFC 7230 §3.3.3). +//! TE is 1.1-only and overrides Content-Length: TE on HTTP/1.0, or TE +//! together with a Content-Length, is Malformed (400). `chunked` must be +//! the final coding (non-final -> 400); any other coding is unimplemented +//! (-> 501). Only a sole final `chunked` sets the flag (RFC 9112 §6.1/§6.3). use crate::conn::{Body, Conn, HeaderMap, HttpVersion, Method, RespBody}; @@ -33,6 +35,10 @@ pub enum ParseError { /// (chunked decoding landed in v0.3); kept for future unsupported /// framings. Connection actor responds 411 + close. Unsupported, + /// `Transfer-Encoding` names a transfer coding we don't implement + /// (`chunked` is the only one urus decodes). Connection actor responds + /// 501 Not Implemented + close (RFC 9112 §6.1, §7). + UnknownTransferCoding, } // --------------------------------------------------------------------------- @@ -103,6 +109,8 @@ pub fn parse_head(buf: &[u8], max_headers: usize) -> Result = Vec::new(); for h in req.headers.iter() { let name_lower = h.name.to_ascii_lowercase(); @@ -121,10 +129,17 @@ pub fn parse_head(buf: &[u8], max_headers: usize) -> Result { - // We only care whether it includes "chunked". Multiple codings - // can appear; chunked is the only one we'd need to decode. - if value.to_ascii_lowercase().split(',').any(|t| t.trim() == "chunked") { - chunked = true; + // Collect the ordered coding list across any number of TE + // headers; finality/known-ness is decided post-loop. Empty + // list elements (legacy `#rule`, e.g. a trailing comma) are + // skipped; a wholly empty value leaves te_codings empty and + // is caught below. + te_present = true; + for coding in value.split(',') { + let c = coding.trim().to_ascii_lowercase(); + if !c.is_empty() { + te_codings.push(c); + } } } "connection" => { @@ -163,14 +178,38 @@ pub fn parse_head(buf: &[u8], max_headers: usize) -> Result 400. + return Err(ParseError::Malformed); + } + if te_codings.iter().any(|c| c != "chunked") { + // Some coding we don't implement (chunked is the only decodable + // one). Whether or not chunked is final, we can't apply it -> 501. + return Err(ParseError::UnknownTransferCoding); + } + // Sole, final `chunked`: the connection actor decodes the body. + chunked = true; } // Keep-alive logic, RFC 7230 §6.3: @@ -540,6 +579,49 @@ mod tests { assert_eq!(head.content_length, Some(5)); } + // --- Transfer-Encoding (RFC 9112 §6.1/§6.3) ------------------------- + + #[test] + fn parse_non_final_chunked_is_malformed() { + // chunked must be the FINAL coding. + let req = b"POST / HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: chunked, gzip\r\n\r\n"; + match parse_head(req, 64) { + Err(ParseError::Malformed) => {} + _ => panic!("expected Malformed for non-final chunked"), + } + } + + #[test] + fn parse_unknown_transfer_coding_is_unimplemented() { + // A coding urus doesn't implement, no chunked at all -> 501. + let req = b"POST / HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: nonsense\r\n\r\n"; + match parse_head(req, 64) { + Err(ParseError::UnknownTransferCoding) => {} + _ => panic!("expected UnknownTransferCoding for unknown coding"), + } + } + + #[test] + fn parse_gzip_then_chunked_is_unimplemented() { + // chunked IS final, but gzip is still a coding we can't apply -> 501. + let req = b"POST / HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: gzip, chunked\r\n\r\n"; + match parse_head(req, 64) { + Err(ParseError::UnknownTransferCoding) => {} + _ => panic!("expected UnknownTransferCoding for gzip,chunked"), + } + } + + #[test] + fn parse_te_with_content_length_is_malformed() { + // ANY Transfer-Encoding + Content-Length -> reject (smuggling), + // not only chunked+CL. This closes the old TE:unknown + CL gap. + let req = b"POST / HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: bogus\r\nContent-Length: 5\r\n\r\nhello"; + match parse_head(req, 64) { + Err(ParseError::Malformed) => {} + _ => panic!("expected Malformed for TE + CL"), + } + } + #[test] fn serialise_basic_200() { let conn = Conn::new().put_status(200).put_body("hi");