Skip to content

Fix whitespace-only header line silently truncating headers (fixes #222)#223

Open
scadastrangelove wants to merge 1 commit into
seanmonstar:masterfrom
scadastrangelove:fix/whitespace-only-line-truncates-headers
Open

Fix whitespace-only header line silently truncating headers (fixes #222)#223
scadastrangelove wants to merge 1 commit into
seanmonstar:masterfrom
scadastrangelove:fix/whitespace-only-line-truncates-headers

Conversation

@scadastrangelove

Copy link
Copy Markdown

Fixes #222.

ParserConfig::allow_space_before_first_header_name(true) + a line consisting only of whitespace right
after the request/status-line silently dropped the entire header block: the parser returned
Ok(Status::Complete) with headers == [], leaving every real header (Host, Content-Length, ...)
unconsumed in the buffer.

The fix

After stripping the leading whitespace run, peek at the next byte before deciding to continue:

match bytes.peek() {
    None => return Ok(Status::Partial),
    Some(b'\r') | Some(b'\n') => break 'header Error::HeaderName,
    _ => {}
}
  • A whitespace-only line (next byte is \r/\n) now rejects with Error::HeaderName — the same error
    the non-leniency path already returns for a malformed first byte, rather than being silently absorbed
    as the terminating blank line.
  • If there's no more data yet (peek() returns None), returns Status::Partial so the streaming/
    incremental case is unaffected — a caller that hasn't yet read the byte deciding whitespace-only vs. a
    real header isn't penalized.
  • The documented, tested, non-degenerate case (space followed by a real header) is untouched.

Tests added

  • test_allow_space_before_first_header_name_rejects_whitespace_only_line — both request and response
    variants of the whitespace-only-line case now return Err(HeaderName) instead of silently truncating.
  • test_allow_space_before_first_header_name_partial_after_whitespace — a buffer ending right after the
    whitespace run still returns Status::Partial, confirming the fix doesn't regress incremental parsing.

Validation

  • Full test suite: 99 → 101 tests, all green (cargo test --release).
  • Existing test_allow_response_response_with_space_before_first_header (the documented non-degenerate
    case) unaffected.
  • Manually re-verified both PoCs from the issue now return Err(HeaderName) instead of
    Ok(Complete)/headers==[].

Found & fixed with the rust-in-peace security pipeline.

…monstar#222)

`ParserConfig::allow_space_before_first_header_name(true)` + a line
consisting only of whitespace right after the request/status-line
silently dropped the entire header block: the parser returned
Ok(Status::Complete) with headers == [], leaving every real header
unconsumed in the buffer for the caller to misinterpret as body/
next-request bytes.

After stripping the leading whitespace run, peek at the next byte
before continuing: whitespace-only (next byte is CR/LF) now rejects
with the same Error::HeaderName the non-leniency path already uses;
no more buffered data returns Status::Partial (preserves streaming
correctness). The documented, tested, non-degenerate case (space then
a real header) is unaffected.

Adds two regression tests (whitespace-only rejection on both request
and response sides; the Partial streaming case).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ParserConfig::allow_space_before_first_header_name silently drops all headers when the first header line is whitespace-only

1 participant