Split off from #1279226: jQuery and Drupal JavaScript libraries and settings are output even when no JS is added to the page

drupal_array_merge_deep_array doesn't check if $array is an array before using it inside a foreach

CommentFileSizeAuthor
#1 i1554142.patch1.68 KBattiks

Comments

attiks’s picture

attiks commented
Status: Active » Needs review
StatusFileSize
new i1554142.patch1.68 KB
nod_’s picture

nod_
Primary language French
Location Lille
commented

Looks rtbc to me. Since it's the base system I'm not rtbc it without another review :)

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

When and why would this be called on something that isn't an array?
I would think this is won't fix, or even add type hinting to the function signature to force it to blow up.

attiks’s picture

attiks commented

I ran into this while making a patch for #1279226: jQuery and Drupal JavaScript libraries and settings are output even when no JS is added to the page and it was blowing up on some tests, that's why I added it. The calling function passes an array, but one of the elements of the array wasn't an array itself, so it failed on the second foreach (foreach ($array as $key => $value) {).

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Closed (won't fix)

re #4: You likely wanted to use drupal_array_merge_deep() (without last _array suffix).

Type-hinting won't help here.

This function is called relatively often on big arrays and is slow enough already (is_array() is slow) and only exists because PHP's native array_merge_recursive() doesn't allow for a flag that would change its merge behavior for nested string keys.