Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions JSTests/stress/json-stringify-non-enumerable-to-json.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
function shouldBe(actual, expected)
{
if (actual !== expected)
throw new Error('bad value: ' + actual);
}

// SerializeJSONProperty looks up toJSON with GetV, so an own toJSON is called
// regardless of its enumerability. The fast stringifier skips DontEnum
// properties, so it has to check for toJSON before doing that.
function withNonEnumerableToJSON(object, toJSON)
{
Object.defineProperty(object, 'toJSON', { value: toJSON, enumerable: false, writable: false, configurable: false });
return object;
}

{
let object = withNonEnumerableToJSON({ uri: 'u', cid: 'c', text: 't' }, function () { return { uri: this.uri, cid: this.cid }; });
Object.freeze(object);
shouldBe(JSON.stringify(object), '{"uri":"u","cid":"c"}');
shouldBe(JSON.stringify({ record: object }), '{"record":{"uri":"u","cid":"c"}}');
shouldBe(JSON.stringify([object]), '[{"uri":"u","cid":"c"}]');
shouldBe(JSON.stringify(object, null, 2), '{\n "uri": "u",\n "cid": "c"\n}');
}

{
shouldBe(JSON.stringify(withNonEnumerableToJSON({ a: 1 }, function () { return 'replaced'; })), '"replaced"');
shouldBe(JSON.stringify(withNonEnumerableToJSON({ a: 1 }, function () { return undefined; })), undefined);
shouldBe(JSON.stringify(withNonEnumerableToJSON({ a: 1 }, function (key) { return key; })), '""');
shouldBe(JSON.stringify({ key: withNonEnumerableToJSON({ a: 1 }, function (key) { return key; }) }), '{"key":"key"}');
}

// The fast stringifier bails out on the DynamicBuffer path too.
{
let array = [];
for (let i = 0; i < 2000; ++i)
array.push(withNonEnumerableToJSON({ index: i, name: 'name' + i }, function () { return this.index; }));
let expected = '[' + array.map((_, i) => i).join(',') + ']';
shouldBe(JSON.stringify(array), expected);
}

// An own toJSON that is not callable is serialized as a normal property.
{
shouldBe(JSON.stringify(withNonEnumerableToJSON({ a: 1 }, 42)), '{"a":1}');
shouldBe(JSON.stringify({ a: 1, toJSON: 42 }), '{"a":1,"toJSON":42}');
}

// An enumerable own toJSON keeps working.
{
shouldBe(JSON.stringify({ a: 1, toJSON() { return 'enumerable'; } }), '"enumerable"');
shouldBe(JSON.stringify(Object.freeze({ a: 1, toJSON() { return 'frozen'; } })), '"frozen"');
}

// The fast path must agree with the general stringifier, which an identity
// callable replacer forces us onto.
{
let values = [
withNonEnumerableToJSON({ a: 1 }, function () { return { b: 2 }; }),
withNonEnumerableToJSON({ a: 1 }, function () { return 'string'; }),
withNonEnumerableToJSON({ a: 1 }, 'not callable'),
{ nested: withNonEnumerableToJSON({ a: 1 }, function () { return [1, 2]; }) },
[withNonEnumerableToJSON({ a: 1 }, function () { return null; })],
withNonEnumerableToJSON({ 'キー': '値' }, function () { return this['キー']; }),
];
for (let value of values) {
for (let space of [undefined, 2, '\t']) {
shouldBe(JSON.stringify(value, null, space), JSON.stringify(value, (key, x) => x, space));
}
}
}
6 changes: 6 additions & 0 deletions Source/JavaScriptCore/runtime/JSONObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1424,6 +1424,12 @@
++m_depth;
const unsigned newLineAndIndent = hasGap == HasGap::Yes ? newLineAndIndentSize() : 0;
structure.forEachProperty(m_vm, [&](const auto& entry) -> bool {
// SerializeJSONProperty looks up toJSON with GetV, so an own toJSON applies even when
// it is not enumerable. It has to be found before DontEnum properties are skipped.
if (entry.key() == m_vm.propertyNames->toJSON) [[unlikely]] {
recordFailure("object has toJSON"_s);
return false;
}

Check notice on line 1432 in Source/JavaScriptCore/runtime/JSONObject.cpp

View check run for this annotation

Claude / Claude Code Review

Pre-existing: ArrayType fast path misses toJSON on a custom array prototype

Not from this PR, but since the description cites the `ArrayType` case as the model for this fix: that path has an analogous gap for *inherited* `toJSON`. It calls `mayHaveToJSON` on `m_globalObject.arrayPrototype()` and scans the array's own properties, but never checks the array's actual `storedPrototype()` — so an array whose prototype was swapped via `Object.setPrototypeOf` to one carrying `toJSON` still fast-paths and serializes as `[1,2,3]`. The object case is safe because of the `storedPr
Comment thread
robobun marked this conversation as resolved.
Resolved
if (entry.attributes() & PropertyAttribute::DontEnum)
return true;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
auto& name = *entry.key();
Expand Down
Loading