Skip to content

Reject CR in quoted-string parameter values per RFC 7230 - #28

Merged
picoHz merged 1 commit into
picoHz:mainfrom
binggao1230:fix/quoted-string-cr-ctl-filter
Jul 25, 2026
Merged

Reject CR in quoted-string parameter values per RFC 7230#28
picoHz merged 1 commit into
picoHz:mainfrom
binggao1230:fix/quoted-string-cr-ctl-filter

Conversation

@binggao1230

Copy link
Copy Markdown
Contributor

Problem

parse_quoted_value special-cases '\n' (LF) and returns Err(InvalidParamValue), but the catch-all _ => () arm accepts '\r' (CR) and all other CTLs as qdtext -- an incomplete control-character filter. Per RFC 7230 section 3.2.6, qdtext excludes all CTLs except HTAB (CR is forbidden); per RFC 2045 section 5.1 qtext explicitly excludes CR.

Because the parser is zero-copy, Display/to_string round-trips the raw CR byte verbatim: text/plain; x="foo\rbar" parses to Ok and re-serializes with the raw CR intact -- a CRLF/header-splitting vector when a parsed MediaType is written into a response header.

Evidence

  • mediatype: text/plain; x="foo\rbar" -> Ok, to_string() preserves raw CR.
  • Go stdlib mime.ParseMediaType -> REJECT ("mime: invalid media parameter").
  • Asymmetry: mediatype rejects x="foo\nbar" (LF) but accepts x="foo\rbar" (CR); RFC treats CR and LF identically, so the split is unambiguously the incomplete filter.

Fix

Widen the existing reject arm to also reject '\r': '\n' | '\r' => return Err(MediaTypeError::InvalidParamValue). Minimal, style-matching, with an RFC-citing comment. Catch-all and quoted-pair arms unchanged (out of scope).

Tests

cargo test --all-features: 42 passed (27 unit incl. new test + 1 codegen + 14 doc-tests). cargo fmt --check clean on touched file. Clippy: 0 new warnings/errors vs HEAD. New test parse_quoted_value_ctl_filter (HTAB/SP/VCHAR valid -> Ok; raw CR/LF/CRLF -> Err). Pre-fix: fails (Ok(...) when Err expected for CR); post-fix: pass. Harness re-run on fixed code: CR and LF both rejected symmetrically; 0 accepts of raw CR (was 1).

Out of scope: NUL/DEL among CTLs (Go mime is also lenient on those).

@picoHz
picoHz merged commit dda4887 into picoHz:main Jul 25, 2026
1 check failed
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.

2 participants