Show more granular error messages from Pillow (image uploads)

* Prevent `invalid_image` from hiding Pillow errors, instead rename key to `invalid_image_extension`
* Django already uses `invalid_image` - https://github.com/django/django/blob/stable/3.0.x/django/forms/fields.py#L639
* Add tests separate for images with invalid extensions vs non-images

CHANGELOG.txt

@@ -16,6 +16,7 @@ Changelog
  * Upgrade internal JS tooling to Gulp v4 & Node v10 (Jim Jazwiecki, Kim LaRocca)
  * Add `after_publish_page`, `before_publish_page`, `after_unpublish_page` & `before_unpublish_page` hooks (Jonatas Baldin, Coen van der Kamp)
  * Add convenience `page_url` shortcut to improve how page URLs can be accessed from site settings in Django templates (Andy Babic)
+ * Show more granular error messages from Pillow when uploading images (Rick van Hattem)
  * Fix: Support IPv6 domain (Alex Gleason, Coen van der Kamp)
  * Fix: Ensure link to add a new user works when no users are visible in the users list (LB (Ben Johnston))
  * Fix: `AbstractEmailForm` saved submission fields are now aligned with the email content fields, `form.cleaned_data` will be used instead of `form.fields` (Haydn Greatnews)

CONTRIBUTORS.rst

@@ -455,6 +455,7 @@ Contributors
 * Jim Jazwiecki
 * Kim LaRocca
 * Jonatas Baldin
+* Rick van Hattem
 
 Translators
 ===========

docs/releases/2.10.rst

@@ -25,6 +25,7 @@ Other features
  * Upgrade internal JS tooling to Gulp v4 & Node v10 (Jim Jazwiecki, Kim LaRocca)
  * Add ``after_publish_page``, ``before_publish_page``, ``after_unpublish_page`` & ``before_unpublish_page`` hooks (Jonatas Baldin, Coen van der Kamp)
  * Add convenience ``page_url`` shortcut to improve how page URLs can be accessed from site settings in Django templates (Andy Babic)
+ * Show more granular error messages from Pillow when uploading images (Rick van Hattem)
 
 
 Bug fixes

wagtail/images/fields.py

@@ -36,7 +36,7 @@ class WagtailImageField(ImageField):
             }
 
         # Error messages
-        self.error_messages['invalid_image'] = _(
+        self.error_messages['invalid_image_extension'] = _(
             "Not a supported image format. Supported formats: %s."
         ) % SUPPORTED_FORMATS_TEXT
 
@@ -61,7 +61,7 @@ class WagtailImageField(ImageField):
         extension = os.path.splitext(f.name)[1].lower()[1:]
 
         if extension not in ALLOWED_EXTENSIONS:
-            raise ValidationError(self.error_messages['invalid_image'], code='invalid_image')
+            raise ValidationError(self.error_messages['invalid_image_extension'], code='invalid_image_extension')
 
         image_format = extension.upper()
         if image_format == 'JPG':

wagtail/images/tests/test_admin_views.py

@@ -933,7 +933,26 @@ class TestImageChooserUploadView(TestCase, WagtailTestUtils):
 
         self.assertEqual(response.status_code, 200)
         self.assertTemplateUsed(response, 'wagtailimages/chooser/chooser.html')
-        self.assertFormError(response, 'uploadform', 'file', "Not a supported image format. Supported formats: GIF, JPEG, PNG, WEBP.")
+        self.assertFormError(response, 'uploadform', 'file', 'Upload a valid image. The file you uploaded was either not an image or a corrupted image.')
+
+        # the action URL of the re-rendered form should include the select_format=true parameter
+        # (NB the HTML in the response is embedded in a JS string, so need to escape accordingly)
+        expected_action_attr = 'action=\\"%s\\"' % submit_url
+        self.assertContains(response, expected_action_attr)
+
+    def test_select_format_flag_after_upload_form_error_bad_extension(self):
+        """
+        Check the error message is accruate for a valid imate bug invalid file extension.
+        """
+        submit_url = reverse('wagtailimages:chooser_upload') + '?select_format=true'
+        response = self.client.post(submit_url, {
+            'image-chooser-upload-title': "accidental markdown extension",
+            'image-chooser-upload-file': SimpleUploadedFile('not-an-image.md', get_test_image_file().file.getvalue()),
+        })
+
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailimages/chooser/chooser.html')
+        self.assertFormError(response, 'uploadform', 'file', 'Not a supported image format. Supported formats: GIF, JPEG, PNG, WEBP.')
 
         # the action URL of the re-rendered form should include the select_format=true parameter
         # (NB the HTML in the response is embedded in a JS string, so need to escape accordingly)
@@ -1130,7 +1149,30 @@ class TestMultipleImageUploader(TestCase, WagtailTestUtils):
         self.assertIn('error_message', response_json)
         self.assertFalse(response_json['success'])
         self.assertEqual(
-            response_json['error_message'], "Not a supported image format. Supported formats: GIF, JPEG, PNG, WEBP."
+            response_json['error_message'], 'Upload a valid image. The file you uploaded was either not an image or a corrupted image.'
+        )
+
+    def test_add_post_bad_extension(self):
+        """
+        The add view must check that the uploaded file extension is a valid
+        """
+        response = self.client.post(reverse('wagtailimages:add_multiple'), {
+            'files[]': SimpleUploadedFile('test.txt', get_test_image_file().file.getvalue()),
+        }, HTTP_X_REQUESTED_WITH='XMLHttpRequest')
+
+        # Check response
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response['Content-Type'], 'application/json')
+
+        # Check JSON
+        response_json = json.loads(response.content.decode())
+        self.assertNotIn('image_id', response_json)
+        self.assertNotIn('form', response_json)
+        self.assertIn('success', response_json)
+        self.assertIn('error_message', response_json)
+        self.assertFalse(response_json['success'])
+        self.assertEqual(
+            response_json['error_message'], 'Not a supported image format. Supported formats: GIF, JPEG, PNG, WEBP.'
         )
 
     def test_edit_get(self):
@@ -1336,7 +1378,7 @@ class TestMultipleImageUploaderWithCustomImageModel(TestCase, WagtailTestUtils):
         self.assertIn('error_message', response_json)
         self.assertFalse(response_json['success'])
         self.assertEqual(
-            response_json['error_message'], "Not a supported image format. Supported formats: GIF, JPEG, PNG, WEBP."
+            response_json['error_message'], 'Upload a valid image. The file you uploaded was either not an image or a corrupted image.'
         )
 
     def test_edit_post(self):
@@ -1482,7 +1524,7 @@ class TestMultipleImageUploaderWithCustomRequiredFields(TestCase, WagtailTestUti
         self.assertIn('error_message', response_json)
         self.assertFalse(response_json['success'])
         self.assertEqual(
-            response_json['error_message'], "Not a supported image format. Supported formats: GIF, JPEG, PNG, WEBP."
+            response_json['error_message'], "Upload a valid image. The file you uploaded was either not an image or a corrupted image."
         )
 
     def test_create_from_upload_invalid_post(self):

wagtail/images/views/multiple.py

@@ -127,7 +127,7 @@ def add(request):
             'help_text': form.fields['file'].help_text,
             'allowed_extensions': ALLOWED_EXTENSIONS,
             'error_max_file_size': form.fields['file'].error_messages['file_too_large_unknown_size'],
-            'error_accepted_file_types': form.fields['file'].error_messages['invalid_image'],
+            'error_accepted_file_types': form.fields['file'].error_messages['invalid_image_extension'],
             'collections': collections_to_choose,
             'form_media': form.media,
         })