diff --git a/changelog.d/10392-fetch-json-key-order.md b/changelog.d/10392-fetch-json-key-order.md new file mode 100644 index 0000000000..23a793a7be --- /dev/null +++ b/changelog.d/10392-fetch-json-key-order.md @@ -0,0 +1,5 @@ +### Fixed + +- `Request.json()` and `Response.json()` now preserve JSON document order for + non-index object keys while retaining JavaScript's numeric-key ordering + (#10392, PR #11049). diff --git a/crates/perry-stdlib/src/fetch/mod.rs b/crates/perry-stdlib/src/fetch/mod.rs index c98dbc2e0e..a72f82bcbc 100644 --- a/crates/perry-stdlib/src/fetch/mod.rs +++ b/crates/perry-stdlib/src/fetch/mod.rs @@ -3,10 +3,7 @@ //! Native implementation of the 'node-fetch' npm package using reqwest. //! Provides fetch() function for making HTTP requests. -use perry_runtime::{ - js_array_alloc, js_array_push, js_object_alloc, js_object_set_field, js_object_set_keys, - js_string_from_bytes, JSValue, StringHeader, -}; +use perry_runtime::{js_string_from_bytes, JSValue, StringHeader}; use std::cell::Cell; use std::collections::HashMap; use std::sync::Mutex; @@ -1100,47 +1097,14 @@ pub unsafe extern "C" fn js_fetch_response_text(handle: f64) -> *mut perry_runti promise } -/// Convert serde_json::Value to JSValue -unsafe fn json_value_to_jsvalue(value: &serde_json::Value) -> JSValue { - match value { - serde_json::Value::Null => JSValue::null(), - serde_json::Value::Bool(b) => JSValue::bool(*b), - serde_json::Value::Number(n) => { - if let Some(f) = n.as_f64() { - JSValue::number(f) - } else if let Some(i) = n.as_i64() { - JSValue::number(i as f64) - } else { - JSValue::number(0.0) - } - } - serde_json::Value::String(s) => { - let ptr = js_string_from_bytes(s.as_ptr(), s.len() as u32); - JSValue::string_ptr(ptr) - } - serde_json::Value::Array(arr) => { - let js_arr = js_array_alloc(arr.len() as u32); - for item in arr { - js_array_push(js_arr, json_value_to_jsvalue(item)); - } - JSValue::object_ptr(js_arr as *mut u8) - } - serde_json::Value::Object(obj) => { - let js_obj = js_object_alloc(0, obj.len() as u32); - // Create keys array for property names - let keys_arr = js_array_alloc(obj.len() as u32); - for (idx, (key, val)) in obj.iter().enumerate() { - // Add key to keys array - let key_ptr = js_string_from_bytes(key.as_ptr(), key.len() as u32); - js_array_push(keys_arr, JSValue::string_ptr(key_ptr)); - // Set field value - js_object_set_field(js_obj, idx as u32, json_value_to_jsvalue(val)); - } - // Associate keys with object - js_object_set_keys(js_obj, keys_arr); - JSValue::object_ptr(js_obj as *mut u8) - } - } +/// Parse a Fetch body with the runtime's `JSON.parse` implementation. Besides +/// matching JavaScript number and error semantics, this preserves document +/// order for non-index object keys; `serde_json::Value` uses a sorted map in +/// this build and silently reordered them (#10392). +unsafe fn parse_json_body(body: &[u8]) -> Result { + let text = String::from_utf8_lossy(body); + let text_ptr = js_string_from_bytes(text.as_ptr(), text.len() as u32); + perry_runtime::json::js_json_parse_result(text_ptr) } /// Get response body as JSON (parses and returns proper JS object) @@ -1161,19 +1125,15 @@ pub unsafe extern "C" fn js_fetch_response_json(handle: f64) -> *mut perry_runti } }; - // Convert body to string and parse as JSON. Resolve the promise - // synchronously — see comment on `js_fetch_response_text`. - let text = String::from_utf8_lossy(&body).to_string(); - match serde_json::from_str::(&text) { - Ok(json_value) => { - let js_value = json_value_to_jsvalue(&json_value); + // Parse and resolve synchronously — see comment on + // `js_fetch_response_text`. + match parse_json_body(&body) { + Ok(js_value) => { let result_nan = f64::from_bits(js_value.bits()); perry_runtime::js_promise_resolve(promise, result_nan); } - Err(e) => { - let err_msg = format!("JSON parse error: {}", e); - let err_nan = f64::from_bits(fetch_error_bits(&err_msg)); - perry_runtime::js_promise_reject(promise, err_nan); + Err(error) => { + perry_runtime::js_promise_reject(promise, error); } } @@ -1918,15 +1878,12 @@ pub unsafe extern "C" fn js_request_json(handle: f64) -> *mut perry_runtime::Pro return promise; } }; - let text = String::from_utf8_lossy(&body).to_string(); - match serde_json::from_str::(&text) { - Ok(json_value) => { - let js_value = json_value_to_jsvalue(&json_value); + match parse_json_body(&body) { + Ok(js_value) => { perry_runtime::js_promise_resolve(promise, f64::from_bits(js_value.bits())); } - Err(e) => { - let err_nan = f64::from_bits(fetch_error_bits(&format!("JSON parse error: {}", e))); - perry_runtime::js_promise_reject(promise, err_nan); + Err(error) => { + perry_runtime::js_promise_reject(promise, error); } } promise diff --git a/crates/perry-stdlib/src/fetch/tests.rs b/crates/perry-stdlib/src/fetch/tests.rs index 11b5fa23e6..2139a8b90e 100644 --- a/crates/perry-stdlib/src/fetch/tests.rs +++ b/crates/perry-stdlib/src/fetch/tests.rs @@ -1,5 +1,20 @@ use super::*; +#[test] +fn fetch_json_preserves_document_key_order() { + let source = br#"{"title":"t","permission":[],"zeta":1,"10":"ten","2":"two"}"#; + let parsed = unsafe { parse_json_body(source) }.expect("valid JSON body"); + let output = + unsafe { perry_runtime::json::js_json_stringify(f64::from_bits(parsed.bits()), 0) }; + assert!(!output.is_null()); + let output = unsafe { perry_ffi::JsString::from_raw(output.cast::()) }; + let bytes = perry_ffi::read_bytes(output).expect("JSON.stringify returned a string"); + assert_eq!( + bytes, + br#"{"2":"two","10":"ten","title":"t","permission":[],"zeta":1}"# + ); +} + /// #8546: Coop hosts each in-process deployment on its own dedicated Perry /// thread. The Fetch scanner registry is thread-local, so a process-global /// registration latch makes the first Next application safe and leaves the diff --git a/scripts/unrooted_local_shape_baseline.json b/scripts/unrooted_local_shape_baseline.json index bc1153f514..1a216a0ea3 100644 --- a/scripts/unrooted_local_shape_baseline.json +++ b/scripts/unrooted_local_shape_baseline.json @@ -34,7 +34,6 @@ "crates/perry-stdlib/src/events/module_helpers.rs": 1, "crates/perry-stdlib/src/events/once_helpers.rs": 1, "crates/perry-stdlib/src/events/warnings.rs": 1, - "crates/perry-stdlib/src/fetch/mod.rs": 6, "crates/perry-stdlib/src/lodash.rs": 21, "crates/perry-stdlib/src/querystring.rs": 2, "crates/perry-stdlib/src/readline/mod.rs": 4, @@ -65,5 +64,5 @@ "crates/perry-stdlib/src/zlib.rs": 3 }, "schema_version": 3, - "total": 390 + "total": 384 } diff --git a/test-files/test_gap_10392_fetch_json_key_order.ts b/test-files/test_gap_10392_fetch_json_key_order.ts new file mode 100644 index 0000000000..9250c83dfc --- /dev/null +++ b/test-files/test_gap_10392_fetch_json_key_order.ts @@ -0,0 +1,12 @@ +const body = '{"title":"t","permission":[],"zeta":1,"10":"ten","2":"two"}'; + +const request = await new Request("http://example.com/", { + method: "POST", + body, +}).json(); +console.log("request json", JSON.stringify(request)); +console.log("request keys", JSON.stringify(Object.keys(request))); + +const response = await new Response(body).json(); +console.log("response json", JSON.stringify(response)); +console.log("response keys", JSON.stringify(Object.keys(response)));