mirror of
https://github.com/RGBCube/serenity
synced 2025-07-25 05:07:34 +00:00
LibJS: Handle circular references in Array.prototype.join()
This fixes Array.prototype.{join,toString}() crashing with arrays containing themselves, i.e. circular references. The spec is suspiciously silent about this, and indeed engine262, a "100% spec compliant" ECMA-262 implementation, can't handle these cases. I had a look at some major engines instead and they all seem to keep track or check for circular references and return an empty string for already seen objects. - SpiderMonkey: "AutoCycleDetector detector(cx, obj)" - V8: "CycleProtectedArrayJoin<JSArray>(...)" - JavaScriptCore: "StringRecursionChecker checker(globalObject, thisObject)" - ChakraCore: "scriptContext->CheckObject(thisArg)" To keep things simple & consistent this uses the same pattern as JSONObject, MarkupGenerator and js: simply putting each seen object in a HashTable<Object*>. Fixes #3929.
This commit is contained in:
parent
e5845ba3a0
commit
0603402c80
2 changed files with 21 additions and 0 deletions
|
@ -27,6 +27,7 @@
|
||||||
*/
|
*/
|
||||||
|
|
||||||
#include <AK/Function.h>
|
#include <AK/Function.h>
|
||||||
|
#include <AK/HashTable.h>
|
||||||
#include <AK/StringBuilder.h>
|
#include <AK/StringBuilder.h>
|
||||||
#include <LibJS/Runtime/Array.h>
|
#include <LibJS/Runtime/Array.h>
|
||||||
#include <LibJS/Runtime/ArrayIterator.h>
|
#include <LibJS/Runtime/ArrayIterator.h>
|
||||||
|
@ -39,6 +40,8 @@
|
||||||
|
|
||||||
namespace JS {
|
namespace JS {
|
||||||
|
|
||||||
|
static HashTable<Object*> s_array_join_seen_objects;
|
||||||
|
|
||||||
ArrayPrototype::ArrayPrototype(GlobalObject& global_object)
|
ArrayPrototype::ArrayPrototype(GlobalObject& global_object)
|
||||||
: Object(*global_object.object_prototype())
|
: Object(*global_object.object_prototype())
|
||||||
{
|
{
|
||||||
|
@ -316,6 +319,13 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayPrototype::join)
|
||||||
auto* this_object = vm.this_value(global_object).to_object(global_object);
|
auto* this_object = vm.this_value(global_object).to_object(global_object);
|
||||||
if (!this_object)
|
if (!this_object)
|
||||||
return {};
|
return {};
|
||||||
|
|
||||||
|
// This is not part of the spec, but all major engines do some kind of circular reference checks.
|
||||||
|
// FWIW: engine262, a "100% spec compliant" ECMA-262 impl, aborts with "too much recursion".
|
||||||
|
if (s_array_join_seen_objects.contains(this_object))
|
||||||
|
return js_string(vm, "");
|
||||||
|
s_array_join_seen_objects.set(this_object);
|
||||||
|
|
||||||
auto length = get_length(vm, *this_object);
|
auto length = get_length(vm, *this_object);
|
||||||
if (vm.exception())
|
if (vm.exception())
|
||||||
return {};
|
return {};
|
||||||
|
@ -339,6 +349,9 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayPrototype::join)
|
||||||
return {};
|
return {};
|
||||||
builder.append(string);
|
builder.append(string);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
s_array_join_seen_objects.remove(this_object);
|
||||||
|
|
||||||
return js_string(vm, builder.to_string());
|
return js_string(vm, builder.to_string());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -14,3 +14,11 @@ test("basic functionality", () => {
|
||||||
expect([1, null, 2, undefined, 3].join()).toBe("1,,2,,3");
|
expect([1, null, 2, undefined, 3].join()).toBe("1,,2,,3");
|
||||||
expect(Array(3).join()).toBe(",,");
|
expect(Array(3).join()).toBe(",,");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("circular references", () => {
|
||||||
|
const a = ["foo", [], [1, 2, []], ["bar"]];
|
||||||
|
a[1] = a;
|
||||||
|
a[2][2] = a;
|
||||||
|
// [ "foo", <circular>, [ 1, 2, <circular> ], [ "bar" ] ]
|
||||||
|
expect(a.join()).toBe("foo,,1,2,,bar");
|
||||||
|
});
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue