Bug #23348

Array returned by t3lib_stdGraphic::imageMagickIdentify() contains useless value for file extension

Added by Grigori Prokhorov over 8 years ago. Updated 12 months ago.

Status:
New
Priority:
Should have
Assignee:
-
Category:
Image Generation / GIFBUILDER
Target version:
-
Start date:
2010-08-04
Due date:
% Done:

0%

TYPO3 Version:
4.4
PHP Version:
4.3
Tags:
Complexity:
Is Regression:
No
Sprint Focus:

Description

The current version of the method t3lib_stdGraphic::imageMagickIdentify() does what it's supposed to do according to documentation.

Nevertheless the returned value for "file extension" is of no real use and ignores ImageMagick's identifying capabilities by returning a ereg result on the originally passed filename instead of extracting the actual file type determined by ImageMagick.

In other words - the documentation states, that imageMagickIdentify() "Returns an array where [0]/[1] is w/h, [2] is extension and [3] is the filename. Using ImageMagick".
The point here is, that [2] is filled with $reg0 which is created via ereg('([^\.]*)$',$imagefile,$reg);
While this works for images which provide a file extension as part of their filename, it obviously fails on files missing it.

This is the case when you try to identify an image e.g. before copying it to your uploads-folder in order to ensure it's a valid image file and thus run the check on a temporary file (which does not have an extension).

This issue could easily be solved by simply using the info ImageMagick provides by itself instead of relying on the filename.

A proposed patch is included with the report and should be checked for use.

The patch basically includes only two changes (line numbers according to r8200):

1. remove call to ereg on line 2672
2. change return array to use $splitinfo1 instead of $reg0.

The second change bases on a reliable assumtion, because ImageMagick's return string always starts according to the following syntax:

"fullFileName fileType imageDimension moreInfo_0 moreInfo_1 moreInfo_2 moreInfo_n"
e.g. rose.jpg JPEG 640x480 DirectClass 87kb 0.050u 0:01

This patch would make sure that the file type is always returned no matter what the filename is as long as the identification succeeds.

With kind regards,
Grigori Prokhorov

(issue imported from #M15347)

t3lib_stdGraphic-imageMagickIdentify-patch.php View (971 Bytes) Administrator Admin, 2010-08-04 12:32

History

#1 Updated by Jigal van Hemert about 8 years ago

First of all the third item in the result array is defined as the file extension of the file which is identify-ied. This in formation is used in this way throughout the core.

It won't be a good thing to change it to the detected file type. We could however add an extra element to that array with this information.

I found no reliable way to implement this with all supported versions of ImageMagick and GraphicsMagick. Output of identify from the manuals of the various vesions:
IM 4.2.9: images/aquarium.miff 640x480 PseudoClass 256c 308135b MIFF 1s
IM 5.2 : images/aquarium.miff 640x480 PseudoClass 256c 308135b MIFF 1s
IM 6.6.7: rose.jpg JPEG 640x480 DirectClass 87kb 0.050u 0:01
GM : images/aquarium.miff 640x480 PseudoClass 256c 308135b MIFF 0.000u 0:01

More problems arrise when there are spaces in the image path:
D:/TYPO3/TYPO3 Core Sources SVN/trunk/typo3/gfx/fileicons/3ds.gif GIF 18x16+0+0 PseudoClass 64c 8-bit 370 0.000u 0:01
splitting on spaces will result in the file type "Core".

Recent versions of IM/GM know the option -format to set up the format of the output of identify. This way only the type and size could be shown, but the manuals of IM 4.2.9 and IM 5.2 did not mention this option.

Do you see a reliable way on all supported versions to get the file type information?

#2 Updated by Stefan Galinski over 7 years ago

  • Target version deleted (0)

Maybe there can be found an easy solution yet as the old IM versions are not supported anymore?

#3 Updated by Alexander Opitz over 5 years ago

Hi,

as this issue is very old. Does the problem still exists within newer versions of TYPO3 CMS (4.5 or 6.1)?

#4 Updated by Grigori Prokhorov over 5 years ago

Hi Alexander,

yes, the issue still exists unchanged in versions 4.5 troughout 6.1, see the following links:

As you can see, the code still relies on the return value of the regex for the file-extension.

Best regards,
Grigori Prokhorov

#5 Updated by Jigal van Hemert over 5 years ago

Proposal: turn this into a feature for 6.2 and use the -format option of IM/GM to return only the necessary information. The detected file type can then be added to the array that is returned.
The rest of the core expects the file extension instead of the file type, so the only possibility is an extra item in the array as a new feature. Will that help?

#6 Updated by Grigori Prokhorov over 5 years ago

Yes, I would also vote for Jigal's proposal to simply add the file type to the returned array and to gather the file type from a new method which implements an Image-/Graphics-Magick version-dependent extraction procedure for the file type from the string returned by identify.

Edit: On a second thought, if the versions < 6.2 can be considered as not patch-worthy in the context of this issue, we can use the -format option and save the effort of a new method.

Edit 2: All in all, Jigal's proposal is just fine, let's agree on this way, I don't believe there is another realistic option to adress this issue. Kudos!

#7 Updated by Alexander Opitz over 5 years ago

  • Status changed from Needs Feedback to New

#8 Updated by Mathias Schreiber about 4 years ago

  • Target version set to 7.2 (Frontend)
  • Is Regression set to No

#9 Updated by Benni Mack almost 4 years ago

  • Target version changed from 7.2 (Frontend) to 7.4 (Backend)

#10 Updated by Susanne Moog over 3 years ago

  • Target version changed from 7.4 (Backend) to 7.5

#11 Updated by Benni Mack over 3 years ago

  • Target version changed from 7.5 to 7 LTS

#12 Updated by Mathias Schreiber over 3 years ago

  • Target version deleted (7 LTS)

#13 Updated by Susanne Moog over 1 year ago

  • Category set to Image Generation / GIFBUILDER

#14 Updated by Sybille Peters 12 months ago

Thank you for your report.

Even though it has been some time, would you consider checking if your patch idea is still up to date and upload it to our Gerrit review server?

Someone could do this for you, but I am thinking you might like the opportunity to contribute to TYPO3 yourself.

You can find a description of the TYPO3 contribution workflow here: https://docs.typo3.org/typo3cms/ContributionWorkflowGuide/

Hint: If you get stuck anywhere, ask on Slack in the #typo3-cms-coredev channel. You can register in the TYPO3 slack workspace here: https://forger.typo3.com/slack

Thank you in advance!

Also available in: Atom PDF