Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid Array.sort infinite loops on full framework #1914

Merged
merged 2 commits into from
Jul 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
37 changes: 21 additions & 16 deletions Jint.Tests/Runtime/ArrayTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Jint.Native;
using Jint.Runtime.Interop;

namespace Jint.Tests.Runtime;

Expand Down Expand Up @@ -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()
{
Expand Down Expand Up @@ -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);
}
}
31 changes: 25 additions & 6 deletions Jint/Native/Array/ArrayPrototype.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down