diff --git a/changelog.d/9224-json-parse-single-pass.md b/changelog.d/9224-json-parse-single-pass.md new file mode 100644 index 0000000000..148efac21c --- /dev/null +++ b/changelog.d/9224-json-parse-single-pass.md @@ -0,0 +1,4 @@ +### Performance + +- `JSON.parse` now validates and constructs values in one strict parser pass, + eliminating the preliminary full-document validation scan. diff --git a/crates/perry-runtime/src/json/mod.rs b/crates/perry-runtime/src/json/mod.rs index 6cef02bbe0..897db20ad2 100644 --- a/crates/perry-runtime/src/json/mod.rs +++ b/crates/perry-runtime/src/json/mod.rs @@ -966,8 +966,118 @@ mod tests { } } + fn direct_parser_accepts(input: &[u8]) -> bool { + let saved_roots = parse_root_save_len(); + let accepted = { + let _suppress = crate::gc::GcSuppressScope::new(); + let mut parser = DirectParser::new(input); + unsafe { + parser.parse_value(); + } + let accepted = parser.finish(); + parse_root_restore(saved_roots); + accepted + }; + accepted + } + + #[test] + fn direct_parser_validates_json_while_building_the_value() { + let invalid: &[&[u8]] = &[ + b"{", + b"}", + b"[", + b"]", + b"", + b" ", + br#"{,}"#, + br#"[,]"#, + br#"[1,]"#, + br#"{"a":1,}"#, + br#"{a:1}"#, + br#"{'a':1}"#, + br#"[01]"#, + br#"[-01]"#, + br#"[1.]"#, + br#"[.5]"#, + br#"[+1]"#, + br#"[1e]"#, + br#"[1e+]"#, + br#"[--1]"#, + br#"[NaN]"#, + br#"[Infinity]"#, + br#"[-Infinity]"#, + br#"[undefined]"#, + br#"[TRUE]"#, + br#""unterminated"#, + br#"["bad\x"]"#, + br#"["\u12"]"#, + br#"["\uZZZZ"]"#, + br#"{"a" 1}"#, + br#"{"a":}"#, + br#"{:1}"#, + br#"[1 2]"#, + br#"[1][2]"#, + br#"{}{}"#, + b"nul", + b"tru", + br#"[1,,2]"#, + br#"{"a":1 "b":2}"#, + br#""\t"x"#, + b"\"\t\"", + b"\"abcdefgh\nijklmnopqrst\"", + b"\"abcdefghijklmnop\nqrst\"", + b"\x0bnull", + ]; + for input in invalid { + assert!( + !direct_parser_accepts(input), + "DirectParser accepted malformed JSON: {:?}", + String::from_utf8_lossy(input) + ); + } + + let valid: &[&[u8]] = &[ + br#"{}"#, + br#"[]"#, + b"0", + b"-0", + b"1e5", + b"1E+5", + b"1e-5", + b"-1.5", + b"null", + b"true", + b"false", + br#""""#, + br#""\u0041""#, + br#""\n""#, + br#"[1,2,3]"#, + br#"{"a":{"b":[1,{"c":null}]}}"#, + br#"{"a":1,"a":2}"#, + br#"[[[[[1]]]]]"#, + br#""\ud83d\ude00""#, + br#""\ud800""#, + br#""\ud800\u0041""#, + br#""\udc00""#, + br#"{"":1}"#, + br#" {"a" : 1 } "#, + br#"[1e308]"#, + br#"[-1e308]"#, + br#"[1e-400]"#, + b"9007199254740993", + ]; + for input in valid { + assert!( + direct_parser_accepts(input), + "DirectParser rejected valid JSON: {:?}", + String::from_utf8_lossy(input) + ); + } + } + #[test] - fn parse_result_streaming_validation_rejects_malformed_and_trailing_input() { + fn parse_result_direct_validation_rejects_malformed_and_trailing_input() { for input in [ br#"{"a":[1,]}"#.as_slice(), br#"{"a":1} trailing"#.as_slice(), @@ -975,7 +1085,7 @@ mod tests { let text = js_string_from_bytes(input.as_ptr(), input.len() as u32); assert!( unsafe { js_json_parse_result(text) }.is_err(), - "invalid JSON must be rejected before Perry tree construction" + "invalid JSON must be rejected by Perry's direct parser" ); } @@ -1541,8 +1651,8 @@ mod tests { let mut parser = DirectParser::new(bytes); let value = unsafe { parser.parse_number() }; assert!( - !parser.has_trailing_content(), - "parse_number left trailing input on {s:?}" + parser.finish(), + "parse_number did not consume valid input {s:?}" ); value.bits() }; diff --git a/crates/perry-runtime/src/json/parse_api.rs b/crates/perry-runtime/src/json/parse_api.rs index 5f3683172b..a8cb9a8fa0 100644 --- a/crates/perry-runtime/src/json/parse_api.rs +++ b/crates/perry-runtime/src/json/parse_api.rs @@ -112,21 +112,6 @@ fn iterative_budget_message() -> String { ) } -fn is_json_null_literal(bytes: &[u8]) -> bool { - let Some(start) = bytes - .iter() - .position(|b| !matches!(b, b' ' | b'\t' | b'\n' | b'\r')) - else { - return false; - }; - let end = bytes - .iter() - .rposition(|b| !matches!(b, b' ' | b'\t' | b'\n' | b'\r')) - .map(|idx| idx + 1) - .unwrap_or(start); - &bytes[start..end] == b"null" -} - /// Parse a deeply nested document through the flat tape representation. Tape /// construction validates syntax with an explicit heap stack; materialization /// likewise keeps pending containers on the heap. This path runs only beyond @@ -146,8 +131,8 @@ unsafe fn try_parse_deep_iterative( let bytes = { let moved = parse_root_get(text_root); let hdr = moved.as_string_ptr(); - let data_ptr = (hdr as *const u8).add(std::mem::size_of::()); - std::slice::from_raw_parts(data_ptr, len) + // Canonical payload accessor, not an open-coded header offset. + std::slice::from_raw_parts(crate::string::string_data(hdr), len) }; let result = crate::json_tape::materialize_iterative(tape_entries, bytes); if let Some(value) = result { @@ -200,30 +185,12 @@ pub unsafe fn js_json_parse_result(text_ptr: *const StringHeader) -> Result(bytes) { - return Err(syntax_error_value(&format!("JSON parse error: {}", err))); - } - // #7341: root the source string BEFORE the collection points, then // re-derive the input slice from the rooted value. - // - // The order used to be: derive `bytes`, run `serde_json::from_slice` (which - // allocates and arms the malloc trigger), call `gc_check_trigger()` (which - // can collect outright), suppress, and only THEN push the root. Two things - // went wrong at once. The slice predated a collection point, and — the part - // that makes re-deriving alone useless — so did the root: pushing - // `text_ptr` after the collection roots an address the collector has - // already moved away from, so reading it back yields the same stale - // pointer. The parser then reads retired from-space for the whole parse, - // which the quarantine reports as a fault at `parse_value + 36`, on the - // very first `peek()`. - // - // Rooting first means the collector rewrites the slot, so the re-read below - // yields the post-move payload address. The suppression that follows was - // already here and was never the bug. + // Pushing `text_ptr` after a collection would root an address the collector + // had already moved away from, so re-deriving from that slot would return + // the same stale pointer. Rooting first means the collector rewrites the + // slot and the parser receives the post-move payload. let text_root = parse_root_push(JSValue::string_ptr(text_ptr as *mut StringHeader)); crate::gc::gc_collect_pending_suppressed_parse(); @@ -231,26 +198,19 @@ pub unsafe fn js_json_parse_result(text_ptr: *const StringHeader) -> Result()); - std::slice::from_raw_parts(data_ptr, len) + // Canonical payload accessor, not an open-coded header offset. + std::slice::from_raw_parts(crate::string::string_data(hdr), len) }; let mut parser = DirectParser::new(bytes); let result = parser.parse_value(); + let parse_ok = parser.finish(); parse_root_push(result); crate::gc::gc_unsuppress(); crate::gc::gc_bump_malloc_trigger(); @@ -266,11 +226,8 @@ pub unsafe fn js_json_parse_result(text_ptr: *const StringHeader) -> Result JSValue None => throw_syntax_error("JSON parse error: malformed deep document"), }; } - // Keep serde_json's strict syntax validation, but discard tokens as they - // are read instead of allocating an intermediate `serde_json::Value` - // immediately before Perry builds its own tree. - if let Err(err) = serde_json::from_slice::(bytes) { - throw_syntax_error(&format!("JSON parse error: {}", err)); - } crate::gc::gc_collect_pending_suppressed_parse(); @@ -427,12 +378,13 @@ pub unsafe extern "C" fn js_json_parse(text_ptr: *const StringHeader) -> JSValue let bytes = { let moved = crate::json::parse_root_get(text_root); let hdr = moved.as_string_ptr(); - let data_ptr = (hdr as *const u8).add(std::mem::size_of::()); - std::slice::from_raw_parts(data_ptr, len) + // Canonical payload accessor, not an open-coded header offset. + std::slice::from_raw_parts(crate::string::string_data(hdr), len) }; let mut parser = DirectParser::new(bytes); let result = parser.parse_value(); + let parse_ok = parser.finish(); parse_root_push(result); // Re-enable GC and rebaseline triggers while the result is still @@ -456,27 +408,8 @@ pub unsafe extern "C" fn js_json_parse(text_ptr: *const StringHeader) -> JSValue } }); - // If parser didn't consume meaningful input (result is null and input wasn't "null"), - // the input was invalid JSON — throw SyntaxError - if result.is_null() { - let is_literal_null = len >= 4 && bytes.starts_with(b"null"); - if !is_literal_null { - let preview_len = len.min(50); - let preview = std::str::from_utf8(&bytes[..preview_len]).unwrap_or("???"); - let msg = format!("JSON parse error: Unexpected token: {}", preview); - throw_syntax_error(&msg); - } else if parser.has_trailing_content() { - // Literal `null` followed by trailing tokens (`JSON.parse("null x")`) - // — reject like any other trailing-token case. - throw_syntax_error("Unexpected non-whitespace character after JSON"); - } - } else if parser.has_trailing_content() { - // A valid value was parsed but non-whitespace input remains - // (`JSON.parse("{}x")`, `JSON.parse("1 2")`). Node rejects trailing - // tokens with a SyntaxError; trailing whitespace is allowed. - crate::exception::js_throw(syntax_error_value( - "Unexpected non-whitespace character after JSON", - )); + if !parse_ok { + throw_syntax_error("JSON parse error: malformed input"); } result @@ -633,14 +566,22 @@ pub unsafe extern "C" fn js_json_parse_typed_array( }; // Same pre-parse cleanup + GC suppression as `js_json_parse` — - // keeps the typed path on the same GC-safety contract. + // root before the collection point and re-derive the source bytes after it. + let text_root = parse_root_push(JSValue::string_ptr(text_ptr as *mut StringHeader)); crate::gc::gc_collect_pending_suppressed_parse(); crate::gc::gc_check_trigger(); crate::gc::gc_suppress(); - let text_root = parse_root_push(JSValue::string_ptr(text_ptr as *mut StringHeader)); + + let bytes = { + let moved = crate::json::parse_root_get(text_root); + let hdr = moved.as_string_ptr(); + // Canonical payload accessor, not an open-coded header offset. + std::slice::from_raw_parts(crate::string::string_data(hdr), len) + }; let mut parser = DirectParser::with_shape(bytes, shape); let result = parser.parse_array_typed(); + let parse_ok = parser.finish(); parse_root_push(result); crate::gc::gc_unsuppress(); @@ -656,16 +597,8 @@ pub unsafe extern "C" fn js_json_parse_typed_array( } }); - if result.is_null() { - let is_literal_null = len >= 4 && bytes.starts_with(b"null"); - if !is_literal_null { - let preview_len = len.min(50); - let preview = std::str::from_utf8(&bytes[..preview_len]).unwrap_or("???"); - let msg = format!("JSON parse error: Unexpected token: {}", preview); - // Throw a real `SyntaxError` (not a bare string) to match Node's - // error identity for invalid JSON. - crate::exception::js_throw(syntax_error_value(&msg)); - } + if !parse_ok { + throw_syntax_error("JSON parse error: malformed input"); } result diff --git a/crates/perry-runtime/src/json/parser.rs b/crates/perry-runtime/src/json/parser.rs index 138a8d4aaa..bdc1d21905 100644 --- a/crates/perry-runtime/src/json/parser.rs +++ b/crates/perry-runtime/src/json/parser.rs @@ -28,6 +28,38 @@ impl<'a> ParsedStr<'a> { } } +#[inline] +fn decode_hex_u16(bytes: &[u8]) -> Option { + if bytes.len() != 4 { + return None; + } + let mut value = 0u16; + for &byte in bytes { + let digit = match byte { + b'0'..=b'9' => byte - b'0', + b'a'..=b'f' => byte - b'a' + 10, + b'A'..=b'F' => byte - b'A' + 10, + _ => return None, + }; + value = (value << 4) | digit as u16; + } + Some(value) +} + +#[inline] +fn push_code_unit_wtf8(output: &mut Vec, unit: u16) { + if (0xD800..=0xDFFF).contains(&unit) { + output.push(0xE0 | (unit >> 12) as u8); + output.push(0x80 | ((unit >> 6) & 0x3F) as u8); + output.push(0x80 | (unit & 0x3F) as u8); + } else { + let ch = + char::from_u32(unit as u32).expect("non-surrogate UTF-16 unit is a Unicode scalar"); + let mut buf = [0u8; 4]; + output.extend_from_slice(ch.encode_utf8(&mut buf).as_bytes()); + } +} + /// Issue #179 typed-parse plan, Step 1b. Pre-computed shape for /// `JSON.parse(blob)` where T is an object type with a known /// field list. Built once per typed-parse call from the codegen- @@ -114,6 +146,7 @@ pub(crate) fn nesting_depth_exceeds(bytes: &[u8], limit: usize) -> bool { pub(crate) struct DirectParser<'a> { input: &'a [u8], pos: usize, + valid: bool, /// Issue #179 typed-parse: if Some, the top-level value is /// expected to be `Array` matching this shape. Each /// record uses the fast path; mismatches silently fall through @@ -134,6 +167,7 @@ impl<'a> DirectParser<'a> { Self { input, pos: 0, + valid: true, shape: None, hot_shape_len: 0, hot_shape_keys: [std::ptr::null(); 8], @@ -145,6 +179,7 @@ impl<'a> DirectParser<'a> { Self { input, pos: 0, + valid: true, shape: Some(shape), hot_shape_len: 0, hot_shape_keys: [std::ptr::null(); 8], @@ -221,14 +256,12 @@ impl<'a> DirectParser<'a> { } } - /// After parsing the top-level value, returns `true` if any - /// non-whitespace input remains. JSON.parse must reject such trailing - /// tokens (`JSON.parse("{}x")`, `JSON.parse("1 2")`) with a `SyntaxError`; - /// trailing whitespace (`"{}\n"`) is allowed. - #[inline] - pub(crate) fn has_trailing_content(&mut self) -> bool { + /// The direct parser constructs values and validates syntax in the same + /// pass. Call this after the root value to reject both a grammar failure + /// and a second non-whitespace root token. + pub(crate) fn finish(&mut self) -> bool { self.skip_whitespace(); - self.pos < self.input.len() + self.valid && self.pos == self.input.len() } #[inline] @@ -238,10 +271,17 @@ impl<'a> DirectParser<'a> { self.advance(); true } else { + self.valid = false; false } } + #[inline] + fn invalid_value(&mut self) -> JSValue { + self.valid = false; + JSValue::null() + } + pub(crate) unsafe fn parse_value(&mut self) -> JSValue { self.skip_whitespace(); match self.peek() { @@ -252,7 +292,7 @@ impl<'a> DirectParser<'a> { Some(b'f') => self.parse_false(), Some(b'n') => self.parse_null(), Some(c) if c == b'-' || c.is_ascii_digit() => self.parse_number(), - _ => JSValue::null(), + _ => self.invalid_value(), } } @@ -275,7 +315,10 @@ impl<'a> DirectParser<'a> { // `PERRY_SSO_FORCE` env var retained as a no-op kept // alive for release-note compatibility — any value // still falls through to the unconditional SSO emit. - if let Some(sso) = JSValue::try_short_string(b) { + if b.len() <= crate::value::SHORT_STRING_MAX_LEN + && !crate::string::bytes_have_lone_surrogate(b) + { + let sso = JSValue::short_string_unchecked(b); return sso; } // ASCII fast path: skip `compute_utf16_len`'s byte scan @@ -286,14 +329,18 @@ impl<'a> DirectParser<'a> { // (16 B/it on aarch64 NEON) so it costs ~1 ns/byte and // saves the equivalent walk inside `compute_utf16_len` // plus the conditional widening for non-ASCII counters. - let ptr = if b.is_ascii() { - crate::string::js_string_from_ascii_bytes(b.as_ptr(), b.len() as u32) - } else { - js_string_from_bytes(b.as_ptr(), b.len() as u32) + let ptr = match s { + ParsedStr::Borrowed(b) if b.is_ascii() => { + crate::string::js_string_from_ascii_bytes(b.as_ptr(), b.len() as u32) + } + ParsedStr::Borrowed(b) => js_string_from_bytes(b.as_ptr(), b.len() as u32), + // Escaped strings live in a Rust Vec, so the builder can derive + // the WTF-8 lone-surrogate flag while allocating the result. + ParsedStr::Owned(ref b) => crate::string::js_string_from_builder_bytes(b), }; JSValue::string_ptr(ptr) } else { - JSValue::null() + self.invalid_value() } } @@ -308,6 +355,7 @@ impl<'a> DirectParser<'a> { /// exactly once before the scalar tail handles the boundary. pub(crate) fn parse_string_bytes(&mut self) -> Option> { if self.peek() != Some(b'"') { + self.valid = false; return None; } self.advance(); @@ -326,9 +374,14 @@ impl<'a> DirectParser<'a> { self.pos += 1; return Some(ParsedStr::Borrowed(slice)); } + if ch < 0x20 { + self.valid = false; + return None; + } // ch == b'\\' — slow path from here. return self.parse_string_bytes_slow(start); } + self.valid = false; None } @@ -336,6 +389,7 @@ impl<'a> DirectParser<'a> { let mut result = Vec::from(&self.input[start..self.pos]); loop { if self.pos >= self.input.len() { + self.valid = false; return None; } let ch = self.input[self.pos]; @@ -344,6 +398,7 @@ impl<'a> DirectParser<'a> { b'"' => return Some(ParsedStr::Owned(result)), b'\\' => { if self.pos >= self.input.len() { + self.valid = false; return None; } let esc = self.input[self.pos]; @@ -359,43 +414,49 @@ impl<'a> DirectParser<'a> { b'f' => result.push(0x0C), b'u' => { if self.pos + 4 > self.input.len() { + self.valid = false; return None; } - let hex = - std::str::from_utf8(&self.input[self.pos..self.pos + 4]).ok()?; - let code = u16::from_str_radix(hex, 16).ok()?; + let Some(code) = decode_hex_u16(&self.input[self.pos..self.pos + 4]) + else { + self.valid = false; + return None; + }; self.pos += 4; - if (0xD800..=0xDBFF).contains(&code) { - if self.pos + 6 <= self.input.len() - && self.input[self.pos] == b'\\' - && self.input[self.pos + 1] == b'u' - { - let hex2 = std::str::from_utf8( - &self.input[self.pos + 2..self.pos + 6], - ) - .ok()?; - let low = u16::from_str_radix(hex2, 16).ok()?; - self.pos += 6; - let codepoint = 0x10000 - + ((code as u32 - 0xD800) << 10) - + (low as u32 - 0xDC00); - if let Some(c) = char::from_u32(codepoint) { - let mut buf = [0u8; 4]; - let s = c.encode_utf8(&mut buf); - result.extend_from_slice(s.as_bytes()); - } - } + let paired_low = if (0xD800..=0xDBFF).contains(&code) + && self.pos + 6 <= self.input.len() + && self.input[self.pos] == b'\\' + && self.input[self.pos + 1] == b'u' + { + decode_hex_u16(&self.input[self.pos + 2..self.pos + 6]) + .filter(|low| (0xDC00..=0xDFFF).contains(low)) } else { - if let Some(c) = char::from_u32(code as u32) { - let mut buf = [0u8; 4]; - let s = c.encode_utf8(&mut buf); - result.extend_from_slice(s.as_bytes()); - } + None + }; + if let Some(low) = paired_low { + self.pos += 6; + let codepoint = 0x10000 + + ((code as u32 - 0xD800) << 10) + + (low as u32 - 0xDC00); + let c = char::from_u32(codepoint) + .expect("surrogate pair is a Unicode scalar"); + let mut buf = [0u8; 4]; + let s = c.encode_utf8(&mut buf); + result.extend_from_slice(s.as_bytes()); + } else { + push_code_unit_wtf8(&mut result, code); } } - _ => result.push(esc), + _ => { + self.valid = false; + return None; + } } } + c if c < 0x20 => { + self.valid = false; + return None; + } _ => result.push(ch), } } @@ -465,6 +526,9 @@ impl<'a> DirectParser<'a> { // shaped record are NOT themselves expected to match the // shape (shape is one-level deep by design in Step 1b). let value = self.parse_value_generic(); + if !self.valid { + break; + } // JSON.parse suppresses GC for the whole parse, so there is // no collection point between `parse_value_generic` and the // direct/slow-path field write below. @@ -596,6 +660,9 @@ impl<'a> DirectParser<'a> { } else { self.parse_value_generic() }; + if !self.valid { + break; + } js_arr = parse_root_array_ptr(arr_slot); // GC is suppressed for the whole typed parse, so array growth // cannot collect before `value` is stored. @@ -630,7 +697,7 @@ impl<'a> DirectParser<'a> { Some(b'f') => self.parse_false(), Some(b'n') => self.parse_null(), Some(c) if c == b'-' || c.is_ascii_digit() => self.parse_number(), - _ => JSValue::null(), + _ => self.invalid_value(), } } @@ -685,6 +752,9 @@ impl<'a> DirectParser<'a> { } let value = self.parse_value(); + if !self.valid { + break; + } // JSON.parse suppresses GC for the whole parse, so key // interning cannot collect before `value` is copied into // the temporary values vector below. @@ -793,6 +863,9 @@ impl<'a> DirectParser<'a> { loop { let value = self.parse_value(); + if !self.valid { + break; + } js_arr = parse_root_array_ptr(arr_slot); // GC is suppressed for the whole direct parse, so array growth // cannot collect before `value` is stored. @@ -821,8 +894,22 @@ impl<'a> DirectParser<'a> { self.advance(); } let int_start = self.pos; - while self.pos < self.input.len() && self.input[self.pos].is_ascii_digit() { - self.pos += 1; + match self.peek() { + Some(b'0') => { + self.advance(); + if self.peek().is_some_and(|byte| byte.is_ascii_digit()) { + while self.peek().is_some_and(|byte| byte.is_ascii_digit()) { + self.advance(); + } + return self.invalid_value(); + } + } + Some(b'1'..=b'9') => { + while self.peek().is_some_and(|byte| byte.is_ascii_digit()) { + self.advance(); + } + } + _ => return self.invalid_value(), } let int_end = self.pos; @@ -876,6 +963,9 @@ impl<'a> DirectParser<'a> { let frac_end = self.pos; let exp_after_frac = self.pos < self.input.len() && (self.input[self.pos] == b'e' || self.input[self.pos] == b'E'); + if frac_end == frac_start { + return self.invalid_value(); + } let int_len = int_end - int_start; let frac_len = frac_end - frac_start; if !exp_after_frac @@ -921,14 +1011,20 @@ impl<'a> DirectParser<'a> { { self.pos += 1; } + let exponent_start = self.pos; while self.pos < self.input.len() && self.input[self.pos].is_ascii_digit() { self.pos += 1; } + if self.pos == exponent_start { + return self.invalid_value(); + } } let num_str = std::str::from_utf8_unchecked(&self.input[start..self.pos]); - let value: f64 = num_str.parse().unwrap_or(0.0); - JSValue::number(value) + match num_str.parse::() { + Ok(value) => JSValue::number(value), + Err(_) => self.invalid_value(), + } } pub(crate) unsafe fn parse_true(&mut self) -> JSValue { @@ -936,7 +1032,7 @@ impl<'a> DirectParser<'a> { self.pos += 4; JSValue::bool(true) } else { - JSValue::null() + self.invalid_value() } } @@ -945,14 +1041,16 @@ impl<'a> DirectParser<'a> { self.pos += 5; JSValue::bool(false) } else { - JSValue::null() + self.invalid_value() } } pub(crate) unsafe fn parse_null(&mut self) -> JSValue { if self.pos + 4 <= self.input.len() && &self.input[self.pos..self.pos + 4] == b"null" { self.pos += 4; + JSValue::null() + } else { + self.invalid_value() } - JSValue::null() } } diff --git a/crates/perry-runtime/src/json/simd.rs b/crates/perry-runtime/src/json/simd.rs index 5d03c83469..d1448c0306 100644 --- a/crates/perry-runtime/src/json/simd.rs +++ b/crates/perry-runtime/src/json/simd.rs @@ -1,7 +1,7 @@ //! SIMD-accelerated string-terminator scanning used by the direct JSON parser. -/// Find the offset of the first `"` or `\` in `bytes`. Returns `None` -/// if neither is found before end-of-input (which is a JSON error — the +/// Find the first `"`, `\`, or raw control byte in `bytes`. Returns `None` +/// if none is found before end-of-input (which is a JSON error — the /// caller handles that by failing the parse). /// /// Issue #179 tier 1 #3: SIMD-accelerated on aarch64 (NEON) and x86_64 @@ -35,7 +35,7 @@ pub(crate) fn find_string_terminator(bytes: &[u8]) -> Option { #[inline(always)] pub(crate) fn find_string_terminator_scalar(bytes: &[u8]) -> Option { for (i, &b) in bytes.iter().enumerate() { - if b == b'"' || b == b'\\' { + if b == b'"' || b == b'\\' || b < 0x20 { return Some(i); } } @@ -49,12 +49,14 @@ pub(crate) fn find_string_terminator_neon(bytes: &[u8]) -> Option { unsafe { let quote = vdupq_n_u8(b'"'); let bslash = vdupq_n_u8(b'\\'); + let space = vdupq_n_u8(0x20); let mut i: usize = 0; while i + 16 <= bytes.len() { let chunk = vld1q_u8(bytes.as_ptr().add(i)); let eq_q = vceqq_u8(chunk, quote); let eq_b = vceqq_u8(chunk, bslash); - let mask = vorrq_u8(eq_q, eq_b); + let control = vcltq_u8(chunk, space); + let mask = vorrq_u8(vorrq_u8(eq_q, eq_b), control); // Fast rejection: reduce the 16-byte mask to a single byte // (max across all lanes). Zero => no match in this chunk. if vmaxvq_u8(mask) == 0 { @@ -88,12 +90,14 @@ pub(crate) fn find_string_terminator_sse2(bytes: &[u8]) -> Option { unsafe { let quote = _mm_set1_epi8(b'"' as i8); let bslash = _mm_set1_epi8(b'\\' as i8); + let control_max = _mm_set1_epi8(0x1F); let mut i: usize = 0; while i + 16 <= bytes.len() { let chunk = _mm_loadu_si128(bytes.as_ptr().add(i) as *const _); let eq_q = _mm_cmpeq_epi8(chunk, quote); let eq_b = _mm_cmpeq_epi8(chunk, bslash); - let mask = _mm_or_si128(eq_q, eq_b); + let control = _mm_cmpeq_epi8(_mm_min_epu8(chunk, control_max), chunk); + let mask = _mm_or_si128(_mm_or_si128(eq_q, eq_b), control); let bitmask = _mm_movemask_epi8(mask) as u32; if bitmask != 0 { return Some(i + bitmask.trailing_zeros() as usize); diff --git a/crates/perry/tests/issue_9184_json_parse_strict.rs b/crates/perry/tests/issue_9184_json_parse_strict.rs new file mode 100644 index 0000000000..968b132138 --- /dev/null +++ b/crates/perry/tests/issue_9184_json_parse_strict.rs @@ -0,0 +1,54 @@ +//! #9184: the direct JSON parser validates in the same pass that constructs +//! Perry values, allowing the separate serde validation scan to be removed. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn workspace_root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../..") + .canonicalize() + .expect("canonicalize workspace root") +} + +#[test] +fn json_parse_matches_node_across_strict_syntax_battery() { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = workspace_root().join("test-files/test_issue_9184_json_parse_strict.ts"); + let output = dir.path().join("main_bin"); + + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-auto-optimize") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output) + .env("PERRY_JSON_TAPE", "0") + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled program failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&run.stdout), + "invalid:43/43:0\nvalid:28/28:0\ntyped:SyntaxError:2:1:2\n" + ); +} diff --git a/scripts/string_payload_access_baseline.txt b/scripts/string_payload_access_baseline.txt index b12ccf3dc2..357cc835c3 100644 --- a/scripts/string_payload_access_baseline.txt +++ b/scripts/string_payload_access_baseline.txt @@ -12,7 +12,7 @@ inline-offset | perry-ext-nodemailer | 1 inline-offset | perry-ext-pg | 2 inline-offset | perry-ext-zlib | 3 inline-offset | perry-ffi | 3 -inline-offset | perry-runtime | 363 +inline-offset | perry-runtime | 360 inline-offset | perry-stdlib | 48 inline-offset | perry-updater | 5 reader-helper | perry-ext-ethers | 1 diff --git a/test-files/test_issue_9184_json_parse_strict.ts b/test-files/test_issue_9184_json_parse_strict.ts new file mode 100644 index 0000000000..c464697b1e --- /dev/null +++ b/test-files/test_issue_9184_json_parse_strict.ts @@ -0,0 +1,135 @@ +// #9184: JSON.parse must validate while DirectParser builds the value. These +// cases intentionally cover truncation, separators, numbers, strings, and +// trailing roots so removing an external validation pass stays safe. +const bs = String.fromCharCode(92); +const invalid: string[] = [ + "{", + "}", + "[", + "]", + "", + " ", + "{,}", + "[,]", + "[1,]", + "{\"a\":1,}", + "{a:1}", + "{'a':1}", + "[01]", + "[-01]", + "[1.]", + "[.5]", + "[+1]", + "[1e]", + "[1e+]", + "[--1]", + "[NaN]", + "[Infinity]", + "[-Infinity]", + "[undefined]", + "[TRUE]", + "\"unterminated", + "[\"bad" + bs + "x\"]", + "[\"" + bs + "u12\"]", + "[\"" + bs + "uZZZZ\"]", + "{\"a\" 1}", + "{\"a\":}", + "{:1}", + "[1 2]", + "[1][2]", + "{}{}", + "nul", + "tru", + "[1,,2]", + "{\"a\":1 \"b\":2}", + "\"" + bs + "t\"x", + "\"\t\"", + "\"abcdefghijklmnop\nqrst\"", + "\vnull", +]; + +const valid: [string, string][] = [ + ["{}", "{}"], + ["[]", "[]"], + ["0", "0"], + ["-0", "0"], + ["1e5", "100000"], + ["1E+5", "100000"], + ["1e-5", "0.00001"], + ["-1.5", "-1.5"], + ["null", "null"], + ["true", "true"], + ["false", "false"], + ["\"\"", "\"\""], + ["\"" + bs + "u0041\"", "\"A\""], + ["\"" + bs + "n\"", "\"" + bs + "n\""], + ["[1,2,3]", "[1,2,3]"], + ["{\"a\":{\"b\":[1,{\"c\":null}]}}", "{\"a\":{\"b\":[1,{\"c\":null}]}}"], + ["{\"a\":1,\"a\":2}", "{\"a\":2}"], + ["[[[[[1]]]]]", "[[[[[1]]]]]"], + ["\"" + bs + "ud83d" + bs + "ude00\"", "\"😀\""], + ["\"" + bs + "ud800\"", "\"" + bs + "ud800\""], + ["\"" + bs + "ud800" + bs + "u0041\"", "\"" + bs + "ud800A\""], + ["\"" + bs + "udc00\"", "\"" + bs + "udc00\""], + ["{\"\":1}", "{\"\":1}"], + [" {\"a\" : 1 } ", "{\"a\":1}"], + ["[1e308]", "[1e+308]"], + ["[-1e308]", "[-1e+308]"], + ["[1e-400]", "[0]"], + ["9007199254740993", "9007199254740992"], +]; + +let correctThrows = 0; +let invalidFailures = 0; +for (let i = 0; i < invalid.length; i++) { + try { + JSON.parse(invalid[i]); + invalidFailures++; + } catch (e: any) { + if (e && e.constructor && e.constructor.name === "SyntaxError") { + correctThrows++; + } else { + invalidFailures++; + } + } +} + +let correctValues = 0; +let validFailures = 0; +for (let i = 0; i < valid.length; i++) { + try { + const actual = JSON.stringify(JSON.parse(valid[i][0])); + if (actual === valid[i][1]) correctValues++; + else { + console.log("valid-mismatch:" + i + ":" + actual + ":" + valid[i][1]); + validFailures++; + } + } catch (_e: any) { + validFailures++; + } +} + +console.log("invalid:" + correctThrows + "/" + invalid.length + ":" + invalidFailures); +console.log("valid:" + correctValues + "/" + valid.length + ":" + validFailures); + +interface StrictRow { + id: number; +} + +let typedError = "none"; +try { + JSON.parse("[{\"id\":1},]"); +} catch (e: any) { + if (e && e.constructor) typedError = e.constructor.name; +} +const typedRows = JSON.parse("[{\"id\":1},{\"id\":2}]"); +console.log( + "typed:" + + typedError + + ":" + + typedRows.length + + ":" + + typedRows[0].id + + ":" + + typedRows[1].id, +);