fix(headers): respect quoted strings when splitting header parameters

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

moli-encoding/src/labels.rs

@@ -1,24 +1,39 @@
 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 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);
         }
     }
     None

moli-encoding/src/tests.rs

@@ -665,3 +665,92 @@ 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")
+    );
+}
+
+#[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());
+}

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)]

moli-header-field/src/parameters.rs

@@ -0,0 +1,93 @@
+use std::borrow::Cow;
+
+/// 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 value may
+/// contain that separator without ending the parameter it belongs to. Plain
+/// 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.
+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 at_value_start = false;
+    let mut inside_quotes = false;
+    let mut index = 0;
+
+    while index < bytes.len() {
+        let byte = bytes[index];
+
+        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;
+            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;
+    }
+
+    segments.push(&value[segment_start.min(value.len())..]);
+    segments
+}
+
+/// Removes a quoted value's delimiters and its quoting backslashes.
+///
+/// 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);
+    };
+    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),
+            '\\' => match characters.next() {
+                Some(escaped) => unquoted.push(escaped),
+                None => unquoted.push('\\'),
+            },
+            _ => unquoted.push(character),
+        }
+    }
+    Cow::Owned(unquoted)
+}

moli-header-field/src/tests.rs

@@ -122,3 +122,105 @@ 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é");
+}
+
+#[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\"");
+}