[!!!][TASK] Harden \TYPO3\CMS\Extbase\Service\ImageService 94/59594/7
authorAlexander Schnitzler <git@alexanderschnitzler.de>
Thu, 31 Jan 2019 18:20:58 +0000 (19:20 +0100)
committerAnja Leichsenring <aleichsenring@ab-softlab.de>
Fri, 1 Feb 2019 14:30:17 +0000 (15:30 +0100)
- Use strict type mode
- Use type hints whereever possible

Releases: master
Resolves: #87599
Change-Id: I8840b7fad16c4fffb7f50973fb97a498e0f683e4
Reviewed-on: https://review.typo3.org/59594
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Mona Muzaffar <mona.muzaffar@gmx.de>
Tested-by: Mona Muzaffar <mona.muzaffar@gmx.de>
Tested-by: TYPO3com <noreply@typo3.com>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
typo3/sysext/core/Documentation/Changelog/master/Breaking-87594-HardenExtbase.rst
typo3/sysext/extbase/Classes/Service/ImageService.php
typo3/sysext/fluid/Classes/ViewHelpers/ImageViewHelper.php
typo3/sysext/fluid/Tests/Unit/ViewHelpers/ImageViewHelperTest.php

index edba892..25175c9 100644 (file)
@@ -27,6 +27,9 @@ While hardening Extbase classes, method signatures changed due to an enforced st
 - :php:`\TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject::_getProperty`
 - :php:`\TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject::_getProperties`
 - :php:`\TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject::_getCleanProperty`
+- :php:`\TYPO3\CMS\Extbase\Service\ImageService::applyProcessingInstructions`
+- :php:`\TYPO3\CMS\Extbase\Service\ImageService::getImageUri`
+- :php:`\TYPO3\CMS\Extbase\Service\ImageService::getImage`
 
 
 Impact
index 8d3139c..c16a746 100644 (file)
@@ -1,4 +1,6 @@
 <?php
+declare(strict_types = 1);
+
 namespace TYPO3\CMS\Extbase\Service;
 
 /*
@@ -57,8 +59,13 @@ class ImageService implements \TYPO3\CMS\Core\SingletonInterface
      * @param array $processingInstructions
      * @return ProcessedFile
      */
