Merge pull request #51 from Lucas-C/hardening

Author: Lucas-C

RELEASE.md

@@ -0,0 +1,4 @@
+Release type: patch
+
+*bug*: handling edge cases where image is location is non-local to the Pelican site source,
+does not exist, or cannot be identified by Pillow.

pelican/plugins/image_process/image_process.py

@@ -17,7 +17,7 @@
 import subprocess
 import sys
 
-from PIL import Image, ImageFilter
+from PIL import Image, ImageFilter, UnidentifiedImageError
 from bs4 import BeautifulSoup
 import six
 from six.moves.urllib_parse import unquote, urlparse
@@ -46,8 +46,9 @@ class ExifTool(object):
     def start_exiftool():
         if shutil.which("exiftool") is None:
             log.warning(
-                "EXIF tags will not be copied because the exiftool program could not "
-                "be found. Please install exiftool and make sure it is in your path."
+                "[image_process] EXIF tags will not be copied because "
+                "the exiftool program could not be found. "
+                "Please install exiftool and make sure it is in your path."
             )
         else:
             ExifTool._instance = ExifTool()
@@ -110,7 +111,9 @@ def _send_command(self, params):
             output += os.read(fd, ExifTool.block_size)
         exiftool_result = output.strip()[: -len(ExifTool.sentinel)]
         log.debug(
-            "image_process: exiftool result: {}".format(exiftool_result.decode("utf-8"))
+            "[image_process] exiftool result: {}".format(
+                exiftool_result.decode("utf-8")
+            )
         )
 
 
@@ -260,7 +263,7 @@ def apply_filter(i, f):
 
 
 def harvest_images(path, context):
-    log.debug("process_images: harvest %r", path)
+    log.debug("[image_process] harvesting %r", path)
     # Set default value for 'IMAGE_PROCESS_DIR'.
     if "IMAGE_PROCESS_DIR" not in context:
         context["IMAGE_PROCESS_DIR"] = "derivatives"
@@ -412,6 +415,12 @@ def compute_paths(img, settings, derivative):
 
 def process_img_tag(img, settings, derivative):
     path = compute_paths(img, settings, derivative)
+    if not is_img_identifiable(path.source):
+        log.warn(
+            "[image_process] Skipping image %s that could not be identified by Pillow",
+            path.source,
+        )
+        return
     process = settings["IMAGE_PROCESS"][derivative]
 
     img["src"] = posixpath.join(path.base_url, path.filename)
@@ -423,16 +432,30 @@ def process_img_tag(img, settings, derivative):
     process_image((path.source, destination, process), settings)
 
 
+def is_img_identifiable(img_filepath):
+    try:
+        Image.open(img_filepath)
+        return True
+    except (FileNotFoundError, UnidentifiedImageError):
+        return False
+
+
 def build_srcset(img, settings, derivative):
     path = compute_paths(img, settings, derivative)
+    if not is_img_identifiable(path.source):
+        log.warn(
+            "[image_process] Skipping image %s that could not be identified by Pillow",
+            path.source,
+        )
+        return
     process = settings["IMAGE_PROCESS"][derivative]
 
     default = process["default"]
     if isinstance(default, six.string_types):
         breakpoints = {i for i, _ in process["srcset"]}
         if default not in breakpoints:
             log.error(
-                'image_process: srcset "%s" does not define default "%s"',
+                '[image_process] srcset "%s" does not define default "%s"',
                 derivative,
                 default,
             )
@@ -672,7 +695,7 @@ def process_image(image, settings):
     image[1] = unquote(image[1])
     # image[2] is the transformation
 
-    log.debug("image_process: {} -> {}".format(image[0], image[1]))
+    log.debug("[image_process] {} -> {}".format(image[0], image[1]))
 
     os.makedirs(os.path.dirname(image[1]), exist_ok=True)
 

pelican/plugins/image_process/test_data/folded_puzzle.png

@@ -10,7 +10,9 @@
 
 from pelican.plugins.image_process import (
     ExifTool,
+    compute_paths,
     harvest_images_in_fragment,
+    is_img_identifiable,
     process_image,
 )
 
@@ -87,6 +89,7 @@ def test_all_transforms(tmp_path, transform_id, transform_params, image_path):
 
     transformed = Image.open(destination_path)
     expected = Image.open(expected_path)
+    # Image.getbbox() returns None if there are only black pixels in the image:
     image_diff = ImageChops.difference(transformed, expected).getbbox()
     assert image_diff is None
 
@@ -115,6 +118,11 @@ def test_all_transforms(tmp_path, transform_id, transform_params, image_path):
     ],
 )
 def test_path_normalization(mocker, orig_src, orig_img, new_src, new_img):
+    # Allow non-existing images to be processed:
+    mocker.patch(
+        "pelican.plugins.image_process.image_process.is_img_identifiable",
+        lambda img_filepath: True,
+    )
     # Silence image transforms.
     process = mocker.patch("pelican.plugins.image_process.image_process.process_image")
 
@@ -431,6 +439,11 @@ def test_path_normalization(mocker, orig_src, orig_img, new_src, new_img):
     ],
 )
 def test_picture_generation(mocker, orig_tag, new_tag, call_args):
+    # Allow non-existing images to be processed:
+    mocker.patch(
+        "pelican.plugins.image_process.image_process.is_img_identifiable",
+        lambda img_filepath: True,
+    )
     process = mocker.patch("pelican.plugins.image_process.image_process.process_image")
 
     settings = get_settings(
@@ -543,3 +556,18 @@ def test_copy_exif_tags(tmp_path, image_path, copy_tags):
             assert expected_tags[tag] == actual_tags[tag]
         else:
             assert tag not in actual_tags.keys()
+
+
+def test_is_img_identifiable():
+    for test_image in TEST_IMAGES:
+        assert is_img_identifiable(test_image)
+
+    assert not is_img_identifiable("image/that/does/not/exist.png")
+
+    assert not is_img_identifiable(TEST_DATA.joinpath("folded_puzzle.png"))
+    assert not is_img_identifiable(TEST_DATA.joinpath("minimal.svg"))
+
+    img = {"src": "https://upload.wikimedia.org/wikipedia/commons/3/34/Exemple.png"}
+    settings = get_settings(IMAGE_PROCESS_DIR="derivatives")
+    path = compute_paths(img, settings, derivative="thumb")
+    assert not is_img_identifiable(path.source)