Browse Source

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.
Linus Groh 4 năm trước cách đây
mục cha
commit
ae95ed5ddd

+ 8 - 3
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()

+ 12 - 0
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);
+});