Images: handle all not found errors in get_file_size

CHANGELOG.txt

@@ -6,6 +6,7 @@ Changelog
 
  * Added another valid AudioBoom oEmbed pattern (Bertrand Bordage)
  * Added `annotate_score` support to PostgreSQL search backend (Bertrand Bordage)
+ * Fix: Handle all exceptions from `Image.get_file_size` (Andrew Plummer)
 
 
 2.1 (22.05.2018)

CONTRIBUTORS.rst

@@ -300,6 +300,7 @@ Contributors
 * Pierre Geier
 * Jérôme Lebleu
 * Victor Miti
+* Andrew Plummer
 
 Translators
 ===========

docs/releases/2.2.rst

@@ -19,5 +19,7 @@ Other features
 Bug fixes
 ~~~~~~~~~
 
+ * Handle all exceptions from ``Image.get_file_size`` (Andrew Plummer)
+
 Upgrade considerations
 ======================

wagtail/images/models.py

@@ -98,9 +98,13 @@ class AbstractImage(CollectionMember, index.Indexed, models.Model):
         if self.file_size is None:
             try:
                 self.file_size = self.file.size
-            except OSError:
-                # File doesn't exist
-                return
+            except Exception as e:
+                # File not found
+                #
+                # Have to catch everything, because the exception
+                # depends on the file subclass, and therefore the
+                # storage being used.
+                raise SourceImageIOError(str(e))
 
             self.save(update_fields=['file_size'])
 

wagtail/images/tests/test_admin_views.py

@@ -373,6 +373,24 @@ class TestImageEditView(TestCase, WagtailTestUtils):
         self.assertEqual(response.status_code, 200)
         self.assertTemplateUsed(response, 'wagtailimages/images/edit.html')
 
+    def check_get_missing_file_displays_warning(self):
+        # Need to recreate image to use a custom storage per test.
+        image = Image.objects.create(title="Test image", file=get_test_image_file())
+        image.file.storage.delete(image.file.name)
+
+        response = self.client.get(reverse('wagtailimages:edit', args=(image.pk,)))
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailimages/images/edit.html')
+        self.assertContains(response, "File not found")
+
+    def test_get_missing_file_displays_warning_with_default_storage(self):
+        self.check_get_missing_file_displays_warning()
+
+    @override_settings(DEFAULT_FILE_STORAGE='wagtail.tests.dummy_external_storage.DummyExternalStorage')
+    def test_get_missing_file_displays_warning_with_custom_storage(self):
+        self.check_get_missing_file_displays_warning()
+
+
     def get_content(self, f=None):
         if f is None:
             f = self.image.file

wagtail/images/tests/test_models.py

@@ -85,6 +85,16 @@ class TestImage(TestCase):
     def test_is_stored_locally_with_external_storage(self):
         self.assertFalse(self.image.is_stored_locally())
 
+    def test_get_file_size(self):
+        file_size = self.image.get_file_size()
+        self.assertIsInstance(file_size, int)
+        self.assertGreater(file_size, 0)
+
+    def test_get_file_size_on_missing_file_raises_sourceimageioerror(self):
+        self.image.file.delete(save=False)
+        with self.assertRaises(SourceImageIOError):
+            self.image.get_file_size()
+
 
 class TestImageQuerySet(TestCase):
     def test_search_method(self):

wagtail/images/views/images.py

@@ -14,7 +14,7 @@ from wagtail.core.models import Collection, Site
 from wagtail.images import get_image_model
 from wagtail.images.exceptions import InvalidFilterSpecError
 from wagtail.images.forms import URLGeneratorForm, get_image_form
-from wagtail.images.models import Filter
+from wagtail.images.models import Filter, SourceImageIOError
 from wagtail.images.permissions import permission_policy
 from wagtail.images.views.serve import generate_signature
 from wagtail.search import index as search_index
@@ -138,11 +138,16 @@ def edit(request, image_id):
                 messages.button(reverse('wagtailimages:delete', args=(image.id,)), _('Delete'))
             ])
 
+    try:
+        filesize = image.get_file_size()
+    except SourceImageIOError:
+        filesize = None
+
     return render(request, "wagtailimages/images/edit.html", {
         'image': image,
         'form': form,
         'url_generator_enabled': url_generator_enabled,
-        'filesize': image.get_file_size(),
+        'filesize': filesize,
         'user_can_delete': permission_policy.user_has_permission_for_instance(
             request.user, 'delete', image
         ),

wagtail/tests/dummy_external_storage.py

@@ -4,6 +4,7 @@
 # The following behaviours have been added to this backend:
 #  - Calling .path on the storage or image file raises NotImplementedError
 #  - File.open() after the file has been closed raises an error
+#  - File.size exceptions raise DummyExternalStorageError
 
 from django.core.files.base import File
 from django.core.files.storage import FileSystemStorage, Storage
@@ -56,6 +57,10 @@ class DummyExternalStorage(Storage):
         return self.wrapped.modified_time(name)
 
 
+class DummyExternalStorageError(Exception):
+    pass
+
+
 class DummyExternalStorageFile(File):
     def open(self, mode=None):
         # Based on:
@@ -71,3 +76,9 @@ class DummyExternalStorageFile(File):
         #    self.file = open(self.name, mode or self.mode)
         else:
             raise ValueError("The file cannot be reopened.")
+
+    def size(self):
+        try:
+            return super().size
+        except Exception as e:
+            raise DummyExternalStorageError(str(e))