Accept surrogates in _json.encode_basestring{,_ascii} - #7673

crates/stdlib/src/json/machinery.rs (1)

75-128: Rewrite looks correct and well-documented.

Both branches preserve WTF-8 validity on the output:

• ascii_only: all write_start_idx advances (idx+1, idx+c.len_utf8(), idx+3) land on code-point boundaries, so every &bytes[write_start_idx..idx] flush is WTF-8 aligned. The fixed +3 for lone surrogates is safe because WTF-8 encodes every surrogate as exactly 3 bytes. Non-BMP scalars correctly emit a UTF-16 surrogate pair via encode_utf16.
• non-ascii: the only escaped targets (< 0x20, \\, ") are all < 0x80, which never appear inside a multi-byte WTF-8 sequence, so byte-level iteration never splits a codepoint. Multi-byte scalars and 3-byte lone-surrogate sequences pass through the trailing flush untouched — matching CPython's py_encode_basestring behavior (including not escaping 0x7F).

Optional readability nit: line 112 re-calls wtf8.as_bytes().iter() while bytes is already bound at line 78.

Optional tidy-up

-        for (idx, c) in wtf8.as_bytes().iter().enumerate() {
-            let escaped_opt: Option<&'static str> = match *c {
+        for (idx, &c) in bytes.iter().enumerate() {
+            let escaped_opt: Option<&'static str> = match c {
                 x if x < 0x20 => Some(ESCAPE_CHARS[x as usize]),
                 b'\\' => Some("\\\\"),
                 b'\"' => Some("\\\""),
                 _ => None,
             };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/json/machinery.rs` around lines 75 - 128, The non-ascii
branch unnecessarily calls wtf8.as_bytes().iter() again even though bytes is
already bound; update the loop in write_json_string (function write_json_string)
to iterate over bytes.iter().enumerate() (or bytes.iter().enumerate()) instead
of wtf8.as_bytes().iter().enumerate() to avoid the redundant call while
preserving the existing logic and use of idx, c, escaped_opt, write_start_idx,
and the trailing flush behavior.

crates/stdlib/src/json.rs (1)

670-684: Solid safety upgrade — checked Wtf8Buf::from_bytes replaces UB-prone from_utf8_unchecked.

The invariant comment accurately describes what write_json_string now guarantees, and switching to the checked constructor turns any accidental violation of that invariant into a clean panic rather than UB. The flush slices in write_json_string are always WTF-8 aligned (code-point boundaries in the ASCII-only path; ASCII-only escape triggers in the non-ASCII path), so this should never actually panic in practice.

Optional nit: Wtf8Buf is imported at line 17 but Wtf8 is referenced via its full path here; importing Wtf8 alongside Wtf8Buf would be a touch more consistent.

Optional refactor

-    use rustpython_common::wtf8::Wtf8Buf;
+    use rustpython_common::wtf8::{Wtf8, Wtf8Buf};

-    fn encode_string(wtf8: &rustpython_common::wtf8::Wtf8, ascii_only: bool) -> Wtf8Buf {
+    fn encode_string(wtf8: &Wtf8, ascii_only: bool) -> Wtf8Buf {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/json.rs` around lines 670 - 684, The code correctly
replaces unsafe from_utf8_unchecked with Wtf8Buf::from_bytes in encode_string,
but for consistency import Wtf8 alongside Wtf8Buf instead of referencing
rustpython_common::wtf8::Wtf8 by full path; update the top-of-file use list to
include Wtf8 (so encode_string can refer to Wtf8 directly) while leaving the
logic in encode_string and the call to machinery::write_json_string unchanged.

crates/stdlib/src/json/machinery.rs (1)

75-128: Encoder rewrite looks correct; couple of minor readability nits.

The ascii_only / non-ascii_only branches trace correctly for mixed ASCII, escaped, multi-byte scalar, and lone-surrogate inputs. The non-ascii_only byte-scan is also safe: WTF-8 continuation/leading bytes and surrogate encodings are all ≥ 0x80, so 0x22 (") and 0x5C (\) cannot appear inside a multi-byte sequence.

Two small, optional polish items:

1. Line 102: the hardcoded 3 for the WTF-8 surrogate length works, but CodePoint::len_wtf8() already exists and is used elsewhere in this file (line 150). Using it makes the intent self-documenting.
2. Line 112: bytes is already bound at line 78; re-deriving wtf8.as_bytes() here is redundant.

♻️ Proposed tidy-ups

             } else {
                 // Lone surrogate code point (U+D800..U+DFFF).
-                // WTF-8 encodes these as 3-byte sequences; skip those raw bytes
-                // and emit a \uXXXX escape with the surrogate value.
+                // WTF-8 encodes surrogates as 3-byte sequences; skip those raw
+                // bytes and emit a \uXXXX escape with the surrogate value.
                 w.write_all(&bytes[write_start_idx..idx])?;
-                write_start_idx = idx + 3;
+                write_start_idx = idx + cp.len_wtf8();
                 write!(w, "\\u{:04x}", cp.to_u32())?;
             }

-        for (idx, c) in wtf8.as_bytes().iter().enumerate() {
+        for (idx, c) in bytes.iter().enumerate() {
             let escaped_opt: Option<&'static str> = match *c {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/json/machinery.rs` around lines 75 - 128, In
write_json_string: replace the hardcoded surrogate byte length 3 (used in the
lone-surrogate branch that currently does write_start_idx = idx + 3) with the
existing CodePoint::len_wtf8() method (call it on cp) to make intent explicit,
and in the else (ensure_ascii == false) branch avoid re-calling wtf8.as_bytes()
by iterating over the already-bound bytes variable (use bytes.iter().enumerate()
instead of wtf8.as_bytes().iter().enumerate()) so the code reuses the buffer and
removes the redundant call.