Project

General

Profile

Actions
Copy link

Bug #65935

closed

Bug in ArrayUtility::mergeRecursiveWithOverrule

Added by Markus Hölzle over 9 years ago. Updated about 7 years ago.

Status:
Rejected
Priority:
Should have
Assignee:
-
Category:
-
Target version:
-
Start date:
2015-03-23
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
7
PHP Version:
Tags:
Complexity:
Is Regression:
No
Sprint Focus:
Remote Sprint

Description

Hello together,

I found a bug in ArrayUtility::mergeRecursiveWithOverrule while merging two arrays. If you merge this:

array(
  'key1' => array(
    'key2' => 'x'
  )
)

with this:
array(
  'key1' => 'y'
)

I expect:
array(
  'key1' => 'y'
)

But I got:
array(
  'key1' => array(
    'key2' => 'x'
  )
)

This is because there is a nested "if" without an "else":

if (isset($original[$key]) && is_array($original[$key])) {
  if (is_array($overrule[$key])) {
    self::mergeRecursiveWithOverrule($original[$key], $overrule[$key], $addKeys, $includeEmptyValues, $enableUnsetFeature);
  }
} elseif ...

this should be:
if (isset($original[$key]) && is_array($original[$key]) && is_array($overrule[$key])) {
  self::mergeRecursiveWithOverrule($original[$key], $overrule[$key], $addKeys, $includeEmptyValues, $enableUnsetFeature);
} elseif ...

What do you think?

Actions
Copy link
 #1

Updated by Markus Klein over 9 years ago

Well this is a very picky method. The current behaviour is covered well with unittests. May I ask for which purpose you need that method?
In 98% of all cases the native PHP methods work out and are much faster.

Actions
Copy link
 #2

Updated by Markus Hölzle over 9 years ago

I use this function because of the "unset" feature. This makes it very simple to unset specific keys in an nested array. There I found this probably buggy code lines...

Actions
Copy link
 #3

Updated by Markus Klein over 9 years ago

Oh dear, so you're one of the remaining 2% ;-)

I found a testcase for this:
typo3/sysext/core/Tests/Unit/Utility/ArrayUtilityTest.php:1530

'Override array does not reset array to string (weird!)' => array(
    array(
        'first' => array(),
    ),
    array(
        'first' => 'foo',
    ),
    TRUE,
    TRUE,
    TRUE,
    array(
        'first' => array(), // This is rather unexpected, naive expectation: first => 'foo'
    ),
),

So this is indeed intended! (don't ask me why)

Actions
Copy link
 #4

Updated by Mathias Schreiber over 9 years ago

  • Sprint Focus set to On Location Sprint
Actions
Copy link
 #5

Updated by Anja Leichsenring over 9 years ago

  • Sprint Focus changed from On Location Sprint to Remote Sprint
Actions
Copy link
 #6

Updated by Benni Mack about 7 years ago

  • Status changed from New to Rejected

Sorry, Markus. We won't change this (instead, I've been trying to get rid of this ugly UNSET for a long time, as this is just too much PHP userland magic). As Markus K. mentioned, this is expected behaviour ;)

Actions
Copy link

Also available in: Atom PDF