Skip to content

ext/standard: Fix array_merge() with an empty array argument - #24404

Open
GromNaN wants to merge 1 commit into
php:masterfrom
GromNaN:fix/array-merge-empty-array-result
Open

GromNaN wants to merge 1 commit into
php:masterfrom
GromNaN:fix/array-merge-empty-array-result

Conversation

@GromNaN

@GromNaN GromNaN commented Oct 18, 2026 •

Copy link
Copy Markdown
Member

array_merge() returns the non-empty argument as is when the other argument is empty. The returned array is the argument itself, shared through copy-on-write, so it also kept the next free index of the argument, which array_merge() is expected to reset because integer keys are renumbered from 0. The general code path does exactly that, and the shortcut is an optimization, so it is not supposed to be observable.

array_values() had the same shortcut and got the same guard in cc96166 ("Don't optimize input arrays with suffix holes", 2017), for the same reason, in the same file, without an RFC. array_merge() just never got it.

Today the same call site returns a differently shaped array depending on whether a variable happens to be empty at runtime:

$a = [1, 2, 3];
unset($a[2]);

$b = [];
$r = array_merge($a, $b);
$r[] = 4;   // key 3

$b = ['k' => 'x'];
$r = array_merge($a, $b);
$r[] = 4;   // key 2

The shortcut now applies only when the argument already behaves like the result of the general path: a packed array without holes, or an array with string keys only, whose next free index is the one the general path would produce. Everything else goes through the general path, which builds a fresh array. Clean arrays are still returned as is, so the copy is still avoided. array_merge_recursive() shares the code path and gets the same fix.

A regression test is added. Output before and after the change:

 array(3) {
   [0]=>
   int(1)
   [1]=>
   int(2)
-  [3]=>
+  [2]=>
   int(4)
 }
 array(2) {
   ["a"]=>
   int(1)
-  [1]=>
+  [0]=>
   int(3)
 }

@GromNaN
GromNaN force-pushed the fix/array-merge-empty-array-result branch from 47ea5d5 to eb27320 Compare October 18, 2026 10:26
@GromNaN
GromNaN marked this pull request as ready for review October 18, 2026 11:58
@GromNaN
GromNaN requested a review from bukka as a code owner October 18, 2026 11:58
When one of the two arguments of array_merge() is empty, the other one is
returned as is instead of being copied. The returned array is the argument
itself, shared through copy-on-write, so it also kept the next free index of the
argument.

array_merge() renumbers integer keys from zero, so the next append to the result
must use the element count. An argument whose next free index was stale, for
example after unset() of its last element, made $result[] = $value use a wrong
key.

The shortcut now applies only when the argument already behaves like the result
of the general code path: a packed array without holes, or an array with string
keys only, whose next free index is the one the general path would produce.
Everything else goes through the general path, which builds a fresh array. Clean
arrays are still returned as is, so the copy is still avoided.

array_merge_recursive() shares the same code path and had the same bug.
@GromNaN
GromNaN force-pushed the fix/array-merge-empty-array-result branch from eb27320 to f7b26a5 Compare October 18, 2026 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant