From f7e2994f35ec8214cffddec802c4cf9f4bd70177 Mon Sep 17 00:00:00 2001
From: davidot <david.tuin@gmail.com>
Date: Tue, 22 Jun 2021 20:13:42 +0200
Subject: [PATCH] LibJS: Make string to integer parsing for properties more
 strict

This does break two TypedArray test262 test but those really were not
  correct because hasProperty was not implemented
---
 .../Libraries/LibJS/Runtime/PropertyName.h    | 13 +++++
 .../builtins/Array/array-index-from-string.js | 49 +++++++++++++++++++
 .../Array/array-index-number-whitespace.js    |  7 ---
 3 files changed, 62 insertions(+), 7 deletions(-)
 create mode 100644 Userland/Libraries/LibJS/Tests/builtins/Array/array-index-from-string.js
 delete mode 100644 Userland/Libraries/LibJS/Tests/builtins/Array/array-index-number-whitespace.js

diff --git a/Userland/Libraries/LibJS/Runtime/PropertyName.h b/Userland/Libraries/LibJS/Runtime/PropertyName.h
index fd6f3696fd1..7e9e791020f 100644
--- a/Userland/Libraries/LibJS/Runtime/PropertyName.h
+++ b/Userland/Libraries/LibJS/Runtime/PropertyName.h
@@ -113,6 +113,19 @@ public:
     bool try_coerce_into_number()
     {
         VERIFY(m_string_may_be_number);
+        if (m_string.is_empty()) {
+            m_string_may_be_number = false;
+            return false;
+        }
+
+        if (char first = m_string.characters()[0]; first < '0' || first > '9') {
+            m_string_may_be_number = false;
+            return false;
+        } else if (m_string.length() > 1 && first == '0') {
+            m_string_may_be_number = false;
+            return false;
+        }
+
         i32 property_index = m_string.to_int(TrimWhitespace::No).value_or(-1);
         if (property_index < 0) {
             m_string_may_be_number = false;
diff --git a/Userland/Libraries/LibJS/Tests/builtins/Array/array-index-from-string.js b/Userland/Libraries/LibJS/Tests/builtins/Array/array-index-from-string.js
new file mode 100644
index 00000000000..5261d5a13c8
--- /dev/null
+++ b/Userland/Libraries/LibJS/Tests/builtins/Array/array-index-from-string.js
@@ -0,0 +1,49 @@
+describe("normal behavior", () => {
+    var arr = [3, 4, 5];
+
+    test("basic", () => {
+        expect(arr["0"]).toBe(3);
+        expect(arr["1"]).toBe(4);
+        expect(arr["2"]).toBe(5);
+    });
+
+    test("above length", () => {
+        expect(arr["3"]).toBeUndefined();
+    });
+});
+
+describe("strings with extra characters", () => {
+    var arr = [3, 4, 5];
+
+    test("whitespace", () => {
+        expect(arr[" 0"]).toBeUndefined();
+        expect(arr["0 "]).toBeUndefined();
+        expect(arr["  0"]).toBeUndefined();
+        expect(arr["  0  "]).toBeUndefined();
+        expect(arr["  3  "]).toBeUndefined();
+    });
+
+    test("leading 0", () => {
+        expect(arr["00"]).toBeUndefined();
+        expect(arr["01"]).toBeUndefined();
+        expect(arr["02"]).toBeUndefined();
+        expect(arr["03"]).toBeUndefined();
+    });
+
+    test("leading +/-", () => {
+        ["+", "-"].forEach(op => {
+            expect(arr[op + "0"]).toBeUndefined();
+            expect(arr[op + "1"]).toBeUndefined();
+
+            expect(arr[op + "-0"]).toBeUndefined();
+            expect(arr[op + "+0"]).toBeUndefined();
+        });
+    });
+
+    test("combined", () => {
+        expect(arr["+00"]).toBeUndefined();
+        expect(arr[" +0"]).toBeUndefined();
+        expect(arr["  +0 "]).toBeUndefined();
+        expect(arr["  00 "]).toBeUndefined();
+    });
+});
diff --git a/Userland/Libraries/LibJS/Tests/builtins/Array/array-index-number-whitespace.js b/Userland/Libraries/LibJS/Tests/builtins/Array/array-index-number-whitespace.js
deleted file mode 100644
index d45cf6dd58a..00000000000
--- a/Userland/Libraries/LibJS/Tests/builtins/Array/array-index-number-whitespace.js
+++ /dev/null
@@ -1,7 +0,0 @@
-test("indexing the array doesn't strip whitespace if it's a number", () => {
-    var a = [];
-    a[1] = 1;
-
-    expect(a["1"]).toBe(1);
-    expect(a[" 1 "]).toBeUndefined();
-});