Project

General

Profile

Actions

Bug #84485

closed

misuse as closure of callable-typed $callback in ArrayUtility::filterRecursive

Added by Stephan Jorek about 6 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Must have
Assignee:
-
Category:
-
Target version:
-
Start date:
2018-03-19
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
9
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

The method \TYPO3\CMS\Core\Utility\ArrayUtility::filterRecursive declares its $callback argument as of type callable, but uses it as a \Closure via $callback(...).

Current State

public static function filterRecursive(array $array, callable $callback = null): array
// ... 
    foreach ($array as $key => $value) {
// ...
        if (!$callback($value)) { // <--- used as a \Closure, but declared as a callable
            unset($array[$key]);
        }
    }
// ...
}

Possible Strategies

We could ...
  1. either change the filterRecursive-methods signature to ..., \Closure $callback..., or ...
  2. we fix the use of the $callback argument in the method-body.

I decided to go for the latter (2.) and fix the use of the $callback argument in the method-body, as it seems to be quite common in the core to use callable type, instead of \Closure. The fixed line will then contain a call_user_func($callback, $value)-function call.

Proposed Solution

public static function filterRecursive(array $array, callable $callback = null): array
// ... 
    foreach ($array as $key => $value) {
// ...
        if (!call_user_func($callback, $value)) { // <--- now used and declared as a callable
            unset($array[$key]);
        }
    }
// ...
}

Conclusion

Fixing the use of $callback in the static \TYPO3\CMS\Core\Utility\ArrayUtility::filterRecursive method, won't harm existing installations. Adding an appropriate unit-test ensures the correct behaviour.

Therefore I'll create a PR to fix this issue with the proposed solution and I'll add an appropriate test to \TYPO3\CMS\Core\Tests\Unit\Utility\ArrayUtilityTest too.

A quick search over the core-sources did not reveal any other misuses of callable-types as closure. But do not bet on me, as I just took 5 minutes to identify potential candidates.


Related issues 1 (0 open1 closed)

Related to TYPO3 Core - Task #83350: Provide array_filter_recursive in ArrayUtilityClosedStefan Neufeind2017-12-16

Actions
Actions #1

Updated by Stephan Jorek about 6 years ago

  • Private changed from Yes to No
Actions #2

Updated by Gerrit Code Review about 6 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56368

Actions #3

Updated by Gerrit Code Review about 6 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56368

Actions #4

Updated by Gerrit Code Review about 6 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56368

Actions #5

Updated by Gerrit Code Review about 6 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56368

Actions #6

Updated by Gerrit Code Review about 6 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56368

Actions #7

Updated by Gerrit Code Review about 6 years ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/56368

Actions #8

Updated by Christian Kuhn about 6 years ago

  • Related to Task #83350: Provide array_filter_recursive in ArrayUtility added
Actions #9

Updated by Stephan Jorek about 6 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #10

Updated by Benni Mack over 5 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF