From 63ef394a57a734195d43ee26c7192bcfb4b524e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Bl=C3=A1zquez?= Date: Fri, 4 Oct 2024 14:20:51 +0200 Subject: [PATCH 1/5] Fix stackoverflow --- Jint.Tests/Runtime/ArrayTests.cs | 8 ++++++++ Jint/Native/Array/ArrayPrototype.cs | 8 ++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/Jint.Tests/Runtime/ArrayTests.cs b/Jint.Tests/Runtime/ArrayTests.cs index 549de4c254..54c5f5c9a7 100644 --- a/Jint.Tests/Runtime/ArrayTests.cs +++ b/Jint.Tests/Runtime/ArrayTests.cs @@ -39,6 +39,14 @@ public void ArrayPrototypeToStringWithObject() Assert.Equal("[object Object]", result); } + [Fact] + public void ArrayPrototypeToStringSelfReference() + { + var result = _engine.Evaluate("Array.prototype.toString.call((c = [1, 2, 3, 4], c[1] = c, c))").AsString(); + + Assert.Equal("1,,3,4", result); + } + [Fact] public void EmptyStringKey() { diff --git a/Jint/Native/Array/ArrayPrototype.cs b/Jint/Native/Array/ArrayPrototype.cs index 0765b9ac90..f693101bc3 100644 --- a/Jint/Native/Array/ArrayPrototype.cs +++ b/Jint/Native/Array/ArrayPrototype.cs @@ -1273,14 +1273,14 @@ private JsValue Join(JsValue thisObject, JsValue[] arguments) return JsString.Empty; } - static string StringFromJsValue(JsValue value) + static string StringFromJsValue(JsValue value, JsValue thisObject) { - return value.IsNullOrUndefined() + return value.IsNullOrUndefined() || thisObject == value ? "" : TypeConverter.ToString(value); } - var s = StringFromJsValue(o.Get(0)); + var s = StringFromJsValue(o.Get(0), thisObject); if (len == 1) { return s; @@ -1294,7 +1294,7 @@ static string StringFromJsValue(JsValue value) { sb.Append(sep); } - sb.Append(StringFromJsValue(o.Get(k))); + sb.Append(StringFromJsValue(o.Get(k), thisObject)); } return sb.ToString(); From 4a2ccebc5b93709f37b96a2b06d0e47143153a83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Bl=C3=A1zquez?= Date: Fri, 4 Oct 2024 17:22:35 +0200 Subject: [PATCH 2/5] Fix typo --- Jint.Tests/Runtime/ArrayTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Jint.Tests/Runtime/ArrayTests.cs b/Jint.Tests/Runtime/ArrayTests.cs index 54c5f5c9a7..af3cf09cab 100644 --- a/Jint.Tests/Runtime/ArrayTests.cs +++ b/Jint.Tests/Runtime/ArrayTests.cs @@ -40,7 +40,7 @@ public void ArrayPrototypeToStringWithObject() } [Fact] - public void ArrayPrototypeToStringSelfReference() + public void ArrayPrototypeToStringWithCircularReference() { var result = _engine.Evaluate("Array.prototype.toString.call((c = [1, 2, 3, 4], c[1] = c, c))").AsString(); From e58edccd08ba590dedc870372ab9fbfadbbe7db3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Bl=C3=A1zquez?= Date: Sat, 5 Oct 2024 14:08:47 +0200 Subject: [PATCH 3/5] Fix deeper circular reference --- Jint.Tests/Runtime/ArrayTests.cs | 4 +-- Jint/Native/Array/ArrayPrototype.cs | 39 ++++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/Jint.Tests/Runtime/ArrayTests.cs b/Jint.Tests/Runtime/ArrayTests.cs index 54c5f5c9a7..e3eff4d155 100644 --- a/Jint.Tests/Runtime/ArrayTests.cs +++ b/Jint.Tests/Runtime/ArrayTests.cs @@ -42,9 +42,9 @@ public void ArrayPrototypeToStringWithObject() [Fact] public void ArrayPrototypeToStringSelfReference() { - var result = _engine.Evaluate("Array.prototype.toString.call((c = [1, 2, 3, 4], c[1] = c, c))").AsString(); + var result = _engine.Evaluate("Array.prototype.toString.call((c = [1, 2, 3, 4], b = [1, 2, 3, 4], b[1] = c, c[1] = b, c))").AsString(); - Assert.Equal("1,,3,4", result); + Assert.Equal("1,1,,3,4,3,4", result); } [Fact] diff --git a/Jint/Native/Array/ArrayPrototype.cs b/Jint/Native/Array/ArrayPrototype.cs index f693101bc3..cef83990a1 100644 --- a/Jint/Native/Array/ArrayPrototype.cs +++ b/Jint/Native/Array/ArrayPrototype.cs @@ -1261,6 +1261,37 @@ private JsValue Reverse(JsValue thisObject, JsValue[] arguments) /// private JsValue Join(JsValue thisObject, JsValue[] arguments) { + static JsValue RemoveCircularReferences(JsValue thisObject, Engine engine, HashSet? visited = null) + { + visited ??= []; + + if (thisObject is JsArray array) + { + if (visited.Contains(array)) + { + return JsUndefined.Undefined; + } + + visited.Add(array); + var filteredArray = new JsValue[array.Length]; + + for (var i = 0; i < array.Length; i++) + { + var item = array[i]; + filteredArray[i] = item is JsArray nestedArray ? RemoveCircularReferences(nestedArray, engine, visited) : item; + } + + return new JsArray(engine, filteredArray); + } + + return thisObject; + } + + if (thisObject is JsArray) + { + thisObject = RemoveCircularReferences(thisObject, Engine); + } + var separator = arguments.At(0); var o = ArrayOperations.For(_realm, thisObject, forWrite: false); var len = o.GetLength(); @@ -1273,14 +1304,14 @@ private JsValue Join(JsValue thisObject, JsValue[] arguments) return JsString.Empty; } - static string StringFromJsValue(JsValue value, JsValue thisObject) + static string StringFromJsValue(JsValue value) { - return value.IsNullOrUndefined() || thisObject == value + return value.IsNullOrUndefined() ? "" : TypeConverter.ToString(value); } - var s = StringFromJsValue(o.Get(0), thisObject); + var s = StringFromJsValue(o.Get(0)); if (len == 1) { return s; @@ -1294,7 +1325,7 @@ static string StringFromJsValue(JsValue value, JsValue thisObject) { sb.Append(sep); } - sb.Append(StringFromJsValue(o.Get(k), thisObject)); + sb.Append(StringFromJsValue(o.Get(k))); } return sb.ToString(); From 5ae492c62e49652fbfaf4f74c614549d45354648 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Bl=C3=A1zquez?= Date: Sat, 5 Oct 2024 15:15:12 +0200 Subject: [PATCH 4/5] Cleanup --- Jint/Native/Array/ArrayPrototype.cs | 31 ++++++++++++----------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/Jint/Native/Array/ArrayPrototype.cs b/Jint/Native/Array/ArrayPrototype.cs index cef83990a1..a90b299248 100644 --- a/Jint/Native/Array/ArrayPrototype.cs +++ b/Jint/Native/Array/ArrayPrototype.cs @@ -1261,35 +1261,30 @@ private JsValue Reverse(JsValue thisObject, JsValue[] arguments) /// private JsValue Join(JsValue thisObject, JsValue[] arguments) { - static JsValue RemoveCircularReferences(JsValue thisObject, Engine engine, HashSet? visited = null) + static JsValue RemoveCircularReferences(JsArray array, Engine engine, HashSet? visited = null) { visited ??= []; - if (thisObject is JsArray array) + if (visited.Contains(array)) { - if (visited.Contains(array)) - { - return JsUndefined.Undefined; - } + return JsUndefined.Undefined; + } - visited.Add(array); - var filteredArray = new JsValue[array.Length]; + visited.Add(array); + var filteredArray = new JsValue[array.Length]; - for (var i = 0; i < array.Length; i++) - { - var item = array[i]; - filteredArray[i] = item is JsArray nestedArray ? RemoveCircularReferences(nestedArray, engine, visited) : item; - } - - return new JsArray(engine, filteredArray); + for (var i = 0; i < array.Length; i++) + { + var item = array[i]; + filteredArray[i] = item is JsArray nestedArray ? RemoveCircularReferences(nestedArray, engine, visited) : item; } - return thisObject; + return new JsArray(engine, filteredArray); } - if (thisObject is JsArray) + if (thisObject is JsArray thisArrayObject) { - thisObject = RemoveCircularReferences(thisObject, Engine); + thisObject = RemoveCircularReferences(thisArrayObject, Engine); } var separator = arguments.At(0); From 42a737c978d892833062220218de5b3f6b9ada28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Bl=C3=A1zquez?= Date: Sun, 6 Oct 2024 20:02:58 +0200 Subject: [PATCH 5/5] Use `ObjectTraverseStack` --- Jint.Tests/Runtime/ArrayTests.cs | 12 ++++++-- Jint/Collections/ObjectTraverseStack.cs | 14 +++++++-- Jint/Native/Array/ArrayPrototype.cs | 41 +++++++++---------------- 3 files changed, 37 insertions(+), 30 deletions(-) diff --git a/Jint.Tests/Runtime/ArrayTests.cs b/Jint.Tests/Runtime/ArrayTests.cs index c75b83ae3c..62e06c8599 100644 --- a/Jint.Tests/Runtime/ArrayTests.cs +++ b/Jint.Tests/Runtime/ArrayTests.cs @@ -40,9 +40,17 @@ public void ArrayPrototypeToStringWithObject() } [Fact] - public void ArrayPrototypeToStringWithCircularReference() + public void ArrayPrototypeJoinWithCircularReference() { - var result = _engine.Evaluate("Array.prototype.toString.call((c = [1, 2, 3, 4], b = [1, 2, 3, 4], b[1] = c, c[1] = b, c))").AsString(); + var result = _engine.Evaluate("Array.prototype.join.call((c = [1, 2, 3, 4], b = [1, 2, 3, 4], b[1] = c, c[1] = b, c))").AsString(); + + Assert.Equal("1,1,,3,4,3,4", result); + } + + [Fact] + public void ArrayPrototypeToLocaleStringWithCircularReference() + { + var result = _engine.Evaluate("Array.prototype.toLocaleString.call((c = [1, 2, 3, 4], b = [1, 2, 3, 4], b[1] = c, c[1] = b, c))").AsString(); Assert.Equal("1,1,,3,4,3,4", result); } diff --git a/Jint/Collections/ObjectTraverseStack.cs b/Jint/Collections/ObjectTraverseStack.cs index 41cedbdd0c..c41d3bbcec 100644 --- a/Jint/Collections/ObjectTraverseStack.cs +++ b/Jint/Collections/ObjectTraverseStack.cs @@ -16,7 +16,7 @@ public ObjectTraverseStack(Engine engine) _engine = engine; } - public void Enter(JsValue value) + public bool TryEnter(JsValue value) { if (value is null) { @@ -25,10 +25,20 @@ public void Enter(JsValue value) if (_stack.Contains(value)) { - ExceptionHelper.ThrowTypeError(_engine.Realm, "Cyclic reference detected."); + return false; } _stack.Push(value); + + return true; + } + + public void Enter(JsValue value) + { + if (!TryEnter(value)) + { + ExceptionHelper.ThrowTypeError(_engine.Realm, "Cyclic reference detected."); + } } public void Exit() diff --git a/Jint/Native/Array/ArrayPrototype.cs b/Jint/Native/Array/ArrayPrototype.cs index a90b299248..b33bd282f4 100644 --- a/Jint/Native/Array/ArrayPrototype.cs +++ b/Jint/Native/Array/ArrayPrototype.cs @@ -21,6 +21,7 @@ public sealed class ArrayPrototype : ArrayInstance { private readonly Realm _realm; private readonly ArrayConstructor _constructor; + private readonly ObjectTraverseStack _joinStack; internal ClrFunction? _originalIteratorFunction; internal ArrayPrototype( @@ -33,6 +34,7 @@ internal ArrayPrototype( _length = new PropertyDescriptor(JsNumber.PositiveZero, PropertyFlag.Writable); _realm = realm; _constructor = arrayConstructor; + _joinStack = new(engine); } protected override void Initialize() @@ -1261,32 +1263,6 @@ private JsValue Reverse(JsValue thisObject, JsValue[] arguments) /// private JsValue Join(JsValue thisObject, JsValue[] arguments) { - static JsValue RemoveCircularReferences(JsArray array, Engine engine, HashSet? visited = null) - { - visited ??= []; - - if (visited.Contains(array)) - { - return JsUndefined.Undefined; - } - - visited.Add(array); - var filteredArray = new JsValue[array.Length]; - - for (var i = 0; i < array.Length; i++) - { - var item = array[i]; - filteredArray[i] = item is JsArray nestedArray ? RemoveCircularReferences(nestedArray, engine, visited) : item; - } - - return new JsArray(engine, filteredArray); - } - - if (thisObject is JsArray thisArrayObject) - { - thisObject = RemoveCircularReferences(thisArrayObject, Engine); - } - var separator = arguments.At(0); var o = ArrayOperations.For(_realm, thisObject, forWrite: false); var len = o.GetLength(); @@ -1299,6 +1275,11 @@ static JsValue RemoveCircularReferences(JsArray array, Engine engine, HashSet