From 547246944108df54a8d56ec9a72ee36f4e3c2c36 Mon Sep 17 00:00:00 2001 From: yuxi-liu-wired <33951560+yuxi-liu-wired@users.noreply.github.com> Date: Thu, 1 Oct 2026 20:23:42 +0000 Subject: [PATCH] fix(utf8_lossy): Value end offset when the input has invalid UTF-8 With `utf8_lossy`, deserializing a `Value` from input that contains invalid UTF-8 parses a lossy copy (`String::from_utf8_lossy`), where each invalid sequence became the 3-byte U+FFFD. The end offset `n` returned for that copy was then used as an offset into the original input (`read.eat(n)`), so: - valid input failed. With the `utf8_lossy` feature on, plain `from_slice::(b"[\"a\xa0b\"]")` returned "Invalid UTF-8 characters" (the documented use of the feature), and so did `Deserializer::utf8_lossy()` + `deserialize::()` + `end()`. - trailing characters were skipped: `[\"a\xa0b\"]xx` was accepted by `end()`, because the offset overshot the real end. Deserializing the same input into any other type (String, Vec, serde_json::Value, ...) was right. The consumed length in the original is now its length minus what the parser left unread in the copy (the tail after the value is identical in both); an overshoot into the padding (unterminated input) is kept, so it is still EOF. The next-invalid-UTF-8 position is then recomputed from that point, as the invalid UTF-8 inside the value was replaced. Test: `test_utf8_lossy_value_trailing` (fails before this change). Suite: 249 (default), 246 (utf8_lossy, main 245), 248 (all features, main 247); Miri clean on the utf8_lossy tests. Found by a differential fuzzer (serde_json, simd-json, sonic-rs, jiter) extended with sonic-rs's utf8_lossy mode against a decode-then-parse reference. --- src/serde/de.rs | 28 +++++++++++++++++++++++----- src/serde/mod.rs | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/src/serde/de.rs b/src/serde/de.rs index 5f666cb..17b2115 100644 --- a/src/serde/de.rs +++ b/src/serde/de.rs @@ -372,14 +372,32 @@ impl<'de, R: Reader<'de>> Deserializer { let json = self.parser.read.as_u8_slice(); // get n to check trailing characters in later - let n = if cfg.utf8_lossy && self.parser.read.next_invalid_utf8() != usize::MAX { + if cfg.utf8_lossy && self.parser.read.next_invalid_utf8() != usize::MAX { // repr the invalid utf8, not need to care about the invalid UTF8 char in non-string // parts, it will cause errors when parsing. - val.parse_with_padding(String::from_utf8_lossy(json).as_bytes(), cfg)? + let lossy = String::from_utf8_lossy(json); + let n = val.parse_with_padding(lossy.as_bytes(), cfg)?; + // `n` is an offset into the lossy copy, where every invalid sequence became the + // 3-byte U+FFFD, so it is not an offset into `json`. What follows the value is + // the same in both (whitespace or trailing characters; invalid UTF-8 there is a + // trailing-character error either way), so count it from the end. + // `n` past the end means the parser ran into the padding (unterminated input): + // keep it past the end of `json` by the same amount, so that is still reported. + let consumed = if n >= lossy.len() { + json.len() + (n - lossy.len()) + } else { + json.len() - (lossy.len() - n) + }; + self.parser.read.eat(consumed); + // the invalid UTF-8 inside the value has been replaced: look for it only in the + // part that is still to be read (if any; past the end is reported as EOF) + if consumed <= json.len() { + self.parser.read.check_invalid_utf8(); + } } else { - val.parse_with_padding(json, cfg)? - }; - self.parser.read.eat(n); + let n = val.parse_with_padding(json, cfg)?; + self.parser.read.eat(n); + } } else { let shared = unsafe { if self.shared.is_none() { diff --git a/src/serde/mod.rs b/src/serde/mod.rs index e7a16bd..61f6d28 100644 --- a/src/serde/mod.rs +++ b/src/serde/mod.rs @@ -765,6 +765,42 @@ mod test { } } + #[test] + fn test_utf8_lossy_value_trailing() { + // Into `Value`, the lossy path parses a copy where each invalid sequence became the 3-byte + // U+FFFD; its end offset was used as an offset into the original input. So valid input + // failed in `end()`, and trailing characters after the value were skipped. + let parse = |json: &[u8]| -> Result { + let mut de = Deserializer::from_slice(json).utf8_lossy(); + let value: Value = de.deserialize()?; + de.end()?; + Ok(value) + }; + for json in [ + &b"[\"a\xa0b\"]"[..], + b"[\"a\xa0b\"]\n", + b"{\"k\":\"\xc3\"}", + b"\"\xff\"", + ] { + parse(json).unwrap_or_else(|e| panic!("{json:?}: {e}")); + } + assert_eq!( + parse(b"[\"a\xa0b\"]").unwrap(), + crate::json!(["a\u{fffd}b"]) + ); + for json in [ + &b"[\"a\xa0b\"]xx"[..], + b"[\"\xa0\xa0\xa0\"] [1]", + b"\"\xa0\"1", + ] { + let err = parse(json).expect_err(&format!("{json:?} has trailing characters")); + assert!(err.to_string().contains("trailing"), "{json:?}: {err}"); + } + // unterminated input is still an error + assert!(parse(b"[\"a\xa0").is_err()); + assert!(parse(b"\"abc\xfa").is_err()); + } + #[test] fn test_utf8_lossy_surrogate_backtrack() { // high surrogate + non-\u content → FFFD + literal chars