From ae95ed5ddd5e7ab2b9cc32d78a0761747f229b5f Mon Sep 17 00:00:00 2001
From: Linus Groh <mail@linusgroh.de>
Date: Sat, 20 Mar 2021 13:43:42 +0100
Subject: [PATCH] LibJS: Append first sparse element to packed elements in
 take_first()

Otherwise we continuously lose the first sparse element (at index
SPARSE_ARRAY_THRESHOLD) without noticing, as we overwrite all indices
with the value at index+1.

Fixes #5884.
---
 .../Libraries/LibJS/Runtime/IndexedProperties.cpp    | 11 ++++++++---
 .../Tests/builtins/Array/Array.prototype.shift.js    | 12 ++++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/Userland/Libraries/LibJS/Runtime/IndexedProperties.cpp b/Userland/Libraries/LibJS/Runtime/IndexedProperties.cpp
index 6ed90ff166..3b1af328ad 100644
--- a/Userland/Libraries/LibJS/Runtime/IndexedProperties.cpp
+++ b/Userland/Libraries/LibJS/Runtime/IndexedProperties.cpp
@@ -180,14 +180,19 @@ ValueAndAttributes GenericIndexedPropertyStorage::take_first()
     VERIFY(m_array_size > 0);
     m_array_size--;
 
+    auto first_element = m_packed_elements.take_first();
+
     if (!m_sparse_elements.is_empty()) {
+        m_packed_elements.append(m_sparse_elements.get(SPARSE_ARRAY_THRESHOLD).value_or({}));
         HashMap<u32, ValueAndAttributes> new_sparse_elements;
-        for (auto& entry : m_sparse_elements)
-            new_sparse_elements.set(entry.key - 1, entry.value);
+        for (auto& entry : m_sparse_elements) {
+            if (entry.key - 1 >= SPARSE_ARRAY_THRESHOLD)
+                new_sparse_elements.set(entry.key - 1, entry.value);
+        }
         m_sparse_elements = move(new_sparse_elements);
     }
 
-    return m_packed_elements.take_first();
+    return first_element;
 }
 
 ValueAndAttributes GenericIndexedPropertyStorage::take_last()
diff --git a/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.shift.js b/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.shift.js
index 0e154eeeb6..45c7168e2e 100644
--- a/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.shift.js
+++ b/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.shift.js
@@ -21,3 +21,15 @@ describe("normal behavior", () => {
         expect(a).toEqual([]);
     });
 });
+
+test("Issue #5884, GenericIndexedPropertyStorage::take_first() loses elements", () => {
+    const a = [];
+    for (let i = 0; i < 300; i++) {
+        a.push(i);
+    }
+    expect(a.length).toBe(300);
+    for (let i = 0; i < 300; i++) {
+        a.shift();
+    }
+    expect(a.length).toBe(0);
+});