frappe icon indicating copy to clipboard operation
frappe copied to clipboard

optimize_image in frappe/utils/image.py fails with unusual image types

Open gbm001 opened this issue 1 year ago • 8 comments

https://github.com/frappe/frappe/blob/3bea50d519b03b8003b56859b8c6846482c1742d/frappe/utils/image.py#L56

If you try and upload an image of an unusual type (i.e. identified as an image by mimetype guessing e.g. a Gimp .xcf file), optimize_image in frappe/utils/image.py will fail, giving an unidentified 500 error. Specifically, on line 56 the image is passed to PIL to open as an image, and if PIL doesn't handle that image (like a .xcf file) it will throw a PIL.UnidentifiedImageError

Solution: Catch this error, and throw a frappe error 'Bad image file' or similar. Alternatively, just don't optimize the image file and return the raw content.

gbm001 avatar Jan 29 '24 14:01 gbm001

Could you share an example image file that caused the issue?

akhilnarang avatar Jan 29 '24 19:01 akhilnarang

test.zip In a ZIP archive as Github doesn't like you opening .xcf (Gimp) files.

I don't think Frappe should actually be handling this file as an image file, but it also shouldn't fail when you try and upload it.

gbm001 avatar Feb 05 '24 08:02 gbm001

Unable to test locally since the mimetype isn't detected for me, will check if I have some missing packages

In the meanwhile could you share the traceback @gbm001, there is already exception handling code there: https://github.com/frappe/frappe/pull/23873

akhilnarang avatar Feb 05 '24 12:02 akhilnarang

Managed to get the mimetype detected, still unable to reproduce your exact scenario.


image


https://github.com/frappe/frappe/blob/develop/frappe/public/js/frappe/file_uploader/FileUploader.vue#L459

akhilnarang avatar Feb 12 '24 04:02 akhilnarang

Traceback:

14:39:06 web.1            | Traceback (most recent call last):
14:39:06 web.1            |   File "apps/frappe/frappe/app.py", line 97, in application
14:39:06 web.1            |     response = frappe.api.handle()
14:39:06 web.1            |                ^^^^^^^^^^^^^^^^^^^
14:39:06 web.1            |   File "apps/frappe/frappe/api.py", line 55, in handle
14:39:06 web.1            |     return frappe.handler.handle()
14:39:06 web.1            |            ^^^^^^^^^^^^^^^^^^^^^^^
14:39:06 web.1            |   File "apps/frappe/frappe/handler.py", line 47, in handle
14:39:06 web.1            |     data = execute_cmd(cmd)
14:39:06 web.1            |            ^^^^^^^^^^^^^^^^
14:39:06 web.1            |   File "apps/frappe/frappe/handler.py", line 85, in execute_cmd
14:39:06 web.1            |     return frappe.call(method, **frappe.form_dict)
14:39:06 web.1            |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14:39:06 web.1            |   File "apps/frappe/frappe/__init__.py", line 1603, in call
14:39:06 web.1            |     return fn(*args, **newargs)
14:39:06 web.1            |            ^^^^^^^^^^^^^^^^^^^^
14:39:06 web.1            |   File "apps/frappe/frappe/handler.py", line 217, in upload_file
14:39:06 web.1            |     content = optimize_image(**args)
14:39:06 web.1            |               ^^^^^^^^^^^^^^^^^^^^^^
14:39:06 web.1            |   File "apps/frappe/frappe/utils/image.py", line 52, in optimize_image
14:39:06 web.1            |     image = Image.open(io.BytesIO(content))
14:39:06 web.1            |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14:39:06 web.1            |   File "env/lib/python3.11/site-packages/PIL/Image.py", line 3280, in open
14:39:06 web.1            |     raise UnidentifiedImageError(msg)
14:39:06 web.1            | PIL.UnidentifiedImageError: cannot identify image file <_io.BytesIO object at 0x7f97b41d80e0>

gbm001 avatar Feb 13 '24 14:02 gbm001

I am on V14 (did a bench update yesterday) just trying to attach the file to a random Item document (plain attachment; not adding as a file). If you are trying to add the file as an image somewhere then the image field might have the additional file type restrictions on it?

gbm001 avatar Feb 13 '24 14:02 gbm001

Actually looks like the proposed fixes should have fixed it in the newer versions, but the backport to V14 failed?

https://github.com/frappe/frappe/pull/23875 https://github.com/frappe/frappe/pull/23875/commits/b112333f32eeb1f5652b5f7f7c1772a3b7033847

So fix is (sort of) already there; just needs the backport into V14 redoing?

gbm001 avatar Feb 13 '24 14:02 gbm001

Ah v14, will check and fix the backport, thanks.

akhilnarang avatar Feb 13 '24 14:02 akhilnarang

The patch is included in the latest v14 release: 14.66.2

akhilnarang avatar Feb 21 '24 06:02 akhilnarang