#2745 - non-standard images will not break thumbnails

backend/src/baserow/core/user_files/handler.py

@@ -18,6 +18,7 @@ from django.utils.http import parse_header_parameters
 
 import advocate
 from advocate.exceptions import UnacceptableAddressException
+from loguru import logger
 from PIL import Image, ImageOps
 from requests.exceptions import RequestException
 
@@ -147,7 +148,11 @@ class UserFileHandler:
                 return unique
 
     def generate_and_save_image_thumbnails(
-        self, image, user_file, storage=None, only_with_name=None
+        self,
+        image: Image,
+        user_file_name: str,
+        storage: Storage | None = None,
+        only_with_name: str | None = None,
     ):
         """
         Generates the thumbnails based on the current settings and saves them to the
@@ -156,24 +161,16 @@ class UserFileHandler:
 
         :param image: The original Pillow image that serves as base when generating the
             image.
-        :type image: Image
-        :param user_file: The user file for which the thumbnails must be generated and
-            saved.
-        :type user_file: UserFile
+        :param user_file_name: The name of the user file that the thumbnail is for.
         :param storage: The storage where the thumbnails must be saved to.
-        :type storage: Storage or None
         :param only_with_name: If provided, then only thumbnail types with that name
             will be regenerated.
-        :type only_with_name: None or String
         :raises ValueError: If the provided user file is not a valid image.
         """
 
-        if not user_file.is_image:
-            raise ValueError("The provided user file is not an image.")
-
         storage = storage or get_default_storage()
-        image_width = user_file.image_width
-        image_height = user_file.image_height
+        image_width = image.width
+        image_height = image.height
 
         for name, size in settings.USER_THUMBNAILS.items():
             if only_with_name and only_with_name != name:
@@ -195,7 +192,7 @@ class UserFileHandler:
                 thumbnail_stream = BytesIO()
                 thumbnail.save(thumbnail_stream, image.format)
                 thumbnail_stream.seek(0)
-                thumbnail_path = self.user_file_thumbnail_path(user_file, name)
+                thumbnail_path = self.user_file_thumbnail_path(user_file_name, name)
 
                 handler = OverwritingStorageHandler(storage)
                 handler.save(thumbnail_path, thumbnail_stream)
@@ -252,25 +249,7 @@ class UserFileHandler:
             or MIME_TYPE_UNKNOWN
         )
         unique = self.generate_unique(stream_hash, extension)
-
-        # By default the provided file is not an image.
-        image = None
-        is_image = False
-        image_width = None
-        image_height = None
-
-        # Try to open the image with Pillow. If that succeeds we know the file is an
-        # image.
-        try:
-            image = Image.open(stream)
-            is_image = True
-            image_width = image.width
-            image_height = image.height
-            mime_type = f"image/{image.format}".lower()
-        except IOError:
-            pass
-
-        user_file = UserFile.objects.create(
+        user_file = UserFile(
             original_name=file_name,
             original_extension=extension,
             size=size,
@@ -278,20 +257,30 @@ class UserFileHandler:
             unique=unique,
             uploaded_by=user,
             sha256_hash=stream_hash,
-            is_image=is_image,
-            image_width=image_width,
-            image_height=image_height,
         )
 
-        # If the uploaded file is an image we need to generate the configurable
-        # thumbnails for it. We want to generate them before the file is saved to the
-        # storage because some storages close the stream after saving.
-        if image:
-            self.generate_and_save_image_thumbnails(image, user_file, storage=storage)
+        image = None
+        try:
+            image = Image.open(stream)
+            user_file.mime_type = f"image/{image.format}".lower()
+            self.generate_and_save_image_thumbnails(
+                image, user_file.name, storage=storage
+            )
+            # Skip marking as images if thumbnails cannot be generated (i.e. PSD files).
+            user_file.is_image = True
+            user_file.image_width = image.width
+            user_file.image_height = image.height
+        except IOError:
+            pass  # Not an image
+        except Exception as exc:
+            logger.warning(
+                f"Failed to generate thumbnails for user file of type {mime_type}: {exc}"
+            )
+        finally:
+            if image is not None:
+                del image
 
-            # When all the thumbnails have been generated, the image can be deleted
-            # from memory.
-            del image
+        user_file.save()
 
         # Save the file to the storage.
         full_path = self.user_file_path(user_file)

backend/tests/baserow/core/user_file/test_user_file_handler.py

@@ -262,6 +262,29 @@ def test_upload_user_file_with_truncated_image(data_fixture, tmpdir):
     assert not file_path.isfile()
 
 
+@pytest.mark.django_db
+def test_upload_user_file_with_unsupported_image_format(
+    data_fixture, tmpdir, open_test_file
+):
+    user = data_fixture.create_user()
+
+    storage = FileSystemStorage(location=str(tmpdir), base_url="http://localhost")
+    handler = UserFileHandler()
+
+    image_bytes = open_test_file("baserow/core/user_file/baserow.logo.psd")
+
+    user_file = handler.upload_user_file(
+        user, "truncated_image.psd", image_bytes, storage=storage
+    )
+    assert user_file.mime_type == "image/psd"
+    assert user_file.is_image is False
+    assert user_file.image_width is None
+    assert user_file.image_height is None
+
+    file_path = tmpdir.join("thumbnails", "tiny", user_file.name)
+    assert not file_path.isfile()
+
+
 @pytest.mark.django_db
 @httpretty.activate(verbose=True, allow_net_connect=False)
 def test_upload_user_file_by_url(data_fixture, tmpdir):

backend/tests/conftest.py

@@ -1,9 +1,14 @@
 from __future__ import print_function
 
+from io import IOBase
+from pathlib import Path
+
 from django.core.management import call_command
 from django.db import connections
 from django.test.testcases import TransactionTestCase
 
+import pytest
+
 # noinspection PyUnresolvedReferences
 from baserow.test_utils.pytest_conftest import *  # noqa: F403, F401
 
@@ -43,3 +48,39 @@ def _fixture_teardown(self):
 
 
 TransactionTestCase._fixture_teardown = _fixture_teardown
+
+
+@pytest.fixture()
+def test_data_dir() -> Path:
+    """
+    Returns root path for test data directory
+    """
+
+    return Path(__file__).parent.joinpath("test_data")
+
+
+@pytest.fixture()
+def open_test_file(test_data_dir):
+    """
+    Opens a test data file on a given path.
+
+    usage:
+
+    def test_me(open_test_file):
+        with open_test_file('baserow/core/test.data', 'rt') as f:
+            assert not f.closed
+
+    Note: the caller can treat this as a context manager factory.
+    """
+
+    fhandle: IOBase | None = None
+
+    def get_path(tpath, /, mode="rb") -> IOBase:
+        nonlocal fhandle
+        fhandle = (test_data_dir / (tpath)).open(mode=mode)
+
+        return fhandle
+
+    yield get_path
+    if fhandle and not fhandle.closed:
+        fhandle.close()

changelog/entries/unreleased/bug/2745_nonstandard_image_formats_dont_break_thumbnails.json

@@ -0,0 +1,7 @@
+{
+    "type": "bug",
+    "message": "Non-standard image formats don't break thumbnails",
+    "issue_number": 2745,
+    "bullet_points": [],
+    "created_at": "2024-12-18"
+}

web-frontend/modules/database/filePreviewTypes.js

@@ -49,7 +49,7 @@ export class ImageFilePreview extends FilePreviewType {
   }
 
   isCompatible(mimeType, fileName) {
-    return mimeType.startsWith('image/')
+    return mimeType.startsWith('image/') && mimeType !== 'image/psd'
   }
 
   getPreviewComponent() {