Bug #92958: Image processing test fails when gif is not in imagefile_ext list - TYPO3 Core - TYPO3 Forge

Custom queries

Accessibility
Easy tasks
Forge Triage
JavaScript issues
JavaScript Issues (w/o Epics/Stories)
My open issues
No feedback within 90 days
Open Bugs
Open issues of 10
Recently modified
Regressions
Stabilization Issues
Usability or UX

Actions

Copy link

Bug #92958

closed

Image processing test fails when gif is not in imagefile_ext list

Added by Tobias D almost 4 years ago. Updated almost 4 years ago.

Status:

Closed

Priority:

Should have

Assignee:

Category:

Install Tool

Target version:

Start date:

2020-11-30

Due date:

% Done:

100%

Estimated time:

TYPO3 Version:

PHP Version:

7.3

Tags:

Complexity:

Is Regression:

Sprint Focus:

Description

When gif is not in the imagefile_ext list, the image processing test throws an exception:

explode() expects parameter 2 to be string, null given
in \TYPO3\CMS\Install\Controller\EnvironmentController::getImageTestResponse() on line 1024

This happens, because gif related tests are not skipped when not in imagefile_ext, like .tif and .ai for example.
Because of that, $imResult is null in line 664, leading to the exception.

History
Notes
Property changes
Associated revisions

Actions

Copy link

Updated by Simon Gilli almost 4 years ago

Category changed from Image Cropping to Install Tool

Actions

Copy link

Updated by Guido Schmechel almost 4 years ago

I can confirm the whole thing for the v11/current master.

You can still secure the code quite well and return a "success: false". The tests then do not throw any errors for the time being.

The question is? What does one expect here as a user?
- Should the GIF tests be removed? It doesn't make sense at first, because you still want to convert. (See .tif example)
- We could output a "Please support gif" flash message, but that might just confuse the integrator. (And is actually not the user's wish either)
- We could manipulate the tests. So they are running always with GIF. Independent of the imagefile_ext setting (My favorite)

The trigger is that the function getImageDimensions() in typo3/sysext/core/Classes/Imaging/GraphicalFunctions.php checks for $this->imageFileExt. gif is now missing there and null is returned.
https://github.com/TYPO3/TYPO3.CMS/blob/master/typo3/sysext/core/Classes/Imaging/GraphicalFunctions.php#L2193

With a public setter we can force the gif extension from the EnvironmentController. Here a simple and hacky example ;-)

$fileExt = explode(',', $GLOBALS['TYPO3_CONF_VARS']['GFX']['imagefile_ext'] . ',gif');
$imageProcessor->setImageFileExt($fileExt);

Actions

Copy link

Updated by Martin Kutschker almost 4 years ago

Hacky does not sound promising. OTOH, dependencies to globals aren't great either.

Within the installer it may not be of interest what files may be uploaded by a user.

If the instance can handle GIFs used in TypoScript or Fluid directly, then the installer must be able run tests on them.

Actions

Copy link

Updated by Gerrit Code Review almost 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/+/67344

Actions

Copy link

Updated by Gerrit Code Review almost 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/+/67344

Actions

Copy link

Updated by Gerrit Code Review almost 4 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/c/Packages/TYPO3.CMS/+/67344

Actions

Copy link

Updated by Gerrit Code Review almost 4 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/c/Packages/TYPO3.CMS/+/67344

Actions

Copy link

Updated by Gerrit Code Review almost 4 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/c/Packages/TYPO3.CMS/+/67344

Actions

Copy link

Updated by Gerrit Code Review almost 4 years ago

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

Actions

Copy link

#10