From 2a95db88bbe9d7a4ec388fa5d6371fde9123facb Mon Sep 17 00:00:00 2001
From: Christian Kuhn <lolli@schwarzbu.ch>
Date: Thu, 22 Feb 2024 12:25:50 +0100
Subject: [PATCH] [TASK] Harden ResourceFactory->getFileReferenceObject()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The method throws an exception on !is_numeric() already.
This includes float, which is bogus. We can simplify
with an argument type hint.
Resolves: #103175
Releases: main
Change-Id: I990f273f16581c553e17cb610524e2cbc3659baa
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83076
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Oliver Klee <typo3-coding@oliverklee.de>
Tested-by: Oliver Klee <typo3-coding@oliverklee.de>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
---
Build/phpstan/phpstan-baseline.neon | 25 -------------------
.../Resource/Filter/FileExtensionFilter.php | 2 +-
.../core/Classes/Resource/ResourceFactory.php | 19 +++-----------
.../extbase/Classes/Service/ImageService.php | 2 +-
.../UploadedFileReferenceConverter.php | 2 +-
.../ContentObject/ContentObjectRenderer.php | 10 ++++----
6 files changed, 12 insertions(+), 48 deletions(-)
diff --git a/Build/phpstan/phpstan-baseline.neon b/Build/phpstan/phpstan-baseline.neon
index af4bc0bde780..1290d950fed7 100644
--- a/Build/phpstan/phpstan-baseline.neon
+++ b/Build/phpstan/phpstan-baseline.neon
@@ -550,16 +550,6 @@ parameters:
count: 1
path: ../../typo3/sysext/core/Classes/Resource/FileReference.php
- -
- message: "#^Parameter \\#1 \\$uid of method TYPO3\\\\CMS\\\\Core\\\\Resource\\\\ResourceFactory\\:\\:getFileReferenceObject\\(\\) expects int, string given\\.$#"
- count: 1
- path: ../../typo3/sysext/core/Classes/Resource/Filter/FileExtensionFilter.php
-
- -
- message: "#^Parameter \\#2 \\$id of method TYPO3\\\\CMS\\\\Core\\\\DataHandling\\\\DataHandler\\:\\:deleteAction\\(\\) expects int, string given\\.$#"
- count: 1
- path: ../../typo3/sysext/core/Classes/Resource/Filter/FileExtensionFilter.php
-
-
message: "#^Call to an undefined method TYPO3\\\\CMS\\\\Core\\\\Resource\\\\FolderInterface\\:\\:getReadablePath\\(\\)\\.$#"
count: 1
@@ -1145,11 +1135,6 @@ parameters:
count: 1
path: ../../typo3/sysext/extbase/Classes/Service/ImageService.php
- -
- message: "#^Parameter \\#1 \\$uid of method TYPO3\\\\CMS\\\\Core\\\\Resource\\\\ResourceFactory\\:\\:getFileReferenceObject\\(\\) expects int, string given\\.$#"
- count: 1
- path: ../../typo3/sysext/extbase/Classes/Service/ImageService.php
-
-
message: "#^Result of && is always false\\.$#"
count: 1
@@ -1630,11 +1615,6 @@ parameters:
count: 1
path: ../../typo3/sysext/form/Classes/Mvc/Property/PropertyMappingConfiguration.php
- -
- message: "#^Parameter \\#1 \\$uid of method TYPO3\\\\CMS\\\\Core\\\\Resource\\\\ResourceFactory\\:\\:getFileReferenceObject\\(\\) expects int, string given\\.$#"
- count: 1
- path: ../../typo3/sysext/form/Classes/Mvc/Property/TypeConverter/UploadedFileReferenceConverter.php
-
-
message: "#^Parameter \\#1 \\$uid of method TYPO3\\\\CMS\\\\Core\\\\Resource\\\\ResourceFactory\\:\\:getStorageObject\\(\\) expects int\\|null, string given\\.$#"
count: 1
@@ -1740,11 +1720,6 @@ parameters:
count: 2
path: ../../typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
- -
- message: "#^Parameter \\#1 \\$uid of method TYPO3\\\\CMS\\\\Core\\\\Resource\\\\ResourceFactory\\:\\:getFileReferenceObject\\(\\) expects int, string given\\.$#"
- count: 2
- path: ../../typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
-
-
message: "#^Variable \\$conf on left side of \\?\\? always exists and is not nullable\\.$#"
count: 14
diff --git a/typo3/sysext/core/Classes/Resource/Filter/FileExtensionFilter.php b/typo3/sysext/core/Classes/Resource/Filter/FileExtensionFilter.php
index e34044c06397..26b06bafbde7 100644
--- a/typo3/sysext/core/Classes/Resource/Filter/FileExtensionFilter.php
+++ b/typo3/sysext/core/Classes/Resource/Filter/FileExtensionFilter.php
@@ -62,7 +62,7 @@ class FileExtensionFilter
continue;
}
$parts = GeneralUtility::revExplode('_', (string)$reference, 2);
- $fileReferenceUid = $parts[count($parts) - 1];
+ $fileReferenceUid = (int)$parts[count($parts) - 1];
try {
$fileReference = GeneralUtility::makeInstance(ResourceFactory::class)->getFileReferenceObject($fileReferenceUid);
$file = $fileReference->getOriginalFile();
diff --git a/typo3/sysext/core/Classes/Resource/ResourceFactory.php b/typo3/sysext/core/Classes/Resource/ResourceFactory.php
index 2244c4624a50..1c098ee757bc 100644
--- a/typo3/sysext/core/Classes/Resource/ResourceFactory.php
+++ b/typo3/sysext/core/Classes/Resource/ResourceFactory.php
@@ -53,7 +53,7 @@ class ResourceFactory implements SingletonInterface
/**
* @var FileReference[]
*/
- protected $fileReferenceInstances = [];
+ protected array $fileReferenceInstances = [];
public function __construct(
protected readonly StorageRepository $storageRepository,
@@ -422,18 +422,10 @@ class ResourceFactory implements SingletonInterface
* @param int $uid The uid of the file usage (sys_file_reference) to instantiate.
* @param array $fileReferenceData The record row from database.
* @param bool $raw Whether to get raw results without performing overlays
- * @return FileReference
- * @throws \InvalidArgumentException
* @throws Exception\ResourceDoesNotExistException
*/
- public function getFileReferenceObject($uid, array $fileReferenceData = [], $raw = false)
+ public function getFileReferenceObject(int $uid, array $fileReferenceData = [], bool $raw = false): FileReference
{
- if (!is_numeric($uid)) {
- throw new \InvalidArgumentException(
- 'The reference UID for the file (sys_file_reference) has to be numeric. UID given: "' . $uid . '"',
- 1300086584
- );
- }
if (!($this->fileReferenceInstances[$uid] ?? false)) {
// Fetches data in case $fileData is empty
if (empty($fileReferenceData)) {
@@ -454,13 +446,10 @@ class ResourceFactory implements SingletonInterface
* Creates a file usage object from an array of fileReference data
* from sys_file_reference table.
* Requires a database row to be already fetched and present.
- *
- * @return FileReference
*/
- public function createFileReferenceObject(array $fileReferenceData)
+ public function createFileReferenceObject(array $fileReferenceData): FileReference
{
- $fileReferenceObject = GeneralUtility::makeInstance(FileReference::class, $fileReferenceData);
- return $fileReferenceObject;
+ return GeneralUtility::makeInstance(FileReference::class, $fileReferenceData);
}
/**
diff --git a/typo3/sysext/extbase/Classes/Service/ImageService.php b/typo3/sysext/extbase/Classes/Service/ImageService.php
index 2d0dbcc5a1c2..3bc93d9c1645 100644
--- a/typo3/sysext/extbase/Classes/Service/ImageService.php
+++ b/typo3/sysext/extbase/Classes/Service/ImageService.php
@@ -156,7 +156,7 @@ class ImageService implements SingletonInterface
}
if (MathUtility::canBeInterpretedAsInteger($src)) {
if ($treatIdAsReference) {
- $image = $this->resourceFactory->getFileReferenceObject($src);
+ $image = $this->resourceFactory->getFileReferenceObject((int)$src);
} else {
$image = $this->resourceFactory->getFileObject($src);
}
diff --git a/typo3/sysext/form/Classes/Mvc/Property/TypeConverter/UploadedFileReferenceConverter.php b/typo3/sysext/form/Classes/Mvc/Property/TypeConverter/UploadedFileReferenceConverter.php
index c8abdf8ac85a..366d8891f2e6 100644
--- a/typo3/sysext/form/Classes/Mvc/Property/TypeConverter/UploadedFileReferenceConverter.php
+++ b/typo3/sysext/form/Classes/Mvc/Property/TypeConverter/UploadedFileReferenceConverter.php
@@ -151,7 +151,7 @@ class UploadedFileReferenceConverter extends AbstractTypeConverter
$resource = $this->createFileReferenceFromFalFileObject($this->resourceFactory->getFileObject($fileUid));
} else {
$resource = $this->createFileReferenceFromFalFileReferenceObject(
- $this->resourceFactory->getFileReferenceObject($resourcePointer),
+ $this->resourceFactory->getFileReferenceObject((int)$resourcePointer),
(int)$resourcePointer
);
}
diff --git a/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php b/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
index 214e64d0e460..719a1ae9d11b 100644
--- a/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
+++ b/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
@@ -428,7 +428,7 @@ class ContentObjectRenderer implements LoggerAwareInterface
if ($objectType === 'File') {
$this->currentFile = GeneralUtility::makeInstance(ResourceFactory::class)->retrieveFileOrFolderObject($identifier);
} elseif ($objectType === 'FileReference') {
- $this->currentFile = GeneralUtility::makeInstance(ResourceFactory::class)->getFileReferenceObject($identifier);
+ $this->currentFile = GeneralUtility::makeInstance(ResourceFactory::class)->getFileReferenceObject((int)$identifier);
}
} catch (ResourceDoesNotExistException $e) {
$this->currentFile = null;
@@ -3587,7 +3587,7 @@ class ContentObjectRenderer implements LoggerAwareInterface
if (MathUtility::canBeInterpretedAsInteger($file)) {
$treatIdAsReference = $this->stdWrapValue('treatIdAsReference', $fileArray);
if (!empty($treatIdAsReference)) {
- $fileReference = $this->getResourceFactory()->getFileReferenceObject($file);
+ $fileReference = $this->getResourceFactory()->getFileReferenceObject((int)$file);
$fileObject = $fileReference->getOriginalFile();
} else {
$fileObject = $this->getResourceFactory()->getFileObject($file);
@@ -3620,10 +3620,10 @@ class ContentObjectRenderer implements LoggerAwareInterface
$processingConfiguration['sample'] = (bool)$this->stdWrapValue('sample', $fileArray);
$processingConfiguration['additionalParameters'] = $this->stdWrapValue('params', $fileArray);
$processingConfiguration['frame'] = (int)$this->stdWrapValue('frame', $fileArray);
- if ($fileReference instanceof FileReference) {
- $processingConfiguration['crop'] = $this->getCropAreaFromFileReference($fileReference, $fileArray);
- } else {
+ if ($fileReference === null) {
$processingConfiguration['crop'] = $this->getCropAreaFromFromTypoScriptSettings($fileObject, $fileArray);
+ } else {
+ $processingConfiguration['crop'] = $this->getCropAreaFromFileReference($fileReference, $fileArray);
}
// Possibility to cancel/force profile extraction
--
GitLab