Bug #89148
closedMedia viewhelper empty additionalConfig causes php warning
100%
Description
Typo3 version 9.5.9
Error message:¶
PHP Warning: array_merge_recursive(): Argument #2 is not an array in /typo3/sysext/fluid/Classes/ViewHelpers/MediaViewHelper.php line 119
How to reproduce.¶
Remove typoscript setting:
lib.contentElement.settings.media.additionalConfig >
or remove value from additionalConfig argument in fluid partial FluidStyledContent/Partials/Media/Rendering/Video.html
<f:media file="{file}" class="{class}" additionalAttributes="" alt="{file.alternative}" title="{file.title}" additionalConfig="" />
Cause:¶
In TYPO3\CMS\Fluid\ViewHelpers\MediaViewHelper
the following takes place without checking if $additionalConfig
is an array
$additionalConfig = array_merge_recursive($this->arguments, $additionalConfig);
Possible solution:¶
Add is_array
check:
if (!is_array($additionalConfig)) { $additionalConfig = []; } $additionalConfig = array_merge_recursive($this->arguments, $additionalConfig);
Updated by Oliver Bartsch about 5 years ago
Hi,
I tried to reproduce this. It seems your examples also lead to another error message. So I got:
Argument 4 passed to TYPO3\CMS\Core\Resource\Rendering\VideoTagRenderer::render() must be of the type array, null given, called in /var/www/html/typo3/sysext/fluid/Classes/ViewHelpers/MediaViewHelper.php on line 120
As the VideoTagRenderer has following signature:
FileInterface $file, $width, $height, array $options = [], $usedPathsRelativeToCurrentScript = false
I get this because the Renderer gets instantiated before the `array_merge_recursive` methode is called.
However, `$additionalConfig` should actually be always an array from looking at the `registerArgument` call.
But what I can see is, that an empty string is treated as valid for type array by TYPO3Fluid\Fluid\Core\ViewHelper\AbstractViewHelper->isValidType().
Thats why the defaultValue - an empty array - doesn't get assigned to the argument.
Your proposed is_array check will fix this Error message, though we should takle the fact that an empty string is treated as valid while the type is actually defined as an array.
What do you think?
Updated by Oliver Bartsch about 5 years ago
To make it clear:
The declaration of
<f:media file="{file}" class="{class}" additionalAttributes="" alt="{file.alternative}" title="{file.title}" additionalConfig="" />
is however wrong. It should be:
<f:media file="{file}" class="{class}" additionalAttributes="" alt="{file.alternative}" title="{file.title}" additionalConfig="{}" />
So the bug here is the wrong type check. Your HTML should trigger a InvalidTypeException.
Something like: 'additionalConfig' was registered as type array but is of type string.
Updated by Jarvis H about 5 years ago
Thanks for the feedback. A couple of things to note:
- This issue only occurs on the first page load after changing the additionalConfig
attribute in the video partial to additionalConfig="{}"
- The issue occurs every time if additionalConfig=""
I suspect your error for argument 4 in VideoTagRenderer
is caused by the same underlying issue which is causing the php warning in the array_merge_recursive
call.
As far as I can tell the issue arises because the value of the argument additionalConfig
is set to null on the first page load and I traced it back to the ViewHelperInvoker class (vendor/typo3fluid/fluid/src/Core/ViewHelper/ViewHelperInvoker.php).
In the main invoke
method, the following happens when additionalConfig="{}"
is used (see my inline comments for clarification):
try {
foreach ($expectedViewHelperArguments as $argumentName => $argumentDefinition) {
if (isset($arguments[$argumentName])) {
/** @var NodeInterface|mixed $argumentValue */
$argumentValue = $arguments[$argumentName];
$evaluatedArguments[$argumentName] = $argumentValue instanceof NodeInterface ? $argumentValue->evaluate($renderingContext) : $argumentValue;
// --------------------------------------------------
// After deleting system caches and loading page '$evaluatedArguments[$argumentName]' equals null
// as $argumentValue is an instance of \TYPO3Fluid\Fluid\Core\Parser\SyntaxTree\RootNode
// where the property 'childNodes' is empty causing '$argumentValue->evaluate($renderingContext)' to return null
if ($argumentName == 'additionalConfig') {
\TYPO3\CMS\Core\Utility\DebugUtility::debug($evaluatedArguments[$argumentName]);
}
// --------------------------------------------------
} else {
// --------------------------------------------------
// After loading page again '$evaluatedArguments[$argumentName]'
// returns an empty array as it should
$evaluatedArguments[$argumentName] = $argumentDefinition->getDefaultValue();
if ($argumentName == 'additionalConfig') {
\TYPO3\CMS\Core\Utility\DebugUtility::debug($evaluatedArguments[$argumentName]);
}
// --------------------------------------------------
}
}
foreach ($arguments as $argumentName => $argumentValue) {
if (!array_key_exists($argumentName, $evaluatedArguments)) {
$undeclaredArguments[$argumentName] = $argumentValue instanceof NodeInterface ? $argumentValue->evaluate($renderingContext) : $argumentValue;
}
}
if ($renderChildrenClosure) {
$viewHelper->setRenderChildrenClosure($renderChildrenClosure);
}
$viewHelper->setRenderingContext($renderingContext);
// --------------------------------------------------
// Here the arguments property is set for the MediaViewHelper
$viewHelper->setArguments($evaluatedArguments);
// --------------------------------------------------
$viewHelper->handleAdditionalArguments($undeclaredArguments);
return $viewHelper->initializeArgumentsAndRender();
} catch (Exception $error) {
return $renderingContext->getErrorHandler()->handleViewHelperError($error);
}
However this only adds to my confusion, as I am guessing it has something to do with the fluid caching mechanism.
Edit: The question is, what would be the expected behaviour when passing either an empty string or empty array to an argument which defaults to an array? InvalidTypeException
for an empty string and no warning at all for an empty array?
Updated by Gerrit Code Review over 4 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/c/Packages/TYPO3.CMS/+/63593
Updated by Georg Ringer over 4 years ago
- Category deleted (
Fluid) - PHP Version deleted (
7.2)
also opened an issue at https://github.com/TYPO3/Fluid/issues/515
Updated by Gerrit Code Review over 4 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/c/Packages/TYPO3.CMS/+/63593
Updated by Gerrit Code Review over 4 years ago
Patch set 1 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63733
Updated by Georg Ringer over 4 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 61eb4b919bd058be521085196038e43c9ea6924f.