-    public function applyProcessingInstructions($image, $processingInstructions)
+    public function applyProcessingInstructions($image, array $processingInstructions): ProcessedFile
     {
+        /*
+         * todo: this method should be split to be able to have a proper method signature.
+         * todo: actually, this method only really works with objects of type \TYPO3\CMS\Core\Resource\File, as this
+         * todo: is the only implementation that supports the support method.
+         */
         if (is_callable([$image, 'getOriginalFile'])) {
             // Get the original file from the file reference
             $image = $image->getOriginalFile();
@@ -77,7 +84,7 @@ class ImageService implements \TYPO3\CMS\Core\SingletonInterface
      * @param bool|false $absolute Force absolute URL
      * @return string
      */
-    public function getImageUri(FileInterface $image, $absolute = false)
+    public function getImageUri(FileInterface $image, bool $absolute = false): string
     {
         $imageUrl = $image->getPublicUrl();
         $parsedUrl = parse_url($imageUrl);
@@ -109,13 +116,13 @@ class ImageService implements \TYPO3\CMS\Core\SingletonInterface
      * It should be removed once we do not support string sources for images anymore.
      *
      * @param string $src
-     * @param mixed $image
+     * @param FileInterface|null $image
      * @param bool $treatIdAsReference
-     * @return FileInterface|FileReference
+     * @return FileInterface
      * @throws \UnexpectedValueException
      * @internal
      */
-    public function getImage($src, $image, $treatIdAsReference)
+    public function getImage(string $src, ?FileInterface $image, bool $treatIdAsReference): FileInterface
     {
         if ($image === null) {
             $image = $this->getImageFromSourceString($src, $treatIdAsReference);
@@ -125,8 +132,7 @@ class ImageService implements \TYPO3\CMS\Core\SingletonInterface
         }
 
         if (!($image instanceof File || $image instanceof FileReference)) {
-            $class = is_object($image) ? get_class($image) : 'null';
-            throw new \UnexpectedValueException('Supplied file object type ' . $class . ' for ' . $src . ' must be File or FileReference.', 1382687163);
+            throw new \UnexpectedValueException('Supplied file object type ' . get_class($image) . ' for ' . $src . ' must be File or FileReference.', 1382687163);
         }
 
         return $image;
@@ -139,7 +145,7 @@ class ImageService implements \TYPO3\CMS\Core\SingletonInterface
      * @param bool $treatIdAsReference
      * @return FileInterface|FileReference|\TYPO3\CMS\Core\Resource\Folder
      */
-    protected function getImageFromSourceString($src, $treatIdAsReference)
+    protected function getImageFromSourceString(string $src, bool $treatIdAsReference): object
     {
         if ($this->environmentService->isEnvironmentInBackendMode() && strpos($src, '../') === 0) {
             $src = substr($src, 3);
@@ -168,7 +174,7 @@ class ImageService implements \TYPO3\CMS\Core\SingletonInterface
      *
      * @param ProcessedFile $processedImage
      */
-    protected function setCompatibilityValues(ProcessedFile $processedImage)
+    protected function setCompatibilityValues(ProcessedFile $processedImage): void
     {
         if ($this->environmentService->isEnvironmentInFrontendMode()) {
             $GLOBALS['TSFE']->lastImageInfo = $this->getCompatibilityImageResourceValues($processedImage);
@@ -184,7 +190,7 @@ class ImageService implements \TYPO3\CMS\Core\SingletonInterface
      * @param ProcessedFile $processedImage
      * @return array
      */
-    protected function getCompatibilityImageResourceValues(ProcessedFile $processedImage)
+    protected function getCompatibilityImageResourceValues(ProcessedFile $processedImage): array
     {
         return [
             0 => $processedImage->getProperty('width'),
index f6a76c7..7acff67 100644 (file)
@@ -95,8 +95,8 @@ class ImageViewHelper extends AbstractTagBasedViewHelper
         $this->registerTagAttribute('longdesc', 'string', 'Specifies the URL to a document that contains a long description of an image', false);
         $this->registerTagAttribute('usemap', 'string', 'Specifies an image as a client-side image-map', false);
 
-        $this->registerArgument('src', 'string', 'a path to a file, a combined FAL identifier or an uid (int). If $treatIdAsReference is set, the integer is considered the uid of the sys_file_reference record. If you already got a FAL object, consider using the $image parameter instead');
-        $this->registerArgument('treatIdAsReference', 'bool', 'given src argument is a sys_file_reference record');
+        $this->registerArgument('src', 'string', 'a path to a file, a combined FAL identifier or an uid (int). If $treatIdAsReference is set, the integer is considered the uid of the sys_file_reference record. If you already got a FAL object, consider using the $image parameter instead', false, '');
+        $this->registerArgument('treatIdAsReference', 'bool', 'given src argument is a sys_file_reference record', false, false);
         $this->registerArgument('image', 'object', 'a FAL object');
         $this->registerArgument('crop', 'string|bool', 'overrule cropping of image (setting to FALSE disables the cropping set in FileReference)');
         $this->registerArgument('cropVariant', 'string', 'select a cropping variant, in case multiple croppings have been specified or stored in FileReference', false, 'default');
@@ -120,7 +120,7 @@ class ImageViewHelper extends AbstractTagBasedViewHelper
      */
     public function render()
     {
-        if (($this->arguments['src'] === null && $this->arguments['image'] === null) || ($this->arguments['src'] !== null && $this->arguments['image'] !== null)) {
+        if (($this->arguments['src'] === '' && $this->arguments['image'] === null) || ($this->arguments['src'] !== '' && $this->arguments['image'] !== null)) {
             throw new Exception('You must either specify a string src or a File object.', 1382284106);
         }
 
index 1c50006..a1384dd 100644 (file)
@@ -16,6 +16,7 @@ namespace TYPO3\CMS\Fluid\Tests\Unit\ViewHelpers;
 
 use TYPO3\CMS\Core\Resource\File;
 use TYPO3\CMS\Core\Resource\FileReference;
+use TYPO3\CMS\Core\Resource\ProcessedFile;
 use TYPO3\CMS\Extbase\Service\ImageService;
 use TYPO3\CMS\Fluid\ViewHelpers\ImageViewHelper;
 use TYPO3\TestingFramework\Fluid\Unit\ViewHelpers\ViewHelperBaseTestcase;
@@ -50,7 +51,7 @@ class ImageViewHelperTest extends ViewHelperBaseTestcase
     {
         return [
             [['image' => null]],
-            [['src' => null]],
+            [['src' => '']],
             [['src' => 'something', 'image' => 'something']],
         ];
     }
@@ -142,14 +143,24 @@ class ImageViewHelperTest extends ViewHelperBaseTestcase
             ->disableOriginalConstructor()
             ->getMock();
         $originalFile->expects($this->any())->method('getProperties')->willReturn([]);
+
+        $processedFile = $this->getMockBuilder(ProcessedFile::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $processedFile->expects($this->any())->method('getProperty')->willReturnMap([
+            ['width', $arguments['width']],
+            ['height', $arguments['height']],
+        ]);
+
         $this->inject($image, 'originalFile', $originalFile);
         $this->inject($image, 'propertiesOfFileReference', []);
         $imageService = $this->getMockBuilder(ImageService::class)
             ->setMethods(['getImage', 'applyProcessingInstructions', 'getImageUri'])
             ->getMock();
         $imageService->expects($this->once())->method('getImage')->willReturn($image);
-        $imageService->expects($this->once())->method('applyProcessingInstructions')->with($image, $this->anything())->willReturn($image);
-        $imageService->expects($this->once())->method('getImageUri')->with($image)->willReturn('test.png');
+        $imageService->expects($this->once())->method('applyProcessingInstructions')->with($image, $this->anything())->willReturn($processedFile);
+        $imageService->expects($this->once())->method('getImageUri')->with($processedFile)->willReturn('test.png');
 
         $this->inject($this->viewHelper, 'imageService', $imageService);