From cf2cc62129196e73af4a4277c36e7c3ff71f02b6 Mon Sep 17 00:00:00 2001
From: Athul Nambiar <108534940+athul-22@users.noreply.github.com>
Date: Mon, 24 Aug 2026 19:43:52 +0530
Subject: [PATCH 1/2] fix(headers): respect quoted strings when splitting
 header parameters

Three structured header values were parsed by splitting on the separator
and stripping quote characters, with no tracking of quoted strings. A
quoted parameter value was therefore not opaque: a separator inside it
ended the parameter, and the text after it was read as a parameter or
directive of its own.

`Content-Disposition` is the one that matters most:

    attachment; filename="a;filename*=UTF-8''evil.exe"

RFC 6266 makes the whole quoted string the `filename`, with no
`filename*` present at all. Splitting on the inner `;` surfaced a
`filename*`, which is preferred over the plain name, so the download was
saved as `evil.exe`. Echoing a user-supplied filename into this header is
common, and quoting the name does not help because the smuggled
parameter travels inside the quotes. The same function already used a
real RFC 6266 parser for the plain filename, so its detection scan and
its extraction disagreed.

`Content-Type` lost or swapped the document's transport encoding:

    text/html; boundary="; charset=gbk"   read gbk, declares no charset
    text/html; charset="utf\-8"           read `utf\-8`, an invalid label

The second dropped the charset entirely and fell back to windows-1252,
which is mojibake for non-ASCII content.

`Cache-Control` read a quoted field list's own text as directives, so
`community="x, no-store, y"` stopped a cacheable response being stored.
That one fails safe and is only a lost cache hit, but it is the same
defect.

`moli-header-field` already exists for this job and already backs
`Content-Disposition` parsing in `moli-multipart`; these three call sites
predate it. Add `split_outside_quoted_strings` and
`unquote_parameter_value` there and move all three onto them.

Parsing the plain filename directly also removes the last use of the
`content_disposition` crate, which splits the same way and would have
reintroduced the truncation, so that dependency is dropped.

Byte indices land only on ASCII bytes. A UTF-8 continuation byte can
never equal `"`, `\` or a separator, so scanning stays on character
boundaries; a multibyte case is covered by a test.

Existing tolerances are preserved deliberately. For `Content-Type` that
means whitespace around `=`, apostrophe delimiters, an unterminated
quoted string, uppercase parameter names and a trailing `;`; eighteen
header forms were compared before and after, and only the three defects
above change.

Verified on aarch64-darwin per AGENTS.md: `cargo fmt --all` and
`cargo clippy --workspace --all-targets --all-features -- -D warnings`
are clean; moli-header-field 13, moli-encoding 56, moli-http-cache 49
and the moli-protocol downloads suite 29 all pass.

Closes #195
---
 Cargo.lock                          |  4 +-
 moli-encoding/Cargo.toml            |  1 +
 moli-encoding/src/labels.rs         | 24 +++++---
 moli-encoding/src/tests.rs          | 68 +++++++++++++++++++++
 moli-header-field/src/lib.rs        |  2 +
 moli-header-field/src/parameters.rs | 67 +++++++++++++++++++++
 moli-header-field/src/tests.rs      | 68 +++++++++++++++++++++
 moli-http-cache/Cargo.toml          |  1 +
 moli-http-cache/src/policy.rs       | 91 ++++++++++++++++++++++++-----
 moli-protocol/Cargo.toml            |  2 +-
 moli-protocol/src/conn/downloads.rs | 62 +++++++++++++++-----
 11 files changed, 353 insertions(+), 37 deletions(-)
 create mode 100644 moli-header-field/src/parameters.rs

diff --git a/Cargo.lock b/Cargo.lock
index ea7b5f060..630937d74 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2161,6 +2161,7 @@ version = "0.1.0"
 dependencies = [
  "encoding_rs",
  "moli-charset-parser",
+ "moli-header-field",
 ]
 
 [[package]]
@@ -2231,6 +2232,7 @@ version = "0.1.0"
 dependencies = [
  "anyhow",
  "httpdate",
+ "moli-header-field",
  "rustc-hash",
  "serde",
  "serde_json",
@@ -2409,7 +2411,6 @@ dependencies = [
  "axum",
  "base64 0.22.1",
  "chromiumoxide_cdp",
- "content_disposition",
  "data-url",
  "http",
  "indexmap",
@@ -2421,6 +2422,7 @@ dependencies = [
  "moli-css-parse",
  "moli-encoding",
  "moli-fetch",
+ "moli-header-field",
  "moli-image",
  "moli-page-types",
  "moli-protocol-cdp",
diff --git a/moli-encoding/Cargo.toml b/moli-encoding/Cargo.toml
index d4b8edf0d..2a49e0f20 100644
--- a/moli-encoding/Cargo.toml
+++ b/moli-encoding/Cargo.toml
@@ -7,6 +7,7 @@ edition = "2024"
 [dependencies]
 encoding_rs = "0.8"
 moli-charset-parser = { path = "../moli-charset-parser" }
+moli-header-field = { path = "../moli-header-field" }
 
 [lints]
 workspace = true
diff --git a/moli-encoding/src/labels.rs b/moli-encoding/src/labels.rs
index f0a6df86f..d29a98ee0 100644
--- a/moli-encoding/src/labels.rs
+++ b/moli-encoding/src/labels.rs
@@ -1,24 +1,30 @@
 use encoding_rs::Encoding;
+use moli_header_field::{split_outside_quoted_strings, unquote_parameter_value};
 
 pub fn encoding_for_label(label: &str) -> Option<&'static Encoding> {
     Encoding::for_label(label.trim().as_bytes())
 }
 
+/// The `charset` parameter of a `Content-Type` value.
+///
+/// Parameters are separated by the `;` characters outside a quoted string, and
+/// a quoted value has its quoting backslashes removed, so a `charset` written
+/// inside another parameter's quoted string is not read as a parameter here.
 pub fn charset_from_content_type(value: &str) -> Option<String> {
-    for parameter in value.split(';').skip(1) {
-        let parameter = parameter.trim();
-        let Some((name, value)) = parameter.split_once('=') else {
+    // The first segment is the media type itself, not a parameter.
+    for parameter in split_outside_quoted_strings(value, ';').into_iter().skip(1) {
+        let Some((name, parameter_value)) = parameter.split_once('=') else {
             continue;
         };
         if !name.trim().eq_ignore_ascii_case("charset") {
             continue;
         }
-        let value = value
-            .trim()
-            .trim_matches(|ch| ch == '"' || ch == '\'')
-            .trim();
-        if !value.is_empty() {
-            return Some(value.to_owned());
+        let charset = unquote_parameter_value(parameter_value.trim());
+        // Apostrophe delimiters are not a quoted string, but receivers have
+        // long tolerated them here, so keep stripping them.
+        let charset = charset.trim_matches(|ch| ch == '"' || ch == '\'').trim();
+        if !charset.is_empty() {
+            return Some(charset.to_owned());
         }
     }
     None
diff --git a/moli-encoding/src/tests.rs b/moli-encoding/src/tests.rs
index 91c4b7bf1..9c11e0345 100644
--- a/moli-encoding/src/tests.rs
+++ b/moli-encoding/src/tests.rs
@@ -665,3 +665,71 @@ fn bom_less_utf16be_truncated_not_detected() {
     assert_eq!(encoding, "windows-1252");
     assert_eq!(text, "\0<\0?");
 }
+
+#[test]
+fn header_charset_stays_inside_another_parameters_quoted_string() {
+    assert_eq!(
+        charset_from_content_type("text/html; boundary=\"; charset=gbk\""),
+        None
+    );
+    let headers = vec![(
+        "Content-Type".to_owned(),
+        "text/html; boundary=\"; charset=gbk\"".to_owned(),
+    )];
+    assert_eq!(
+        decode_html_document(b"<p>hi</p>", &headers).1,
+        "windows-1252"
+    );
+}
+
+#[test]
+fn header_charset_is_not_displaced_by_an_escaped_quote() {
+    assert_eq!(
+        charset_from_content_type("text/html; name=\"a\\\"; charset=gbk\"; charset=utf-8")
+            .as_deref(),
+        Some("utf-8")
+    );
+}
+
+#[test]
+fn header_charset_removes_quoting_backslashes() {
+    assert_eq!(
+        charset_from_content_type("text/html; charset=\"utf\\-8\"").as_deref(),
+        Some("utf-8")
+    );
+    let headers = vec![(
+        "Content-Type".to_owned(),
+        "text/html; charset=\"utf\\-8\"".to_owned(),
+    )];
+    assert_eq!(decode_html_document(b"<p>hi</p>", &headers).1, "UTF-8");
+}
+
+#[test]
+fn header_charset_keeps_its_existing_tolerances() {
+    for header in [
+        "text/html; charset=utf-8",
+        "text/html;charset=utf-8",
+        "TEXT/HTML; CHARSET=UTF-8",
+        "text/html; charset = utf-8 ",
+        "text/html; charset=\"utf-8\"",
+        "text/html;charset=utf-8;",
+        "text/html; charset='utf-8'",
+        "text/html; charset=\"utf-8",
+    ] {
+        assert_eq!(
+            charset_from_content_type(header)
+                .as_deref()
+                .and_then(encoding_for_label)
+                .map(Encoding::name),
+            Some("UTF-8"),
+            "header={header}"
+        );
+    }
+    assert_eq!(charset_from_content_type("text/html"), None);
+    assert_eq!(charset_from_content_type("text/html; charset="), None);
+    assert_eq!(charset_from_content_type("charset=utf-8"), None);
+    assert_eq!(
+        charset_from_content_type("text/html; charset=gbk; boundary=x").as_deref(),
+        Some("gbk")
+    );
+}
diff --git a/moli-header-field/src/lib.rs b/moli-header-field/src/lib.rs
index c909e2561..cfd0c91d0 100644
--- a/moli-header-field/src/lib.rs
+++ b/moli-header-field/src/lib.rs
@@ -10,8 +10,10 @@
 //! also admits MIME `tspecials` other than space, semicolon, and quote, matching
 //! Blink rather than defining another standards-compliance mode.
 
+mod parameters;
 mod tokenizer;
 
+pub use parameters::{split_outside_quoted_strings, unquote_parameter_value};
 pub use tokenizer::{HeaderFieldTokenMode, HeaderFieldTokenizer};
 
 #[cfg(test)]
diff --git a/moli-header-field/src/parameters.rs b/moli-header-field/src/parameters.rs
new file mode 100644
index 000000000..0eef50e86
--- /dev/null
+++ b/moli-header-field/src/parameters.rs
@@ -0,0 +1,67 @@
+use std::borrow::Cow;
+
+/// Splits `value` on every `separator` that sits outside a quoted string.
+///
+/// Structured header field values carry their parameters after a separator —
+/// `;` after a media type, `,` between list members — but a quoted string may
+/// contain that separator without ending the parameter it belongs to. Plain
+/// splitting lets such a separator escape the quoted string, so text that is
+/// part of one parameter's value is read as a parameter of its own.
+///
+/// `separator` must be an ASCII character. A UTF-8 continuation byte can never
+/// equal one, so every returned slice falls on a character boundary.
+pub fn split_outside_quoted_strings(value: &str, separator: char) -> Vec<&str> {
+    debug_assert!(
+        separator.is_ascii(),
+        "a non-ASCII separator cannot be matched bytewise"
+    );
+    let separator = separator as u8;
+    let bytes = value.as_bytes();
+    let mut segments = Vec::new();
+    let mut segment_start = 0;
+    let mut inside_quotes = false;
+    let mut index = 0;
+
+    while index < bytes.len() {
+        let byte = bytes[index];
+        if byte == b'"' {
+            inside_quotes = !inside_quotes;
+        } else if byte == b'\\' && inside_quotes {
+            // Step over the escaped byte so a quoted `\"` does not close the
+            // string it appears in.
+            index += 1;
+        } else if byte == separator && !inside_quotes {
+            segments.push(&value[segment_start..index]);
+            segment_start = index + 1;
+        }
+        index += 1;
+    }
+
+    segments.push(&value[segment_start.min(value.len())..]);
+    segments
+}
+
+/// Removes a quoted string's delimiters and its quoting backslashes.
+///
+/// A value that is not quoted is returned as-is. An unterminated quoted string
+/// yields everything that was read rather than discarding the parameter, which
+/// is how receivers generally treat one.
+pub fn unquote_parameter_value(raw: &str) -> Cow<'_, str> {
+    let Some(quoted) = raw.strip_prefix('"') else {
+        return Cow::Borrowed(raw);
+    };
+    let mut unquoted = String::with_capacity(quoted.len());
+    let mut characters = quoted.chars();
+    while let Some(character) = characters.next() {
+        match character {
+            '"' => return Cow::Owned(unquoted),
+            '\\' => {
+                if let Some(escaped) = characters.next() {
+                    unquoted.push(escaped);
+                }
+            }
+            _ => unquoted.push(character),
+        }
+    }
+    Cow::Owned(unquoted)
+}
diff --git a/moli-header-field/src/tests.rs b/moli-header-field/src/tests.rs
index 948c1ef8a..f7b8a7189 100644
--- a/moli-header-field/src/tests.rs
+++ b/moli-header-field/src/tests.rs
@@ -122,3 +122,71 @@ fn consume_before_any_match_stops_without_consuming_the_separator() {
         Some("beta")
     );
 }
+
+#[test]
+fn separators_inside_a_quoted_string_do_not_split() {
+    assert_eq!(
+        split_outside_quoted_strings("text/html; boundary=\"; charset=gbk\"", ';'),
+        vec!["text/html", " boundary=\"; charset=gbk\""]
+    );
+    assert_eq!(
+        split_outside_quoted_strings("private=\"Set-Cookie, X-Auth\", max-age=60", ','),
+        vec!["private=\"Set-Cookie, X-Auth\"", " max-age=60"]
+    );
+}
+
+#[test]
+fn an_escaped_quote_does_not_close_a_quoted_string() {
+    assert_eq!(
+        split_outside_quoted_strings("attachment; name=\"a\\\"; x=1\"; y=2", ';'),
+        vec!["attachment", " name=\"a\\\"; x=1\"", " y=2"]
+    );
+}
+
+#[test]
+fn splitting_without_quotes_matches_a_plain_split() {
+    for value in [
+        "text/html; charset=utf-8",
+        "a;b;c",
+        "",
+        ";",
+        "trailing;",
+        ";leading",
+    ] {
+        assert_eq!(
+            split_outside_quoted_strings(value, ';'),
+            value.split(';').collect::<Vec<_>>(),
+            "value={value}"
+        );
+    }
+}
+
+#[test]
+fn an_unterminated_quoted_string_runs_to_the_end() {
+    assert_eq!(
+        split_outside_quoted_strings("text/html; charset=\"gbk; q=1", ';'),
+        vec!["text/html", " charset=\"gbk; q=1"]
+    );
+}
+
+#[test]
+fn quoted_values_lose_their_delimiters_and_backslashes() {
+    assert_eq!(unquote_parameter_value("\"utf-8\""), "utf-8");
+    assert_eq!(unquote_parameter_value("\"utf\\-8\""), "utf-8");
+    assert_eq!(unquote_parameter_value("\"a\\\"b\""), "a\"b");
+    // Not a quoted string, so it is returned untouched.
+    assert_eq!(unquote_parameter_value("utf-8"), "utf-8");
+    assert_eq!(unquote_parameter_value("'utf-8'"), "'utf-8'");
+    // Unterminated keeps what was read.
+    assert_eq!(unquote_parameter_value("\"gbk"), "gbk");
+}
+
+#[test]
+fn multibyte_values_keep_character_boundaries() {
+    let value = "text/plain; name=\"café; x\"; charset=utf-8";
+    assert_eq!(
+        split_outside_quoted_strings(value, ';'),
+        vec!["text/plain", " name=\"café; x\"", " charset=utf-8"]
+    );
+    assert_eq!(unquote_parameter_value("\"caf\\é\""), "café");
+}
diff --git a/moli-http-cache/Cargo.toml b/moli-http-cache/Cargo.toml
index 594c9f80e..bd125a894 100644
--- a/moli-http-cache/Cargo.toml
+++ b/moli-http-cache/Cargo.toml
@@ -7,6 +7,7 @@ edition = "2024"
 [dependencies]
 anyhow = "1.0.100"
 httpdate = "1.0.3"
