Fix exception of Pillow for SVGs

Review Request #10851 — Created Jan. 24, 2020 and updated

misery
Djblets
release-1.0.x
4687
67970d0...
djblets
If someone uploads a SVG as file attachment
Review Boards tries to generate a thumbnail.

Pillow do not support SVGs and throws an exception.

https://github.com/python-pillow/Pillow/issues/3509

 - djblets.util.templatetags.djblets_images - Error thumbnailing image file uploaded/files/2020/01/24/d6a12e49-05b7-42d9-bd6c-50985fd98f8e__check.svg and saving as uploaded/files/2020/01/24/d6a12e49-05b7-42d9-bd6c-50985fd98f8e__check_600.svg: cannot identify image file <cStringIO.StringI object at 0x7ff6a9b57b58>
Traceback (most recent call last):
  File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/util/templatetags/djblets_images.py", line 126, in thumbnail
    image = Image.open(data)
  File "/opt/reviewboard/dist/lib/python2.7/site-packages/PIL/Image.py", line 2705, in open
    % (filename if filename else fp))
IOError: cannot identify image file <cStringIO.StringI object at 0x7ff6a9b57b58>

As SVG do not need thumbnail because Browsers can
scale that without a problem we can provide
the original file here.

Added an SVG file and saw that the thumbnail exists
instead of an error 404.

Description From Last Updated

Can you add unit tests for this in djblets/util/tests/test_djblets_image_tags.py?

chipx86chipx86

E111 indentation is not a multiple of four

reviewbotreviewbot

This should probably be filename.lower().endswith('.svg')

daviddavid

This should be updated to say that SVGs are returned directly without generating a new image in storage.

chipx86chipx86

This is happening probably too late. We're doing a lot of computation of sizes and filenames, we're checking for a ...

chipx86chipx86
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

misery
david
  1. 
      
  2. This should probably be filename.lower().endswith('.svg')

  3. 
      
misery
Review request changed

Commit:

-a7fe460e2288ca87c02ec3f5ba1cc9af21b63f05
+67970d04668e7a0cc1c207d2167938db00d9ec5e

Diff:

Revision 3 (+3)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
chipx86
  1. 
      
  2. Can you add unit tests for this in djblets/util/tests/test_djblets_image_tags.py?

  3. djblets/util/templatetags/djblets_images.py (Diff revision 3)
     
     
     
     
     
     
     
     
     
     

    This should be updated to say that SVGs are returned directly without generating a new image in storage.

  4. djblets/util/templatetags/djblets_images.py (Diff revision 3)
     
     
     

    This is happening probably too late. We're doing a lot of computation of sizes and filenames, we're checking for a thumbnail variant from storage (which will never exist for the svg), and then we're doing this. I think we'd instead want to perform this check first (along with handling any failures/non-existent images).

  5. 
      
Loading...