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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

attiks’s picture

Status: Active » Needs review
FileSize
1.68 KB
nod_’s picture

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

tim.plunkett’s picture

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

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

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.