Bug #23348
closed
Array returned by t3lib_stdGraphic::imageMagickIdentify() contains useless value for file extension
100%
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)
Files
Updated by Jigal van Hemert over 13 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?
Updated by Stefan Galinski over 12 years ago
- Target version deleted (
0)
Maybe there can be found an easy solution yet as the old IM versions are not supported anymore?
Updated by Alexander Opitz almost 11 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)?
Updated by Grigori Prokhorov almost 11 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
Updated by Jigal van Hemert almost 11 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?
Updated by Grigori Prokhorov almost 11 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!
Updated by Alexander Opitz almost 11 years ago
- Status changed from Needs Feedback to New
Updated by Mathias Schreiber over 9 years ago
- Target version set to 7.2 (Frontend)
- Is Regression set to No
Updated by Benni Mack almost 9 years ago
- Target version changed from 7.2 (Frontend) to 7.4 (Backend)
Updated by Susanne Moog over 8 years ago
- Target version changed from 7.4 (Backend) to 7.5
Updated by Benni Mack over 8 years ago
- Target version changed from 7.5 to 7 LTS
Updated by Susanne Moog over 6 years ago
- Category set to Image Generation / GIFBUILDER
Updated by Sybille Peters about 6 years 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!
Updated by Gerrit Code Review about 5 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/+/60311
Updated by Gerrit Code Review about 5 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/+/60311
Updated by Gerrit Code Review about 5 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/+/60311
Updated by Gerrit Code Review about 5 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/+/60311
Updated by Gerrit Code Review about 5 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/+/60311
Updated by Gerrit Code Review about 5 years ago
Patch set 6 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/+/60311
Updated by Benni Mack about 5 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 4e807a8a24286e5701a7eb7f8a7bb7d0c7acc4f9.
Updated by Benni Mack almost 5 years ago
- Status changed from Resolved to Closed
Updated by Gerrit Code Review over 3 years ago
- Status changed from Closed to Under Review
Patch set 1 for branch 9.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/+/65711
Updated by Gerrit Code Review over 3 years ago
Patch set 2 for branch 9.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/+/65711
Updated by Gerrit Code Review over 3 years ago
Patch set 3 for branch 9.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/+/65711
Updated by Gerrit Code Review over 3 years ago
Patch set 3 for branch 9.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/+/65711
Updated by Gerrit Code Review over 3 years ago
Patch set 3 for branch 9.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/+/65711
Updated by Gerrit Code Review over 3 years ago
Patch set 4 for branch 9.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/+/65711
Updated by Gerrit Code Review over 3 years ago
Patch set 4 for branch 9.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/+/65711
Updated by Gerrit Code Review over 3 years ago
Patch set 4 for branch 9.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/+/65711
Updated by Gerrit Code Review over 3 years ago
Patch set 4 for branch 9.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/+/65711
Updated by Benni Mack over 3 years ago
- Status changed from Under Review to Resolved
Applied in changeset 1451e5605abb242ebb120998dc23cc2416eb5582.