From b0e5609b88deae3f30479277cbeb7691e8c4ea5f Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Sun, 6 Feb 2022 22:02:17 -0500 Subject: [PATCH] LibJS: Use GetV to look up the toJSON property in SerializeJSONProperty The current implementation of step 2a sort of manually implemented GetV with a ToObject + Get combo. But in the call to Get, the receiver wasn't the correct object. So when invoking toJSON, the receiver was an Object type rather than a BigInt. This also adds spec comments to SerializeJSONProperty. --- .../Libraries/LibJS/Runtime/JSONObject.cpp | 69 ++++++++++++++++--- .../Tests/builtins/JSON/JSON.stringify.js | 13 ++++ 2 files changed, 71 insertions(+), 11 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/JSONObject.cpp b/Userland/Libraries/LibJS/Runtime/JSONObject.cpp index 4daba847c4..f1e014c9bb 100644 --- a/Userland/Libraries/LibJS/Runtime/JSONObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/JSONObject.cpp @@ -128,48 +128,95 @@ JS_DEFINE_NATIVE_FUNCTION(JSONObject::stringify) ThrowCompletionOr JSONObject::serialize_json_property(GlobalObject& global_object, StringifyState& state, const PropertyKey& key, Object* holder) { auto& vm = global_object.vm(); + + // 1. Let value be ? Get(holder, key). auto value = TRY(holder->get(key)); + + // 2. If Type(value) is Object or BigInt, then if (value.is_object() || value.is_bigint()) { - auto* value_object = TRY(value.to_object(global_object)); - auto to_json = TRY(value_object->get(vm.names.toJSON)); - if (to_json.is_function()) + // a. Let toJSON be ? GetV(value, "toJSON"). + auto to_json = TRY(value.get(global_object, vm.names.toJSON)); + + // b. If IsCallable(toJSON) is true, then + if (to_json.is_function()) { + // i. Set value to ? Call(toJSON, value, « key »). value = TRY(call(global_object, to_json.as_function(), value, js_string(vm, key.to_string()))); + } } - if (state.replacer_function) + // 3. If state.[[ReplacerFunction]] is not undefined, then + if (state.replacer_function) { + // a. Set value to ? Call(state.[[ReplacerFunction]], holder, « key, value »). value = TRY(call(global_object, *state.replacer_function, holder, js_string(vm, key.to_string()), value)); + } + // 4. If Type(value) is Object, then if (value.is_object()) { auto& value_object = value.as_object(); - if (is(value_object)) + + // a. If value has a [[NumberData]] internal slot, then + if (is(value_object)) { + // i. Set value to ? ToNumber(value). value = TRY(value.to_number(global_object)); - else if (is(value_object)) + } + // b. Else if value has a [[StringData]] internal slot, then + else if (is(value_object)) { + // i. Set value to ? ToString(value). value = TRY(value.to_primitive_string(global_object)); - else if (is(value_object)) + } + // c. Else if value has a [[BooleanData]] internal slot, then + else if (is(value_object)) { + // i. Set value to value.[[BooleanData]]. value = Value(static_cast(value_object).boolean()); - else if (is(value_object)) + } + // d. Else if value has a [[BigIntData]] internal slot, then + else if (is(value_object)) { + // i. Set value to value.[[BigIntData]]. value = Value(&static_cast(value_object).bigint()); + } } + // 5. If value is null, return "null". if (value.is_null()) - return "null"; + return "null"sv; + + // 6. If value is true, return "true". + // 7. If value is false, return "false". if (value.is_boolean()) - return value.as_bool() ? "true" : "false"; + return value.as_bool() ? "true"sv : "false"sv; + + // 8. If Type(value) is String, return QuoteJSONString(value). if (value.is_string()) return quote_json_string(value.as_string().string()); + + // 9. If Type(value) is Number, then if (value.is_number()) { + // a. If value is finite, return ! ToString(value). if (value.is_finite_number()) return MUST(value.to_string(global_object)); - return "null"; + + // b. Return "null". + return "null"sv; } + + // 10. If Type(value) is BigInt, throw a TypeError exception. if (value.is_bigint()) return vm.throw_completion(global_object, ErrorType::JsonBigInt); + + // 11. If Type(value) is Object and IsCallable(value) is false, then if (value.is_object() && !value.is_function()) { + // a. Let isArray be ? IsArray(value). auto is_array = TRY(value.is_array(global_object)); + + // b. If isArray is true, return ? SerializeJSONArray(state, value). if (is_array) return serialize_json_array(global_object, state, static_cast(value.as_object())); + + // c. Return ? SerializeJSONObject(state, value). return serialize_json_object(global_object, state, value.as_object()); } + + // 12. Return undefined. return String {}; } diff --git a/Userland/Libraries/LibJS/Tests/builtins/JSON/JSON.stringify.js b/Userland/Libraries/LibJS/Tests/builtins/JSON/JSON.stringify.js index ae4398d9ad..e1630ee7a0 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/JSON/JSON.stringify.js +++ b/Userland/Libraries/LibJS/Tests/builtins/JSON/JSON.stringify.js @@ -38,6 +38,19 @@ describe("correct behavior", () => { }); }); + test("serialize BigInt with a toJSON property", () => { + Object.defineProperty(BigInt.prototype, "toJSON", { + configurable: true, // Allows deleting this property at the end of this test case. + get() { + "use strict"; + return () => typeof this; + }, + }); + + expect(JSON.stringify(1n)).toBe('"bigint"'); + delete BigInt.prototype.toJSON; + }); + test("ignores non-enumerable properties", () => { let o = { foo: "bar" }; Object.defineProperty(o, "baz", { value: "qux", enumerable: false });