+moli-header-field = { path = "../moli-header-field" }
 serde = { version = "1.0.228", features = ["derive"] }
 serde_json = "1.0.145"
 rustc-hash = "2.1.1"
diff --git a/moli-http-cache/src/policy.rs b/moli-http-cache/src/policy.rs
index 972a54cfd..fd4c92222 100644
--- a/moli-http-cache/src/policy.rs
+++ b/moli-http-cache/src/policy.rs
@@ -1,4 +1,5 @@
 use httpdate::parse_http_date;
+use moli_header_field::{split_outside_quoted_strings, unquote_parameter_value};
 use url::Url;
 
 /// Cache storage and freshness policy derived from response headers.
@@ -20,11 +21,18 @@ pub fn response_cache_policy(headers: &[(String, String)]) -> HttpCacheResponseP
 
     for (name, value) in headers {
         if name.eq_ignore_ascii_case("cache-control") {
-            for directive in value.split(',').map(str::trim) {
+            // A quoted directive argument may contain `,`, as in
+            // `private="Set-Cookie, X-Auth"`, so splitting on every comma
+            // would read the argument's own text as further directives.
+            for directive in split_outside_quoted_strings(value, ',')
+                .into_iter()
+                .map(str::trim)
+            {
                 let (directive_name, directive_value) = directive
                     .split_once('=')
-                    .map(|(name, value)| (name.trim(), Some(value.trim().trim_matches('"'))))
+                    .map(|(name, value)| (name.trim(), Some(unquote_parameter_value(value.trim()))))
                     .unwrap_or((directive, None));
+                let directive_value = directive_value.as_deref();
                 if directive_name.eq_ignore_ascii_case("no-store")
                     || directive_name.eq_ignore_ascii_case("private")
                 {
@@ -139,7 +147,7 @@ fn cached_response_is_fresh_immutable_at(
         && headers
             .iter()
             .filter(|(name, _)| name.eq_ignore_ascii_case("cache-control"))
-            .flat_map(|(_, value)| value.split(','))
+            .flat_map(|(_, value)| split_outside_quoted_strings(value, ','))
             .map(str::trim)
             .any(|directive| {
                 directive
@@ -161,19 +169,22 @@ pub fn request_header_requires_validation(name: &str, value: &str) -> bool {
 }
 
 pub fn request_cache_control_requires_validation(value: &str) -> bool {
-    value.split(',').map(str::trim).any(|directive| {
-        let (name, value) = directive
-            .split_once('=')
-            .map(|(name, value)| (name.trim(), Some(value.trim().trim_matches('"'))))
-            .unwrap_or((directive, None));
-        name.eq_ignore_ascii_case("no-cache")
-            || (name.eq_ignore_ascii_case("max-age") && value == Some("0"))
-    })
+    split_outside_quoted_strings(value, ',')
+        .into_iter()
+        .map(str::trim)
+        .any(|directive| {
+            let (name, value) = directive
+                .split_once('=')
+                .map(|(name, value)| (name.trim(), Some(unquote_parameter_value(value.trim()))))
+                .unwrap_or((directive, None));
+            name.eq_ignore_ascii_case("no-cache")
+                || (name.eq_ignore_ascii_case("max-age") && value.as_deref() == Some("0"))
+        })
 }
 
 pub fn request_pragma_requires_validation(value: &str) -> bool {
-    value
-        .split(',')
+    split_outside_quoted_strings(value, ',')
+        .into_iter()
         .map(str::trim)
         .any(|directive| directive.eq_ignore_ascii_case("no-cache"))
 }
@@ -230,4 +241,58 @@ mod tests {
             Some(100)
         ));
     }
+
+    #[test]
+    fn quoted_directive_arguments_do_not_leak_further_directives() {
+        // RFC 9111 lets `private` and `no-cache` carry a quoted field list,
+        // and that list may contain commas. Splitting on every comma read the
+        // list's own text as further directives.
+        assert!(!request_cache_control_requires_validation(
+            "max-age=600, private=\"Set-Cookie, X-Auth\""
+        ));
+        assert!(request_cache_control_requires_validation(
+            "max-age=600, no-cache=\"Set-Cookie, X-Auth\""
+        ));
+        // A directive name appearing inside a quoted argument is not a
+        // directive of its own.
+        assert!(!request_cache_control_requires_validation(
+            "max-age=600, private=\"a, no-cache, b\""
+        ));
+    }
+
+    #[test]
+    fn response_policy_ignores_directives_inside_a_quoted_argument() {
+        let headers = vec![(
+            "Cache-Control".to_owned(),
+            "max-age=600, community=\"x, no-store, y\"".to_owned(),
+        )];
+
+        assert!(response_cache_policy(&headers).store);
+    }
+
+    #[test]
+    fn response_policy_still_honours_real_directives() {
+        for value in [
+            "no-store",
+            "private",
+            "max-age=0, no-store",
+            "no-store, max-age=600",
+        ] {
+            let headers = vec![("Cache-Control".to_owned(), value.to_owned())];
+            assert!(!response_cache_policy(&headers).store, "value={value}");
+        }
+        let headers = vec![(
+            "Cache-Control".to_owned(),
+            "private=\"Set-Cookie\"".to_owned(),
+        )];
+        assert!(!response_cache_policy(&headers).store);
+    }
+
+    #[test]
+    fn quoted_pragma_arguments_do_not_leak_directives() {
+        assert!(!request_pragma_requires_validation(
+            "token=\"a, no-cache, b\""
+        ));
+        assert!(request_pragma_requires_validation("no-cache"));
+    }
 }
diff --git a/moli-protocol/Cargo.toml b/moli-protocol/Cargo.toml
index 63bfa27e4..6315508e2 100644
--- a/moli-protocol/Cargo.toml
+++ b/moli-protocol/Cargo.toml
@@ -15,6 +15,7 @@ base64 = "0.22"
 indexmap = "2.6"
 moli-bounded-buffer = { path = "../moli-bounded-buffer" }
 moli-core = { path = "../moli-core" }
+moli-header-field = { path = "../moli-header-field" }
 moli-page-types = { path = "../moli-page-types" }
 moli-browser-profile = { path = "../moli-browser-profile" }
 moli-cookie-jar = { path = "../moli-cookie-jar" }
@@ -43,7 +44,6 @@ v8 = "146.8.0"
 time = "0.3"
 chromiumoxide_cdp = "0.9.1"
 data-url = "0.3.2"
-content_disposition = "0.4.0"
 moli-crypto = { path = "../moli-crypto" }
 http = "1"
 sanitize-filename = "=0.6.0"
diff --git a/moli-protocol/src/conn/downloads.rs b/moli-protocol/src/conn/downloads.rs
index c593a604a..a94886cb7 100644
--- a/moli-protocol/src/conn/downloads.rs
+++ b/moli-protocol/src/conn/downloads.rs
@@ -6,11 +6,11 @@ use std::{
     sync::Arc,
 };
 
-use content_disposition::parse_content_disposition;
 use http::HeaderName;
 use moli_core::network::ResourceRequestClient;
 use moli_core::page::RendererPendingDownloadActivation;
 use moli_fetch::{FetchCancelHandle, Request};
+use moli_header_field::{split_outside_quoted_strings, unquote_parameter_value};
 use moli_web_mime::response_headers_indicate_attachment_download;
 use parking_lot::Mutex;
 use sanitize_filename::Options;
@@ -1373,36 +1373,47 @@ fn filename_from_headers(headers: &[(String, String)]) -> Option<String> {
 }
 
 fn filename_from_content_disposition(value: &str) -> Option<String> {
+    let mut plain = None;
     let mut extended = None;
     let mut saw_extended = false;
-    let mut saw_plain = false;
 
-    for part in value.split(';').skip(1) {
+    // A `;` inside a quoted string does not start a new parameter. Splitting on
+    // every `;` let text inside a quoted `filename` be read as a parameter of
+    // its own, so a site that echoes an attacker-supplied name into the header
+    // could smuggle a `filename*` and choose the extension the file is saved
+    // under.
+    for part in split_outside_quoted_strings(value, ';').into_iter().skip(1) {
         let part = part.trim();
-        if let Some(raw) = part.strip_prefix("filename*=") {
+        if let Some(raw) = strip_parameter_name(part, "filename*") {
             saw_extended = true;
             extended = decode_extended_filename(raw);
-            continue;
-        }
-        if part.strip_prefix("filename=").is_some() {
-            saw_plain = true;
+        } else if let Some(raw) = strip_parameter_name(part, "filename") {
+            plain = Some(unquote_parameter_value(raw.trim()).into_owned());
         }
     }
 
     if extended.is_some() {
         return extended;
     }
-    if saw_extended && !saw_plain {
+    if saw_extended && plain.is_none() {
         return None;
     }
 
-    parse_content_disposition(value)
-        .params
-        .get("filename")
-        .and_then(|filename| non_empty_filename(filename))
+    plain
+        .as_deref()
+        .and_then(non_empty_filename)
         .map(sanitize_filename)
 }
 
+/// Strips a case-insensitive `name=` prefix from one parameter.
+fn strip_parameter_name<'a>(part: &'a str, name: &str) -> Option<&'a str> {
+    let rest = part
+        .get(..name.len())?
+        .eq_ignore_ascii_case(name)
+        .then(|| &part[name.len()..])?;
+    rest.trim_start().strip_prefix('=')
+}
+
 fn decode_extended_filename(raw: &str) -> Option<String> {
     let raw = raw.trim().trim_matches('"');
     let mut parts = raw.splitn(3, '\'');
@@ -1877,6 +1888,31 @@ mod tests {
         );
     }
 
+    #[test]
+    fn content_disposition_ignores_filename_star_inside_a_quoted_filename() {
+        // RFC 6266: the whole quoted string is the `filename` value, and there
+        // is no `filename*` parameter here at all. Reading the inner text as
+        // one let a site that echoes an attacker-supplied name into the header
+        // choose the extension the file is saved under.
+        let filename = filename_from_content_disposition(
+            "attachment; filename=\"a;filename*=UTF-8''evil.exe\"",
+        );
+
+        // The saved name is the quoted string itself, with `*` removed by
+        // the Windows-safe sanitizer rather than by the parameter scan.
+        assert_ne!(filename.as_deref(), Some("evil.exe"));
+        assert_eq!(filename.as_deref(), Some("a;filename=UTF-8''evil.exe"));
+    }
+
+    #[test]
+    fn content_disposition_still_reads_a_real_filename_star_after_a_quoted_filename() {
+        let filename = filename_from_content_disposition(
+            "attachment; filename=\"plain;name.txt\"; filename*=UTF-8''%E4%B8%AD%E6%96%87.txt",
+        );
+
+        assert_eq!(filename.as_deref(), Some("中文.txt"));
+    }
+
     #[test]
     fn content_disposition_prefers_filename_star_when_present() {
         let filename = filename_from_content_disposition(

From f9d24a48be430779e9141d936fea62aca7276783 Mon Sep 17 00:00:00 2001
From: Athul Nambiar <108534940+athul-22@users.noreply.github.com>
Date: Tue, 25 Aug 2026 22:15:18 +0530
Subject: [PATCH 2/2] fix(headers): start quoting only at a parameter value

Review on #196 found three regressions in the shared helper.

The splitter toggled quoted-string mode on every `"`, including one that
does not open a parameter value. A stray quote therefore swallowed the
rest of the field and hid the parameters behind it, so
`text/html;";charset=gbk` stopped resolving to gbk. Quoting now begins
only at the first non-whitespace character after `=`, which is where a
parameter value can start; a `"` anywhere else is ordinary data.

Unquoting dropped a trailing `\` that had nothing to escape, which turned
a malformed value into a well-formed one: `max-age="31536000\` became a
one-year freshness lifetime. The trailing backslash is now kept, matching
the Fetch quoted-string algorithm.

The `Content-Type` caller trimmed `"` from the unquoted result, but the
delimiters are already gone by then, so any remaining quote is data from
an escaped quote. `charset="utf-8\""` is the label `utf-8"`, which is not
a valid encoding, and trimming manufactured a valid one. The legacy
apostrophe tolerance only ever applied to unquoted values, so it is now
applied only to those.

Each case has a regression test, including the whitespace-before-quote
form that must still open a quoted value.
---
 moli-encoding/src/labels.rs         | 19 ++++++---
 moli-encoding/src/tests.rs          | 21 ++++++++++
 moli-header-field/src/parameters.rs | 64 ++++++++++++++++++++---------
 moli-header-field/src/tests.rs      | 34 +++++++++++++++
 moli-http-cache/src/policy.rs       | 11 +++++
 5 files changed, 125 insertions(+), 24 deletions(-)

diff --git a/moli-encoding/src/labels.rs b/moli-encoding/src/labels.rs
index d29a98ee0..99973bac1 100644
--- a/moli-encoding/src/labels.rs
+++ b/moli-encoding/src/labels.rs
@@ -19,12 +19,21 @@ pub fn charset_from_content_type(value: &str) -> Option<String> {
         if !name.trim().eq_ignore_ascii_case("charset") {
             continue;
         }
-        let charset = unquote_parameter_value(parameter_value.trim());
-        // Apostrophe delimiters are not a quoted string, but receivers have
-        // long tolerated them here, so keep stripping them.
-        let charset = charset.trim_matches(|ch| ch == '"' || ch == '\'').trim();
+        let parameter_value = parameter_value.trim();
+        let charset = if parameter_value.starts_with('"') {
+            // Delimiters are already gone, so any `"` left is data produced by
+            // an escaped quote and must not be trimmed away.
+            unquote_parameter_value(parameter_value).into_owned()
+        } else {
+            // Apostrophe delimiters are not a quoted string, but receivers
+            // have long tolerated them here, so keep stripping them.
+            parameter_value
+                .trim_matches(|ch| ch == '"' || ch == '\'')
+                .trim()
+                .to_owned()
+        };
         if !charset.is_empty() {
-            return Some(charset.to_owned());
+            return Some(charset);
         }
     }
     None
diff --git a/moli-encoding/src/tests.rs b/moli-encoding/src/tests.rs
index 9c11e0345..d305a0b29 100644
--- a/moli-encoding/src/tests.rs
+++ b/moli-encoding/src/tests.rs
@@ -733,3 +733,24 @@ fn header_charset_keeps_its_existing_tolerances() {
         Some("gbk")
     );
 }
+
+#[test]
+fn header_charset_recovers_after_a_stray_quote() {
+    // WPT MIME case: the `"` does not open a parameter value, so the following
+    // `;` still separates parameters and the real charset is found.
+    assert_eq!(
+        charset_from_content_type("text/html;\";charset=gbk").as_deref(),
+        Some("gbk")
+    );
+}
+
+#[test]
+fn header_charset_keeps_an_escaped_quote_as_data() {
+    // The quoted value is `utf-8"`, which is not a valid label. Trimming the
+    // data quote would manufacture a valid one.
+    let label = charset_from_content_type("text/html; charset=\"utf-8\\\"\"")
+        .expect("a parameter value is present");
+
+    assert_eq!(label, "utf-8\"");
+    assert!(encoding_for_label(&label).is_none());
+}
diff --git a/moli-header-field/src/parameters.rs b/moli-header-field/src/parameters.rs
index 0eef50e86..436955dcb 100644
--- a/moli-header-field/src/parameters.rs
+++ b/moli-header-field/src/parameters.rs
@@ -1,12 +1,18 @@
 use std::borrow::Cow;
 
-/// Splits `value` on every `separator` that sits outside a quoted string.
+/// Splits `value` on every `separator` that is not inside a quoted parameter
+/// value.
 ///
 /// Structured header field values carry their parameters after a separator —
-/// `;` after a media type, `,` between list members — but a quoted string may
+/// `;` after a media type, `,` between list members — but a quoted value may
 /// contain that separator without ending the parameter it belongs to. Plain
-/// splitting lets such a separator escape the quoted string, so text that is
-/// part of one parameter's value is read as a parameter of its own.
+/// splitting lets such a separator escape the quoted value, so text that is
+/// part of one parameter is read as a parameter of its own.
+///
+/// Quoting only begins where a parameter value begins, which is the first
+/// non-whitespace character after `=`. A `"` anywhere else is ordinary data
+/// and does not open a quoted string, so a stray quote cannot swallow the rest
+/// of the field and hide the parameters behind it.
 ///
 /// `separator` must be an ASCII character. A UTF-8 continuation byte can never
 /// equal one, so every returned slice falls on a character boundary.
@@ -19,20 +25,40 @@ pub fn split_outside_quoted_strings(value: &str, separator: char) -> Vec<&str> {
     let bytes = value.as_bytes();
     let mut segments = Vec::new();
     let mut segment_start = 0;
+    let mut at_value_start = false;
     let mut inside_quotes = false;
     let mut index = 0;
 
     while index < bytes.len() {
         let byte = bytes[index];
-        if byte == b'"' {
-            inside_quotes = !inside_quotes;
-        } else if byte == b'\\' && inside_quotes {
-            // Step over the escaped byte so a quoted `\"` does not close the
-            // string it appears in.
+
+        if inside_quotes {
+            if byte == b'\\' {
+                // Step over the escaped byte so a quoted `\"` does not close
+                // the value.
+                index += 2;
+                continue;
+            }
+            if byte == b'"' {
+                inside_quotes = false;
+            }
             index += 1;
-        } else if byte == separator && !inside_quotes {
+            continue;
+        }
+
+        if byte == separator {
             segments.push(&value[segment_start..index]);
             segment_start = index + 1;
+            at_value_start = false;
+        } else if byte == b'=' {
+            at_value_start = true;
+        } else if byte == b'"' && at_value_start {
+            inside_quotes = true;
+            at_value_start = false;
+        } else if !matches!(byte, b' ' | b'\t') || !at_value_start {
+            // Whitespace between `=` and the value keeps the value still to
+            // come; anything else means the value was not quoted.
+            at_value_start = false;
         }
         index += 1;
     }
@@ -41,11 +67,12 @@ pub fn split_outside_quoted_strings(value: &str, separator: char) -> Vec<&str> {
     segments
 }
 
-/// Removes a quoted string's delimiters and its quoting backslashes.
+/// Removes a quoted value's delimiters and its quoting backslashes.
 ///
-/// A value that is not quoted is returned as-is. An unterminated quoted string
-/// yields everything that was read rather than discarding the parameter, which
-/// is how receivers generally treat one.
+/// A value that is not quoted is returned as-is. An unterminated quoted value
+/// yields everything that was read rather than discarding the parameter, and a
+/// trailing `\` with nothing to escape is kept, so a malformed value is not
+/// quietly turned into a well-formed one.
 pub fn unquote_parameter_value(raw: &str) -> Cow<'_, str> {
     let Some(quoted) = raw.strip_prefix('"') else {
         return Cow::Borrowed(raw);
@@ -55,11 +82,10 @@ pub fn unquote_parameter_value(raw: &str) -> Cow<'_, str> {
     while let Some(character) = characters.next() {
         match character {
             '"' => return Cow::Owned(unquoted),
-            '\\' => {
-                if let Some(escaped) = characters.next() {
-                    unquoted.push(escaped);
-                }
-            }
+            '\\' => match characters.next() {
+                Some(escaped) => unquoted.push(escaped),
+                None => unquoted.push('\\'),
+            },
             _ => unquoted.push(character),
         }
     }
diff --git a/moli-header-field/src/tests.rs b/moli-header-field/src/tests.rs
index f7b8a7189..75fd6ec7e 100644
--- a/moli-header-field/src/tests.rs
+++ b/moli-header-field/src/tests.rs
@@ -190,3 +190,37 @@ fn multibyte_values_keep_character_boundaries() {
     );
     assert_eq!(unquote_parameter_value("\"caf\\é\""), "café");
 }
+
+#[test]
+fn a_quote_outside_a_parameter_value_is_ordinary_data() {
+    // A `"` that does not open a parameter value must not swallow the rest of
+    // the field, or the parameters behind it become invisible.
+    assert_eq!(
+        split_outside_quoted_strings("text/html;\";charset=gbk", ';'),
+        vec!["text/html", "\"", "charset=gbk"]
+    );
+    assert_eq!(
+        split_outside_quoted_strings("a\"b;c", ';'),
+        vec!["a\"b", "c"]
+    );
+}
+
+#[test]
+fn whitespace_between_equals_and_a_quoted_value_still_opens_it() {
+    assert_eq!(
+        split_outside_quoted_strings("text/html; boundary =  \"; x\"; charset=utf-8", ';'),
+        vec!["text/html", " boundary =  \"; x\"", " charset=utf-8"]
+    );
+}
+
+#[test]
+fn a_trailing_backslash_is_kept_rather_than_dropped() {
+    // Dropping it would turn a malformed value into a well-formed one.
+    assert_eq!(unquote_parameter_value("\"31536000\\"), "31536000\\");
+    assert_eq!(unquote_parameter_value("\"a\\"), "a\\");
+}
+
+#[test]
+fn an_escaped_quote_survives_as_data() {
+    assert_eq!(unquote_parameter_value("\"utf-8\\\"\""), "utf-8\"");
+}
diff --git a/moli-http-cache/src/policy.rs b/moli-http-cache/src/policy.rs
index fd4c92222..bcf027e08 100644
--- a/moli-http-cache/src/policy.rs
+++ b/moli-http-cache/src/policy.rs
@@ -295,4 +295,15 @@ mod tests {
         ));
         assert!(request_pragma_requires_validation("no-cache"));
     }
+
+    #[test]
+    fn a_trailing_backslash_does_not_manufacture_a_freshness_lifetime() {
+        // `31536000\` is malformed and must not parse as one year.
+        let headers = vec![(
+            "Cache-Control".to_owned(),
+            "max-age=\"31536000\\".to_owned(),
+        )];
+
+        assert_eq!(response_cache_policy(&headers).expires_at_unix_ms, None);
+    }
 }