Bug #99406
closedException in LocalImageProcessor::checkForExistingTargetFile because accessing array elements without null coalese and "array" is bool (false)
100%
Description
Exception when opening the file brower in v11.5.21:
#1476107295 TYPO3\CMS\Core\Error\Exception
PHP Warning: Trying to access array offset on value of type bool in /var/www/site-uol11/htdocs/typo3/sysext/core/Classes/Resource/Processing/LocalImageProcessor.php line 124
LocalImageProcessor:
Line 122: $imageDimensions = $this->getGraphicalFunctionsObject()->getImageDimensions($localProcessedFile); Line 123: $properties = [ Line 124 'width' => $imageDimensions[0],
LocalImageProcessor::checkForExistingTargetFile
calls $this->getGraphicalFunctionsObject()->getImageDimensions( and expects result to be array and accesses array elements without ??.
But getImageDimensions may not return an array with height and width.
Reproduce¶
1. Open the file browser, for example, for adding images to textmedia
2. Enable preview images
3. close browser again and prepare one of the preview thumbnail files. For example set to empty:
rm path/filename touch path/filename
4. Open the file browser again. This time, an exception will occur.
(occurred when file browser was opened to select a file. The file it tried to access was: /var/www/site-uol11/htdocs/fileadmin/_processed/7/3/preview_18db9f6295_aa23222219.gif. This file was empty (zero content). This should normally not happen but should probably not result in exception either if image dimensions cannot be detected.).
Problems¶
1. wrong return type declaration in getImageDimensions. Either getCachedImageDimensions should not return bool or the phpdoc for getImageDimensions should be changed.
2. no checking if getImageDimensions() returned array in checkForExistingTargetFile and not checking if array elements filled in checkForExistingTargetFile
3. Generally no strong type hinting so these problems can easily occur without being detected by phpstan (for example).
Source¶
in main branch:
protected function checkForExistingTargetFile(TaskInterface $task)
{
// the storage of the processed file, not of the original file!
$storage = $task->getTargetFile()->getStorage();
$processingFolder = $storage->getProcessingFolder($task->getSourceFile());
// explicitly check for the raw filename here, as we check for files that existed before we even started
// processing, i.e. that were processed earlier
if ($processingFolder->hasFile($task->getTargetFileName())) {
// When the processed file already exists set it as processed file
$task->getTargetFile()->setName($task->getTargetFileName());
// If the processed file is stored on a remote server, we must fetch a local copy of the file, as we
// have no API for fetching file metadata from a remote file.
$localProcessedFile = $storage->getFileForLocalProcessing($task->getTargetFile(), false);
$task->setExecuted(true);
$imageDimensions = $this->getGraphicalFunctionsObject()->getImageDimensions($localProcessedFile);
$properties = [
'width' => $imageDimensions[0],
'height' => $imageDimensions[1],
...
/**
* Gets the input image dimensions.
*
* @param string $imageFile The image filepath
* @return array|null Returns an array where [0]/[1] is w/h, [2] is extension and [3] is the filename.
* @see imageMagickConvert()
* @see \TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer::getImgResource()
*/
public function getImageDimensions($imageFile)
{
$returnArr = null;
preg_match('/([^\\.]*)$/', $imageFile, $reg);
if (file_exists($imageFile) && in_array(strtolower($reg[0]), $this->imageFileExt, true)) {
$returnArr = $this->getCachedImageDimensions($imageFile);
if (!$returnArr) {
$imageInfoObject = GeneralUtility::makeInstance(ImageInfo::class, $imageFile);
if ($imageInfoObject->getWidth()) {
$returnArr = [
$imageInfoObject->getWidth(),
$imageInfoObject->getHeight(),
strtolower($reg[0]),
$imageFile,
];
$this->cacheImageDimensions($returnArr);
}
}
}
return $returnArr;
}
/**
* Fetches the cached image dimensions from the cache. Does not check if the image file exists.
*
* @param string $filePath Image file path, relative to public web path
*
* @return array|bool an array where [0]/[1] is w/h, [2] is extension and [3] is the file name,
* or FALSE for a cache miss
*/
public function getCachedImageDimensions($filePath)
{
$statusHash = $this->generateStatusHashForImageFile($filePath);
$identifier = $this->generateCacheKeyForImageFile($filePath);
$cache = GeneralUtility::makeInstance(CacheManager::class)->getCache('imagesizes');
$cachedImageDimensions = $cache->get($identifier);
if (!isset($cachedImageDimensions['hash'])) {
return false;
}
...
Updated by Gerrit Code Review almost 2 years ago
- Status changed from New to Under Review
Patch set 1 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/77844
Updated by Gerrit Code Review almost 2 years ago
Patch set 1 for branch 11.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/+/77757
Updated by Sybille Peters almost 2 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 66d3625281f47ef788f8663c0dbd050081d3fe74.
Updated by Sybille Peters over 1 year ago
- Related to Bug #99998: Trying to access array offset on value of type bool in typo3/sysext/core/Classes/Imaging/GraphicalFunctions.php line 2180 added