Skip to content

Lone high surrogate "\ud800" is decoded as U+0000 instead of an error #481

Description

@ddxd

A high surrogate escape that is not followed by a low-surrogate escape is decoded as U+0000 instead of producing an error, so invalid input silently turns into different data.

Reproduce (simd-json 0.18.1)

let mut v = br#""\ud800""#.to_vec();
let t = simd_json::to_tape(&mut v).unwrap();   // Ok
assert_eq!(t.0[0], simd_json::Node::String("\0")); // "\ud800" became "\0"

The same happens for "\ud800x", ["\udbff"], "\ud800\n", and for a pair whose second escape has invalid hex digits ("\ud800\uzzzz"). A lone low surrogate ("\udc00") and "\ud800A" are correctly rejected. It reproduces through to_tape, to_borrowed_value and serde, on every SIMD implementation, because they all go through get_unicode_codepoint.

Cause

In src/stringparse.rs, get_unicode_codepoint returns Ok((0, src_offset)) in both of those cases. That 0 is a leftover sentinel. When this code returned the number of bytes written (handle_unicode_codepoint before 29d9998), 0 meant "failed", and callers checked if o == 0 { return Err(...) } (those checks are still in the avx2/sse42/neon/simd128/portable parse_str). Since 29d9998 ("Implement std::simd portable") the function returns the code point instead, so the 0 means U+0000, codepoint_to_utf8 writes a NUL byte, and the o == 0 checks can no longer fire.

Expected

An error (InvalidUnicodeCodepoint), as for a lone low surrogate, and as serde_json, sonic-rs and orjson do.

I have a fix with a regression test and will open a PR referencing this issue.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions