Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad exif data prevents image upload #68

Closed
mauritsvanrees opened this issue Nov 5, 2018 · 4 comments
Closed

Bad exif data prevents image upload #68

mauritsvanrees opened this issue Nov 5, 2018 · 4 comments
Assignees

Comments

@mauritsvanrees
Copy link
Member

I have an image with apparently bad Exif data. Uploading this image in Plone 5.1.4 goes wrong. With a pdb in place:

WARNING plone.namedfile.utils cannot convert argument to integer
> /Users/maurits/shared-eggs/cp27m/plone.namedfile-4.2.5-py2.7.egg/plone/namedfile/utils/__init__.py(274)rotate_image()
-> raise
(Pdb) l
266          try:
267              exif_bytes = piexif.dump(exif_data)
268          except Exception as e:
269              log.warn(e)
270              try:
271                  del(exif_data['Exif'][piexif.ExifIFD.SceneType])
272              except KeyError as ke:
273                  import pdb; pdb.set_trace()
274  ->                raise
275              # This Element piexif.ExifIFD.SceneType cause error on dump
276              exif_bytes = piexif.dump(exif_data)
277          output_image_data = StringIO()
278          img.save(output_image_data, format=fmt, exif=exif_bytes)
279          width, height = img.size
(Pdb) exif_data.keys()
['Exif', '0th', 'Interop', '1st', 'thumbnail', 'GPS’]
(Pdb) piexif.ExifIFD.SceneType
41729
(Pdb) sorted(exif_data['Exif'].keys())
[33434, 33437, 34850, 34855, 36864, 36867, 36868, 37121, 37377, 37378, 37380, 37383, 37385, 37386, 37500, 37510, 37520, 37521, 37522, 40960, 40961, 40962, 40963, 40965, 41486, 41487, 41488, 41985, 41986, 41987, 41990]

So in line 367, piexif.dump(exif_data) throws an exception:

Traceback (most recent call last):
  File "/Users/maurits/shared-eggs/cp27m/plone.namedfile-4.2.5-py2.7.egg/plone/namedfile/utils/__init__.py", line 267, in rotate_image
    exif_bytes = piexif.dump(exif_data)
  File "/Users/maurits/shared-eggs/cp27m/piexif-1.0.13-py2.7.egg/piexif/_dump.py", line 74, in dump
    gps_set = _dict_to_bytes(gps_ifd, "GPS", zeroth_length + exif_length)
  File "/Users/maurits/shared-eggs/cp27m/piexif-1.0.13-py2.7.egg/piexif/_dump.py", line 337, in _dict_to_bytes
    offset)
  File "/Users/maurits/shared-eggs/cp27m/piexif-1.0.13-py2.7.egg/piexif/_dump.py", line 193, in _value_to_bytes
    value_str = (_pack_byte(*raw_value) +
  File "/Users/maurits/shared-eggs/cp27m/piexif-1.0.13-py2.7.egg/piexif/_dump.py", line 162, in _pack_byte
    return struct.pack("B" * len(args), *args)
error: cannot convert argument to integer

plone.namedfile catches this exception and turns it into a warning.
It then tries to fix the Exif data by deleting the SceneType key. Apparently that key caused a problem in earlier testing. But that key is not there in my image, so it throws a KeyError.
In fact, only when I delete all keys from exif_data[‘Exif’] can I call piexif.dump(exif_data) without getting an error.

In my particular use case, this is an image that was uploaded without problems to a Plone 4.3 site, and that I am importing with transmogrifier into Plone 5.1. It is not an issue with transmogrifier: simply uploading the image already throws the error.

I would propose that in NamedBlobImage.__init__ we catch all exceptions thrown by rotate_image and ignore the Exif data. That is the only place where the function is called.
With that change, the full image actually shows up rotated just fine, contrary to how it shows in Plone 4.3. That may depend on the browser. The scales show up fine in both versions.

Note: the Exif handling was added in plone.namedfile 4.1.1. The error should be there in Plone 5.1 and 5.2. I will create PRs.

@mauritsvanrees
Copy link
Member Author

mauritsvanrees commented Nov 5, 2018

Here is an example JPEG with bad Exif data, taken from a client website:

recipe-with-bad-exif-data

I have no idea what the Exif data says, but the cake on the photo should presumably be black-yellow-black from top to bottom. It currently shows wrong when you follow the direct link. (Link may expire soon as we are doing migration, or it may start showing a correct image. Image may be copyrighted.)

@1letter
Copy link

1letter commented Nov 7, 2018

I run in the following error when i migrate a plone site from 4.3.18 to 5.1.4 via p.a.c migratorview. it sounds like the same issue?

Traceback (most recent call last):
  File "/migration/Plone5/buildout-cache/eggs/Products.contentmigration-2.1.19-py2.7.egg/Products/contentmigration/basemigrator/walker
.py", line 194, in migrate
    migrator.migrate()
  File "/migration/Plone5/buildout-cache/eggs/Products.contentmigration-2.1.19-py2.7.egg/Products/contentmigration/basemigrator/migrat
or.py", line 220, in migrate
    method()
  File "/migration/Plone5/buildout-cache/eggs/plone.app.contenttypes-1.4.15-py2.7.egg/plone/app/contenttypes/migration/migration.py", 
line 318, in migrate_schema_fields
    migrate_imagefield(self.old, self.new, 'image', 'image')
  File "/migration/Plone5/buildout-cache/eggs/plone.app.contenttypes-1.4.15-py2.7.egg/plone/app/contenttypes/migration/field_migrators
.py", line 78, in migrate_imagefield
    filename=filename)
  File "/migration/Plone5/buildout-cache/eggs/plone.namedfile-4.2.5-py2.7.egg/plone/namedfile/file.py", line 391, in __init__
    self.data)
  File "/migration/Plone5/buildout-cache/eggs/plone.namedfile-4.2.5-py2.7.egg/plone/namedfile/utils/__init__.py", line 270, in rotate_
image
    del(exif_data['Exif'][piexif.ExifIFD.SceneType])
KeyError: 41729

@mauritsvanrees
Copy link
Member Author

Yes, that looks like exactly the same problem.
Good to know I am not the only one. :-)

If you need a fix fast, you could try a checkout of plone.namedfile branch issue-68-branch42, from PR #70.

mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Nov 8, 2018
Branch: refs/heads/4.2.x
Date: 2018-11-05T21:10:24+01:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.namedfile@82c9caf

Do not fail image upload when Exif data is bad.

plone/plone.namedfile#68

Files changed:
M plone/namedfile/file.py
Repository: plone.namedfile

Branch: refs/heads/4.2.x
Date: 2018-11-05T22:08:30+01:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.namedfile@2f553c5

Forgot to commit news file

Files changed:
A news/68.bugfix
Repository: plone.namedfile

Branch: refs/heads/4.2.x
Date: 2018-11-09T00:32:47+01:00
Author: Jens W. Klein (jensens) <jk@kleinundpartner.at>
Commit: plone/plone.namedfile@d7163c4

Merge pull request #70 from plone/issue-68-branch42

Do not fail image upload when Exif data is bad. [4.2]

Files changed:
A news/68.bugfix
M plone/namedfile/file.py
mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Nov 8, 2018
Branch: refs/heads/master
Date: 2018-11-05T21:09:41+01:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.namedfile@44bd9e5

Do not fail image upload when Exif data is bad.

plone/plone.namedfile#68

Files changed:
M plone/namedfile/file.py
Repository: plone.namedfile

Branch: refs/heads/master
Date: 2018-11-05T22:06:40+01:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.namedfile@6a202e9

Forgot to commit news file

Files changed:
A news/68.bugfix
Repository: plone.namedfile

Branch: refs/heads/master
Date: 2018-11-09T00:34:38+01:00
Author: Jens W. Klein (jensens) <jk@kleinundpartner.at>
Commit: plone/plone.namedfile@d29f01e

Merge pull request #69 from plone/issue-68-master

Do not fail image upload when Exif data is bad. [master]

Files changed:
A news/68.bugfix
M plone/namedfile/file.py
@mauritsvanrees
Copy link
Member Author

Fixed in 4.2.6 (Plone 5.1) and 5.0.2 (Plone 5.2).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants