From 752da88c377a0d217ca109388a536163bd1b8794 Mon Sep 17 00:00:00 2001
From: Marko Lahma <marko.lahma@gmail.com>
Date: Sat, 25 May 2024 10:58:00 +0300
Subject: [PATCH] Fix string-indexing optimizations against custom strings
 (#1871)

---
 .../RavenApiUsageTests.cs                     | 29 +++++++++++++------
 Jint/Engine.cs                                |  9 ++----
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/Jint.Tests.PublicInterface/RavenApiUsageTests.cs b/Jint.Tests.PublicInterface/RavenApiUsageTests.cs
index 1356315319..e23781cbfd 100644
--- a/Jint.Tests.PublicInterface/RavenApiUsageTests.cs
+++ b/Jint.Tests.PublicInterface/RavenApiUsageTests.cs
@@ -68,12 +68,11 @@ public void CanInjectConstructedObjects()
         var obj = new JsObject(engine);
         obj.FastSetDataProperty("name", "test");
 
-        var array1 = new JsArray(engine, new JsValue[]
-        {
+        var array1 = new JsArray(engine, [
             JsNumber.Create(1),
             JsNumber.Create(2),
             JsNumber.Create(3)
-        });
+        ]);
         engine.SetValue("array1", array1);
 
         TestArrayAccess(engine, array1, "array1");
@@ -85,7 +84,7 @@ public void CanInjectConstructedObjects()
         Assert.Equal(0, engine.Evaluate("emptyArray.length"));
         Assert.Equal(1, engine.Evaluate("emptyArray.push(1); return emptyArray.length"));
 
-        engine.SetValue("emptyArray", new JsArray(engine, Array.Empty<JsValue>()));
+        engine.SetValue("emptyArray", new JsArray(engine, []));
         Assert.Equal(0, engine.Evaluate("emptyArray.length"));
         Assert.Equal(1, engine.Evaluate("emptyArray.push(1); return emptyArray.length"));
 
@@ -99,7 +98,7 @@ private static void TestArrayAccess(Engine engine, JsArray array, string name)
         Assert.Equal(2, array.GetOwnProperty("1").Value);
 
         array.Push(4);
-        array.Push(new JsValue[] { 5, 6 });
+        array.Push([5, 6]);
 
         var i = 0;
         foreach (var entry in array.GetEntries())
@@ -143,6 +142,8 @@ public void CanInheritCustomString()
         var empty = new CustomString("");
         engine.SetValue("empty", empty);
 
+        engine.SetValue("x", new CustomString("x", allowMaterialize: true));
+
         var obj = new JsObject(engine);
         obj.Set("name", new CustomString("the name"));
         engine.SetValue("obj", obj);
@@ -156,7 +157,7 @@ public void CanInheritCustomString()
         Assert.True(engine.Evaluate("array.includes('2')").AsBoolean());
         Assert.True(engine.Evaluate("array.filter(x => x === '2').length > 0").AsBoolean());
 
-        engine.SetValue("objArray", new JsArray(engine, new JsValue[] { obj, obj }));
+        engine.SetValue("objArray", new JsArray(engine, [obj, obj]));
         Assert.True(engine.Evaluate("objArray.filter(x => x.name === 'the name').length === 2").AsBoolean());
 
         Assert.Equal(9, engine.Evaluate("str.length"));
@@ -173,6 +174,9 @@ public void CanInheritCustomString()
         Assert.True(engine.Evaluate("empty.trim() === ''").AsBoolean());
         Assert.True(engine.Evaluate("empty.trimStart() === ''").AsBoolean());
         Assert.True(engine.Evaluate("empty.trimEnd() === ''").AsBoolean());
+
+        Assert.True(engine.Evaluate("str[1] === 'h'").AsBoolean());
+        Assert.True(engine.Evaluate("str[x] === undefined").AsBoolean());
     }
 
     [Fact]
@@ -202,16 +206,23 @@ public void CanResetCallStack()
 file sealed class CustomString : JsString
 {
     private readonly string _value;
+    private readonly bool _allowMaterialize;
 
-    public CustomString(string value) : base(null)
+    public CustomString(string value, bool allowMaterialize = false) : base(null)
     {
         _value = value;
+        _allowMaterialize = allowMaterialize;
     }
 
     public override string ToString()
     {
-        // when called we know that we couldn't use fast paths
-        throw new InvalidOperationException("I don't want to be materialized!");
+        if (!_allowMaterialize)
+        {
+            // when called we know that we couldn't use fast paths
+            throw new InvalidOperationException("I don't want to be materialized!");
+        }
+
+        return _value;
     }
 
     public override char this[int index] => _value[index];
diff --git a/Jint/Engine.cs b/Jint/Engine.cs
index 2d9a8fa554..da0f7ed2fd 100644
--- a/Jint/Engine.cs
+++ b/Jint/Engine.cs
@@ -656,20 +656,17 @@ private bool TryHandleStringValue(JsValue property, JsString s, ref ObjectInstan
             if (property is JsNumber number && number.IsInteger())
             {
                 var index = number.AsInteger();
-                var str = s._value;
-                if (index < 0 || index >= str.Length)
+                if (index < 0 || index >= s.Length)
                 {
                     jsValue = JsValue.Undefined;
                     return true;
                 }
 
-                jsValue = JsString.Create(str[index]);
+                jsValue = JsString.Create(s[index]);
                 return true;
             }
 
-            if (property is JsString propertyString
-                && propertyString._value.Length > 0
-                && char.IsLower(propertyString._value[0]))
+            if (property is JsString { Length: > 0 } propertyString && char.IsLower(propertyString[0]))
             {
                 // trying to find property that's always in prototype
                 o = Realm.Intrinsics.String.PrototypeObject;