Ticket #5397 (closed defect: fixed)

Opened 7 months ago

Last modified 7 months ago

exif_read_data() called for non-jpeg/tiff image files, resulting in error

Reported by: fitztrev Assigned to: westi
Priority: high Milestone: 2.5
Component: Administration Version: 2.5
Severity: major Keywords: has-patch
Cc:

Description

exif_read_data(), added in #5162, only accepts JPEG and TIFF files but is being called for GIF and PNG as well.

When uploading a GIF file (ex: the Google logo) via the Write Post upload form, I'm getting the following error message:

Warning: exif_read_data(logo.gif) [function.exif-read-data]: File not supported in /var/www/trunk/wp-admin/includes/image.php on line 291

Attachments

5397.exif.suppress-errors.diff (0.9 kB) - added by DD32 on 12/10/07 06:22:50.
5397.exif.the-proper-way.diff (1.7 kB) - added by DD32 on 12/10/07 06:58:13.

Change History

12/10/07 06:21:22 changed by DD32

Confirmed with a .bmp on PHP5.2.4 + Apache/win32

Given that the filetypes that the exif functions accept might change between installs/versions (i'm honestly not sure), I think the best method might to suppress the errors, That works fine here and allows the upload to go ahead as usual.

12/10/07 06:22:50 changed by DD32

  • attachment 5397.exif.suppress-errors.diff added.

12/10/07 06:23:09 changed by DD32

  • keywords set to has-patch.
  • version set to 2.4.

12/10/07 06:38:12 changed by darkdragon

Suppressing errors is never sexy.

If I was going to make a patch, I would explicitly check for either GIF and TIFF and run that through a filter.

However, since I'm not going to make a patch to counter yours, then I think yours is sexy... enough.

12/10/07 06:58:02 changed by DD32

I agree that error suppressing is never sexy, So i'll attach another patch without it.

12/10/07 06:58:13 changed by DD32

  • attachment 5397.exif.the-proper-way.diff added.

12/10/07 07:22:43 changed by darkdragon

Your first patch covered two areas or functions. Your second, while sexier, only covers one of the areas/functions. Is this intentional? 

Line 273 - iptcparse() (not covered)
Line 291 - exif_read_data() (covered)

12/10/07 07:25:29 changed by DD32

Yep, Its intentional, The first function(iptcparse()) is not called unless there is data for it to parse( $infoAPP13? from getimagesize() ).

12/10/07 07:27:28 changed by darkdragon

That is one fine patch then.

12/20/07 22:13:39 changed by westi

  • owner changed from anonymous to westi.
  • status changed from new to assigned.

12/20/07 22:18:28 changed by westi

  • status changed from assigned to closed.
  • resolution set to fixed.

(In [6438]) Ensure we don't call exif_read_data() on unsupported file types. Fixes #5397 props DD32