Avoid Array.sort infinite loops on full framework (#1914)

sebastienros · Jul 13, 2024 · 9f55b0d · 9f55b0d
1 parent 0ce19d8
commit 9f55b0d
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 22 deletions.
diff --git a/Jint.Tests/Runtime/ArrayTests.cs b/Jint.Tests/Runtime/ArrayTests.cs
@@ -1,4 +1,5 @@
 using Jint.Native;
+using Jint.Runtime.Interop;
 
 namespace Jint.Tests.Runtime;
 
@@ -107,22 +108,6 @@ function compare(a, b) {
         _engine.Execute(code);
     }
 
-#if !NETCOREAPP
-        // this test case only triggers on older full framework where the is no checks for infinite comparisons
-        [Fact]
-        public void ArraySortShouldObeyExecutionConstraints()
-        {
-            const string script = @"
-                let cases = [5,5];
-                let latestCase = cases.sort((c1, c2) => c1 > c2 ? -1: 1);";
-
-            var engine = new Engine(options => options
-                .TimeoutInterval(TimeSpan.FromSeconds(1))
-            );
-            Assert.Throws<TimeoutException>(() => engine.Evaluate(script));
-        }
-#endif
-
     [Fact]
     public void ExtendingArrayAndInstanceOf()
     {
@@ -352,4 +337,24 @@ public void ShouldBeAbleToInitFromArray()
         Assert.Equal("length", propertyDescriptors[1].Key);
         Assert.Equal(1, propertyDescriptors[1].Value.Value);
     }
+
+    [Fact]
+    public void ArrayFromSortTest()
+    {
+        var item1 = new KeyValuePair<string, string>("Id1", "0020");
+        var item2 = new KeyValuePair<string, string>("Id2", "0001");
+
+        var engine = new Engine();
+        engine.SetValue("Root", new { Inner = new { Items = new[] { item1, item2 } } });
+
+        var result = engine.Evaluate("Array.from(Root.Inner.Items).sort((a, b) => a.Value === '0001' ? -1 : 1)").AsArray();
+
+        var enumerableResult = result
+            .Select(x => (KeyValuePair<string, string>) ((IObjectWrapper) x).Target)
+            .ToList();
+
+        enumerableResult.Should().HaveCount(2);
+        enumerableResult[0].Key.Should().Be(item2.Key);
+        enumerableResult[1].Key.Should().Be(item1.Key);
+    }
 }
diff --git a/Jint/Native/Array/ArrayPrototype.cs b/Jint/Native/Array/ArrayPrototype.cs
@@ -1077,14 +1077,33 @@ private JsValue Sort(JsValue thisObject, JsValue[] arguments)
             // don't eat inner exceptions
             try
             {
-                var array = items.OrderBy(x => x, ArrayComparer.WithFunction(_engine, compareFn!)).ToArray();
-
-                uint j;
-                for (j = 0; j < itemCount; ++j)
+                var comparer = ArrayComparer.WithFunction(_engine, compareFn);
+                IEnumerable<JsValue> ordered;
+#if !NETCOREAPP
+                if (comparer is not null)
+                {
+                    // sort won't be stable on .NET Framework, but at least it cant go into infinite loop when comparer is badly implemented
+                    items.Sort(comparer);
+                    ordered = items;
+                }
+                else
                 {
-                    var updateLength = j == itemCount - 1;
-                    obj.Set(j, array[j], updateLength: updateLength, throwOnError: true);
+                    ordered = items.OrderBy(x => x, comparer);
                 }
+#else
+    #if NET8_0_OR_GREATER
+                ordered = items.Order(comparer);
+    #else
+                ordered = items.OrderBy(x => x, comparer);
+    #endif
+#endif
+                uint j = 0;
+                foreach (var item in ordered)
+                {
+                    obj.Set(j, item, updateLength: false, throwOnError: true);
+                    j++;
+                }
+
                 for (; j < len; ++j)
                 {
                     obj.DeletePropertyOrThrow(j);