From 032dc1d5b1819f3cb318f4470532480482d82b05 Mon Sep 17 00:00:00 2001 From: Levi Date: Fri, 26 Dec 2025 17:13:02 +0800 Subject: [PATCH 1/2] Fix stack overflow when converting arrays with circular references to primitives --- builtin_array.go | 37 ++++++++++ builtin_array_circular_test.go | 123 +++++++++++++++++++++++++++++++++ runtime.go | 4 ++ 3 files changed, 164 insertions(+) create mode 100644 builtin_array_circular_test.go diff --git a/builtin_array.go b/builtin_array.go index 147bf4b8c..ca6354baf 100644 --- a/builtin_array.go +++ b/builtin_array.go @@ -176,8 +176,38 @@ func (r *Runtime) arrayproto_pop(call FunctionCall) Value { } } +// pushToStringStack checks for circular references and pushes an object onto the toString stack. +// Returns true if the object is already in the stack (circular reference detected), false otherwise. +// If false is returned, the caller must ensure the object is popped from the stack when done. +func (r *Runtime) pushToStringStack(o *Object) bool { + // Check for circular reference in the toString stack + for _, obj := range r.toStringStack { + if o.SameAs(obj) { + // Circular reference detected + return true + } + } + + // Push this object onto the stack + r.toStringStack = append(r.toStringStack, o) + return false +} + +// popToStringStack removes an object from the toString stack. +func (r *Runtime) popToStringStack() { + r.toStringStack = r.toStringStack[:len(r.toStringStack)-1] +} + func (r *Runtime) arrayproto_join(call FunctionCall) Value { o := call.This.ToObject(r) + + if r.pushToStringStack(o) { + // Circular reference detected, return empty string to avoid infinite recursion + // This matches the behavior of mainstream JavaScript engines (V8, SpiderMonkey) + return stringEmpty + } + defer r.popToStringStack() + l := int(toLength(o.self.getStr("length", nil))) var sep String if s := call.Argument(0); s != _undefined { @@ -249,6 +279,13 @@ func (r *Runtime) writeItemLocaleString(item Value, buf *StringBuilder) { func (r *Runtime) arrayproto_toLocaleString(call FunctionCall) Value { array := call.This.ToObject(r) + + if r.pushToStringStack(array) { + // Circular reference detected, return empty string to avoid infinite recursion + return stringEmpty + } + defer r.popToStringStack() + var buf StringBuilder if a := r.checkStdArrayObj(array); a != nil { for i, item := range a.values { diff --git a/builtin_array_circular_test.go b/builtin_array_circular_test.go new file mode 100644 index 000000000..6ebebb51c --- /dev/null +++ b/builtin_array_circular_test.go @@ -0,0 +1,123 @@ +package goja + +import ( + "testing" +) + +func TestArrayCircularReferenceToString(t *testing.T) { + const SCRIPT = ` + var T = [1, 2, 3]; + T[42] = T; // Create circular reference + var str = String(T); + // Circular reference should be replaced with empty string + str === "1,2,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,"; + ` + testScript(SCRIPT, valueTrue, t) +} + +func TestArrayCircularReferenceNumericOperation(t *testing.T) { + const SCRIPT = ` + var T = [1, 2, 3]; + T[42] = T; // Create circular reference + try { + var x = T % 2; // This should not crash + true; + } catch (e) { + false; + } + ` + testScript(SCRIPT, valueTrue, t) +} + +func TestArrayCircularReferenceJoin(t *testing.T) { + const SCRIPT = ` + var T = [1, 2, 3]; + T[42] = T; // Create circular reference + var str = T.join(','); + // Circular reference should be replaced with empty string + str === "1,2,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,"; + ` + testScript(SCRIPT, valueTrue, t) +} + +func TestArrayCircularReferenceConcat(t *testing.T) { + const SCRIPT = ` + var T = [1, 2, 3]; + T[42] = T; // Create circular reference + var str = '' + T; // String concatenation + // Circular reference should be replaced with empty string + str === "1,2,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,"; + ` + testScript(SCRIPT, valueTrue, t) +} + +func TestArrayCircularReferenceToLocaleString(t *testing.T) { + const SCRIPT = ` + var T = [1, 2, 3]; + T[42] = T; // Create circular reference + var str = T.toLocaleString(); + // Circular reference should be replaced with empty string + str === "1,2,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,"; + ` + testScript(SCRIPT, valueTrue, t) +} + +func TestArrayMultipleCircularReferences(t *testing.T) { + const SCRIPT = ` + var T = [1, 2, 3]; + T[42] = T; + T[76] = T; + T[80] = T; + var str = String(T); + // Should handle multiple circular references - all should be empty strings + str.split(',').length === 81; + ` + testScript(SCRIPT, valueTrue, t) +} + +func TestArrayNestedCircularReference(t *testing.T) { + const SCRIPT = ` + var A = [1, 2]; + var B = [3, 4]; + A[2] = B; + B[2] = A; // Mutual circular reference + var str = String(A); + // A contains B which contains A - circular refs should be empty + str === "1,2,3,4,"; + ` + testScript(SCRIPT, valueTrue, t) +} + +func TestArrayCircularReferenceAccessOK(t *testing.T) { + const SCRIPT = ` + // These operations should still work fine + var T = [1, 2, 3]; + T[42] = T; + + // Accessing circular reference is OK + var same = T[42] === T; + + // Accessing elements through circular reference is OK + var first = T[42][0]; + + // Deep nesting is OK + var deep = T[42][42][42][42][42][0]; + + same && first === 1 && deep === 1; + ` + testScript(SCRIPT, valueTrue, t) +} + +func TestArrayCircularReferenceComparison(t *testing.T) { + const SCRIPT = ` + var T = [1, 2, 3]; + T[42] = T; // Create circular reference + try { + var result = T == 5; // Comparison should not crash + true; + } catch (e) { + false; + } + ` + testScript(SCRIPT, valueTrue, t) +} diff --git a/runtime.go b/runtime.go index 9cccb4693..eadb8b302 100644 --- a/runtime.go +++ b/runtime.go @@ -201,6 +201,10 @@ type Runtime struct { promiseRejectionTracker PromiseRejectionTracker asyncContextTracker AsyncContextTracker + + // Stack for tracking objects currently being converted to string + // to detect and handle circular references + toStringStack []*Object } type StackFrame struct { From 06550edfc3e947b33e390c0e32371388c480b7f9 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Tue, 6 Jan 2026 15:14:35 +0800 Subject: [PATCH 2/2] rename popFromStringStack, fix GC leak, optimize circular reference check --- builtin_array.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/builtin_array.go b/builtin_array.go index ca6354baf..d8a9b05f2 100644 --- a/builtin_array.go +++ b/builtin_array.go @@ -182,7 +182,7 @@ func (r *Runtime) arrayproto_pop(call FunctionCall) Value { func (r *Runtime) pushToStringStack(o *Object) bool { // Check for circular reference in the toString stack for _, obj := range r.toStringStack { - if o.SameAs(obj) { + if o == obj { // Circular reference detected return true } @@ -193,8 +193,10 @@ func (r *Runtime) pushToStringStack(o *Object) bool { return false } -// popToStringStack removes an object from the toString stack. -func (r *Runtime) popToStringStack() { +// popFromStringStack removes an object from the toString stack. +func (r *Runtime) popFromStringStack() { + // Set the last element to nil to allow GC to collect it + r.toStringStack[len(r.toStringStack)-1] = nil r.toStringStack = r.toStringStack[:len(r.toStringStack)-1] } @@ -206,7 +208,7 @@ func (r *Runtime) arrayproto_join(call FunctionCall) Value { // This matches the behavior of mainstream JavaScript engines (V8, SpiderMonkey) return stringEmpty } - defer r.popToStringStack() + defer r.popFromStringStack() l := int(toLength(o.self.getStr("length", nil))) var sep String @@ -284,7 +286,7 @@ func (r *Runtime) arrayproto_toLocaleString(call FunctionCall) Value { // Circular reference detected, return empty string to avoid infinite recursion return stringEmpty } - defer r.popToStringStack() + defer r.popFromStringStack() var buf StringBuilder if a := r.checkStdArrayObj(array); a != nil {