Skip to content

fix(serde): correct Value end offset with utf8_lossy and invalid UTF-8 - #253

Merged
liuq19 merged 1 commit into
cloudwego:mainfrom
yuxi-liu-wired:pr/utf8-lossy-offset
Oct 11, 2026
Merged

liuq19 merged 1 commit into
cloudwego:mainfrom
yuxi-liu-wired:pr/utf8-lossy-offset

Conversation

@yuxi-liu-wired

Copy link
Copy Markdown
Contributor

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::<Value>(b"[\"a\xa0b\"]") returned "Invalid UTF-8
    characters" (the documented use of the feature), and so did
    Deserializer::utf8_lossy() + deserialize::<Value>() + 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.

Testing

Each commit adds a regression test that fails before the change. The full test suite passes and
cargo fmt --check is clean on the changed files.

Found with a differential fuzzer comparing serde_json, simd-json, sonic-rs and jiter.

@CLAassistant

CLAassistant commented Oct 1, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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::<Value>(b"[\"a\xa0b\"]")` returned "Invalid UTF-8
  characters" (the documented use of the feature), and so did
  `Deserializer::utf8_lossy()` + `deserialize::<Value>()` + `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.
@liuq19
liuq19 force-pushed the pr/utf8-lossy-offset branch from f5974e1 to 5472469 Compare October 10, 2026 07:58
@codecov

codecov Bot commented Oct 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.83%. Comparing base (c34a535) to head (5472469).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #253      +/-   ##
==========================================
+ Coverage   72.73%   72.83%   +0.09%     
==========================================
  Files          42       42              
  Lines        9944     9979      +35     
==========================================
+ Hits         7233     7268      +35     
  Misses       2711     2711              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@liuq19
liuq19 merged commit 9de5a49 into cloudwego:main Oct 11, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants