Skip to content

Commit b7a94a9

Browse files
changjoon-parkchangjoon-park
committed
Accept surrogates in _json.JsonScanner decode path
The _json decoder had two failure modes when a Python str value would contain a lone surrogate (legal per the Python 3 str model): 1. Boundary UnicodeEncodeError: JsonScanner::Callable::call rejected any input str with surrogates via try_into_utf8 before scanning began. 2. Silent U+FFFD corruption: call_scan_once and parse_object's key path called .to_string() on scanstring's Wtf8Buf output, which routes through Wtf8::Display (lossy). Array values and dict keys decoded from JSON \uXXXX escapes silently became U+FFFD. Switch JsonScanner's five PyUtf8StrRef signatures to PyStrRef, drop the entry-point try_into_utf8 call, and feed Wtf8Buf directly to new_str instead of going through .to_string(). Key memoization now uses HashMap<Wtf8Buf, PyStrRef> so surrogate-bearing keys survive interning. parse_number takes &[u8] since JSON numbers are ASCII. Extends the WTF-8 refactor pattern established in #7673 to the decoder. machinery::scanstring already returns Wtf8Buf and is unchanged. Unmasks test_single_surrogate_decode. 214 tests in test.test_json pass with no regressions. Decoder output verified byte-identical to CPython 3.13.4 over 10,000 random fuzz cases (JSON docs containing random surrogate escapes at root/list/dict positions, compared via json.dumps(..., ensure_ascii=True, sort_keys=True)).
1 parent 2e5c2be commit b7a94a9

2 files changed

Lines changed: 47 additions & 48 deletions

File tree

‎Lib/test/test_json/test_unicode.py‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
from collections import OrderedDict
33
from test.test_json import PyTest, CTest
44

5-
import unittest # XXX: RUSTPYTHON; importing to be able to skip tests
6-
75

86
class TestUnicode:
97
# test_encoding1 and test_encoding2 from 2.x are irrelevant (only str
@@ -137,7 +135,4 @@ def test_object_pairs_hook_with_unicode(self):
137135

138136

139137
class TestPyUnicode(TestUnicode, PyTest): pass
140-
class TestCUnicode(TestUnicode, CTest):
141-
@unittest.skip("TODO: RUSTPYTHON; decode path still uses PyUtf8StrRef")
142-
def test_single_surrogate_decode(self):
143-
return super().test_single_surrogate_decode()
138+
class TestCUnicode(TestUnicode, CTest): pass

‎crates/stdlib/src/json.rs‎

Lines changed: 46 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ mod _json {
66
use super::machinery;
77
use crate::vm::{
88
AsObject, Py, PyObjectRef, PyPayload, PyResult, VirtualMachine,
9-
builtins::{PyBaseExceptionRef, PyStrRef, PyType, PyUtf8StrRef},
9+
builtins::{PyBaseExceptionRef, PyStrRef, PyType},
1010
convert::ToPyResult,
1111
function::{IntoFuncArgs, OptionalArg},
1212
protocol::PyIterReturn,
@@ -91,7 +91,7 @@ mod _json {
9191
impl JsonScanner {
9292
fn parse(
9393
&self,
94-
pystr: PyUtf8StrRef,
94+
pystr: PyStrRef,
9595
char_idx: usize,
9696
byte_idx: usize,
9797
scan_once: PyObjectRef,
@@ -115,7 +115,7 @@ mod _json {
115115
// Parse string - pass slice starting after the quote
116116
let (wtf8_result, chars_consumed, _bytes_consumed) =
117117
machinery::scanstring(&wtf8[byte_idx + 1..], char_idx + 1, self.strict)
118-
.map_err(|e| py_decode_error(e, pystr.clone().into_wtf8(), vm))?;
118+
.map_err(|e| py_decode_error(e, pystr.clone(), vm))?;
119119
let end_char_idx = char_idx + 1 + chars_consumed;
120120
return Ok(PyIterReturn::Return(
121121
vm.new_tuple((wtf8_result, end_char_idx)).into(),
@@ -142,11 +142,11 @@ mod _json {
142142
_ => {}
143143
}
144144

145-
let s = &pystr.as_str()[byte_idx..];
145+
let rest = &bytes[byte_idx..];
146146

147147
macro_rules! parse_const {
148148
($s:literal, $val:expr) => {
149-
if s.starts_with($s) {
149+
if rest.starts_with($s.as_bytes()) {
150150
return Ok(PyIterReturn::Return(
151151
vm.new_tuple(($val, char_idx + $s.len())).into(),
152152
));
@@ -158,15 +158,15 @@ mod _json {
158158
parse_const!("true", true);
159159
parse_const!("false", false);
160160

161-
if let Some((res, len)) = self.parse_number(s, vm) {
161+
if let Some((res, len)) = self.parse_number(rest, vm) {
162162
return Ok(PyIterReturn::Return(
163163
vm.new_tuple((res?, char_idx + len)).into(),
164164
));
165165
}
166166

167167
macro_rules! parse_constant {
168168
($s:literal) => {
169-
if s.starts_with($s) {
169+
if rest.starts_with($s.as_bytes()) {
170170
return Ok(PyIterReturn::Return(
171171
vm.new_tuple((
172172
self.parse_constant.call(($s,), vm)?,
@@ -187,28 +187,31 @@ mod _json {
187187
)))
188188
}
189189

190-
fn parse_number(&self, s: &str, vm: &VirtualMachine) -> Option<(PyResult, usize)> {
190+
fn parse_number(&self, bytes: &[u8], vm: &VirtualMachine) -> Option<(PyResult, usize)> {
191191
flame_guard!("JsonScanner::parse_number");
192192
let mut has_neg = false;
193193
let mut has_decimal = false;
194194
let mut has_exponent = false;
195195
let mut has_e_sign = false;
196196
let mut i = 0;
197-
for c in s.chars() {
198-
match c {
199-
'-' if i == 0 => has_neg = true,
200-
n if n.is_ascii_digit() => {}
201-
'.' if !has_decimal => has_decimal = true,
202-
'e' | 'E' if !has_exponent => has_exponent = true,
203-
'+' | '-' if !has_e_sign => has_e_sign = true,
197+
// JSON numbers are ASCII per RFC 8259 (digits, '-', '+', '.', 'e', 'E'),
198+
// so byte iteration is equivalent to char iteration here.
199+
for &b in bytes {
200+
match b {
201+
b'-' if i == 0 => has_neg = true,
202+
b'0'..=b'9' => {}
203+
b'.' if !has_decimal => has_decimal = true,
204+
b'e' | b'E' if !has_exponent => has_exponent = true,
205+
b'+' | b'-' if !has_e_sign => has_e_sign = true,
204206
_ => break,
205207
}
206208
i += 1;
207209
}
208210
if i == 0 || (i == 1 && has_neg) {
209211
return None;
210212
}
211-
let buf = &s[..i];
213+
// SAFETY: the loop above accepts only ASCII bytes, so bytes[..i] is valid UTF-8.
214+
let buf = unsafe { core::str::from_utf8_unchecked(&bytes[..i]) };
212215
let ret = if has_decimal || has_exponent {
213216
// float
214217
if let Some(ref parse_float) = self.parse_float {
@@ -228,11 +231,11 @@ mod _json {
228231
/// Returns (parsed_object, end_char_index, end_byte_index).
229232
fn parse_object(
230233
&self,
231-
pystr: PyUtf8StrRef,
234+
pystr: PyStrRef,
232235
start_char_idx: usize,
233236
start_byte_idx: usize,
234237
scan_once: &PyObjectRef,
235-
memo: &mut HashMap<String, PyStrRef>,
238+
memo: &mut HashMap<Wtf8Buf, PyStrRef>,
236239
vm: &VirtualMachine,
237240
) -> PyResult<(PyObjectRef, usize, usize)> {
238241
flame_guard!("JsonScanner::parse_object");
@@ -275,18 +278,19 @@ mod _json {
275278
// Parse key string using scanstring with byte slice
276279
let (key_wtf8, chars_consumed, bytes_consumed) =
277280
machinery::scanstring(&wtf8[byte_idx..], char_idx, self.strict)
278-
.map_err(|e| py_decode_error(e, pystr.clone().into_wtf8(), vm))?;
281+
.map_err(|e| py_decode_error(e, pystr.clone(), vm))?;
279282

280283
char_idx += chars_consumed;
281284
byte_idx += bytes_consumed;
282285

283-
// Key memoization - reuse existing key strings
284-
let key_str = key_wtf8.to_string();
285-
let key: PyObjectRef = match memo.get(&key_str) {
286+
// Key memoization - reuse existing key strings.
287+
// Keyed by Wtf8Buf so lone surrogates in keys (legal per Python str)
288+
// are preserved; using String here would lossy-collapse surrogates to U+FFFD.
289+
let key: PyObjectRef = match memo.get(&key_wtf8) {
286290
Some(cached) => cached.clone().into(),
287291
None => {
288-
let py_key = vm.ctx.new_str(key_str.clone());
289-
memo.insert(key_str, py_key.clone());
292+
let py_key = vm.ctx.new_str(key_wtf8.clone());
293+
memo.insert(key_wtf8, py_key.clone());
290294
py_key.into()
291295
}
292296
};
@@ -389,11 +393,11 @@ mod _json {
389393
/// Returns (parsed_array, end_char_index, end_byte_index).
390394
fn parse_array(
391395
&self,
392-
pystr: PyUtf8StrRef,
396+
pystr: PyStrRef,
393397
start_char_idx: usize,
394398
start_byte_idx: usize,
395399
scan_once: &PyObjectRef,
396-
memo: &mut HashMap<String, PyStrRef>,
400+
memo: &mut HashMap<Wtf8Buf, PyStrRef>,
397401
vm: &VirtualMachine,
398402
) -> PyResult<(PyObjectRef, usize, usize)> {
399403
flame_guard!("JsonScanner::parse_array");
@@ -507,10 +511,10 @@ mod _json {
507511
fn call_scan_once(
508512
&self,
509513
scan_once: &PyObjectRef,
510-
pystr: PyUtf8StrRef,
514+
pystr: PyStrRef,
511515
char_idx: usize,
512516
byte_idx: usize,
513-
memo: &mut HashMap<String, PyStrRef>,
517+
memo: &mut HashMap<Wtf8Buf, PyStrRef>,
514518
vm: &VirtualMachine,
515519
) -> PyResult<(PyObjectRef, usize, usize)> {
516520
// Recursion guard: parse_object/parse_array recurse into call_scan_once
@@ -521,7 +525,6 @@ mod _json {
521525
vm.with_recursion("while decoding a JSON object from a string", || {
522526
let bytes = pystr.as_bytes();
523527
let wtf8 = pystr.as_wtf8();
524-
let s = pystr.as_str();
525528

526529
let first_byte = match bytes.get(byte_idx) {
527530
Some(&b) => b,
@@ -532,11 +535,13 @@ mod _json {
532535

533536
match first_byte {
534537
b'"' => {
535-
// String - pass slice starting after the quote
538+
// String - pass slice starting after the quote.
539+
// Feed the Wtf8Buf directly to new_str; routing through
540+
// .to_string() here would lossy-collapse surrogates to U+FFFD.
536541
let (wtf8_result, chars_consumed, bytes_consumed) =
537542
machinery::scanstring(&wtf8[byte_idx + 1..], char_idx + 1, self.strict)
538-
.map_err(|e| py_decode_error(e, pystr.clone().into_wtf8(), vm))?;
539-
let py_str = vm.ctx.new_str(wtf8_result.to_string());
543+
.map_err(|e| py_decode_error(e, pystr.clone(), vm))?;
544+
let py_str = vm.ctx.new_str(wtf8_result);
540545
Ok((
541546
py_str.into(),
542547
char_idx + 1 + chars_consumed,
@@ -580,14 +585,14 @@ mod _json {
580585
return Ok((result, char_idx + 9, byte_idx + 9));
581586
}
582587
// Negative number - numbers are ASCII so len == bytes
583-
if let Some((result, len)) = self.parse_number(&s[byte_idx..], vm) {
588+
if let Some((result, len)) = self.parse_number(&bytes[byte_idx..], vm) {
584589
return Ok((result?, char_idx + len, byte_idx + len));
585590
}
586591
Err(self.make_decode_error("Expecting value", pystr, char_idx, vm))
587592
}
588593
b'0'..=b'9' => {
589594
// Positive number - numbers are ASCII so len == bytes
590-
if let Some((result, len)) = self.parse_number(&s[byte_idx..], vm) {
595+
if let Some((result, len)) = self.parse_number(&bytes[byte_idx..], vm) {
591596
return Ok((result?, char_idx + len, byte_idx + len));
592597
}
593598
Err(self.make_decode_error("Expecting value", pystr, char_idx, vm))
@@ -608,11 +613,11 @@ mod _json {
608613
let end_char_idx: isize = tuple.as_slice()[1].try_to_value(vm)?;
609614
// For fallback, we need to calculate byte_idx from char_idx
610615
// This is expensive but fallback should be rare
611-
let end_byte_idx = s
612-
.char_indices()
616+
let end_byte_idx = wtf8
617+
.code_point_indices()
613618
.nth(end_char_idx as usize)
614619
.map(|(i, _)| i)
615-
.unwrap_or(s.len());
620+
.unwrap_or(wtf8.len());
616621
Ok((value, end_char_idx as usize, end_byte_idx))
617622
}
618623
Err(err) if err.fast_isinstance(vm.ctx.exceptions.stop_iteration) => {
@@ -629,12 +634,12 @@ mod _json {
629634
fn make_decode_error(
630635
&self,
631636
msg: &str,
632-
s: PyUtf8StrRef,
637+
s: PyStrRef,
633638
pos: usize,
634639
vm: &VirtualMachine,
635640
) -> PyBaseExceptionRef {
636641
let err = machinery::DecodeError::new(msg, pos);
637-
py_decode_error(err, s.into_wtf8(), vm)
642+
py_decode_error(err, s, vm)
638643
}
639644
}
640645

@@ -645,14 +650,13 @@ mod _json {
645650
return Err(vm.new_value_error("idx cannot be negative"));
646651
}
647652
let char_idx = char_idx as usize;
648-
let pystr = pystr.try_into_utf8(vm)?;
649-
let s = pystr.as_str();
653+
let wtf8 = pystr.as_wtf8();
650654

651655
// Calculate byte index from char index (O(char_idx) but only at entry point)
652656
let byte_idx = if char_idx == 0 {
653657
0
654658
} else {
655-
match s.char_indices().nth(char_idx) {
659+
match wtf8.code_point_indices().nth(char_idx) {
656660
Some((byte_i, _)) => byte_i,
657661
None => {
658662
// char_idx is beyond the string length

0 commit comments

Comments
 (0)