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).
This commit is contained in:
@@ -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",
|
b"HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n",
|
||||||
ParseError::Unsupported =>
|
ParseError::Unsupported =>
|
||||||
b"HTTP/1.1 411 Length Required\r\ncontent-length: 0\r\nconnection: close\r\n\r\n",
|
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
|
// Incomplete and Malformed both lead here; Incomplete shouldn't
|
||||||
// appear (read_head loops on it).
|
// appear (read_head loops on it).
|
||||||
_ =>
|
_ =>
|
||||||
|
|||||||
+94
-12
@@ -9,8 +9,10 @@
|
|||||||
//! - No body header — empty body.
|
//! - No body header — empty body.
|
||||||
//! - `Transfer-Encoding: chunked` (HTTP/1.1) — flagged in `ParsedHead`;
|
//! - `Transfer-Encoding: chunked` (HTTP/1.1) — flagged in `ParsedHead`;
|
||||||
//! the connection actor decodes incrementally (`read_chunked_body`).
|
//! the connection actor decodes incrementally (`read_chunked_body`).
|
||||||
//! Chunked + Content-Length together, or chunked on HTTP/1.0, is
|
//! TE is 1.1-only and overrides Content-Length: TE on HTTP/1.0, or TE
|
||||||
//! Malformed (request-smuggling ambiguity; RFC 7230 §3.3.3).
|
//! 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};
|
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
|
/// (chunked decoding landed in v0.3); kept for future unsupported
|
||||||
/// framings. Connection actor responds 411 + close.
|
/// framings. Connection actor responds 411 + close.
|
||||||
Unsupported,
|
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<ParsedHead, ParseErr
|
|||||||
let mut host_count = 0usize;
|
let mut host_count = 0usize;
|
||||||
let mut host_ok = true;
|
let mut host_ok = true;
|
||||||
let mut cl_count = 0usize;
|
let mut cl_count = 0usize;
|
||||||
|
let mut te_present = false;
|
||||||
|
let mut te_codings: Vec<String> = Vec::new();
|
||||||
|
|
||||||
for h in req.headers.iter() {
|
for h in req.headers.iter() {
|
||||||
let name_lower = h.name.to_ascii_lowercase();
|
let name_lower = h.name.to_ascii_lowercase();
|
||||||
@@ -121,10 +129,17 @@ pub fn parse_head(buf: &[u8], max_headers: usize) -> Result<ParsedHead, ParseErr
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
"transfer-encoding" => {
|
"transfer-encoding" => {
|
||||||
// We only care whether it includes "chunked". Multiple codings
|
// Collect the ordered coding list across any number of TE
|
||||||
// can appear; chunked is the only one we'd need to decode.
|
// headers; finality/known-ness is decided post-loop. Empty
|
||||||
if value.to_ascii_lowercase().split(',').any(|t| t.trim() == "chunked") {
|
// list elements (legacy `#rule`, e.g. a trailing comma) are
|
||||||
chunked = true;
|
// 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" => {
|
"connection" => {
|
||||||
@@ -163,14 +178,38 @@ pub fn parse_head(buf: &[u8], max_headers: usize) -> Result<ParsedHead, ParseErr
|
|||||||
return Err(ParseError::BadContentLength);
|
return Err(ParseError::BadContentLength);
|
||||||
}
|
}
|
||||||
|
|
||||||
if chunked {
|
// Transfer-Encoding (RFC 9112 §6.1/§6.3). TE is a 1.1 mechanism and
|
||||||
// Transfer-Encoding is an HTTP/1.1 mechanism; a 1.0 request
|
// overrides Content-Length; only `chunked` is implemented here.
|
||||||
// carrying it is malformed. And a request carrying BOTH a
|
if te_present {
|
||||||
// Content-Length and TE: chunked is the classic request-smuggling
|
// TE on HTTP/1.0 is malformed (no 1.0 chunked).
|
||||||
// ambiguity — RFC 7230 §3.3.3 lets a server reject it, and we do.
|
if version == HttpVersion::Http10 {
|
||||||
if version == HttpVersion::Http10 || content_length.is_some() {
|
|
||||||
return Err(ParseError::Malformed);
|
return Err(ParseError::Malformed);
|
||||||
}
|
}
|
||||||
|
// TE together with Content-Length is the classic smuggling
|
||||||
|
// ambiguity; TE overrides CL and we reject rather than forward.
|
||||||
|
if content_length.is_some() {
|
||||||
|
return Err(ParseError::Malformed);
|
||||||
|
}
|
||||||
|
// A Transfer-Encoding header that carries no coding frames nothing.
|
||||||
|
if te_codings.is_empty() {
|
||||||
|
return Err(ParseError::Malformed);
|
||||||
|
}
|
||||||
|
|
||||||
|
let last_is_chunked = te_codings.last().map(String::as_str) == Some("chunked");
|
||||||
|
let has_chunked = te_codings.iter().any(|c| c == "chunked");
|
||||||
|
|
||||||
|
if has_chunked && !last_is_chunked {
|
||||||
|
// chunked present but not final: body length isn't reliably
|
||||||
|
// determinable -> 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:
|
// Keep-alive logic, RFC 7230 §6.3:
|
||||||
@@ -540,6 +579,49 @@ mod tests {
|
|||||||
assert_eq!(head.content_length, Some(5));
|
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]
|
#[test]
|
||||||
fn serialise_basic_200() {
|
fn serialise_basic_200() {
|
||||||
let conn = Conn::new().put_status(200).put_body("hi");
|
let conn = Conn::new().put_status(200).put_body("hi");
|
||||||
|
|||||||
Reference in New Issue
Block a user