Address agent review: format validation, AVIF, animated WebP, cleanup

- Drop 'or PNG' fallback: img.format is always set after a successful
  PillowImage.open() call; UnidentifiedImageError fires before returning None.
- Add extension/content consistency check using PillowImage.MIME to catch
  mismatches like PNG data with a .jpg extension (content-type drift).
- AVIF/JPEG2000: raise a specific ValidationError when save() fails due to
  a missing native codec library, replacing the generic 'unable to process'
  message that was a silent regression from the previous store-as-is behaviour.
- Animated WebP: extend the ImageSequence frame-preservation path to WebP
  (n_frames > 1) alongside GIF.
- Hoist ImageSequence import to module level; split PIL imports onto separate
  lines per ruff isort requirement.
- Tests: use img.n_frames instead of manual seek loop; add animated WebP test;
  add comment explaining why test_clean_rejects_disallowed_extension calls
  clean() directly rather than full_clean().

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Brian Tiemann 2026-06-09 19:45:57 -04:00
parent 0227be7ebe
commit 7f676ea004
2 changed files with 50 additions and 21 deletions

View File

@ -15,6 +15,7 @@ from django.utils.html import escape
from django.utils.safestring import mark_safe
from django.utils.translation import gettext_lazy as _
from PIL import Image as PillowImage
from PIL import ImageSequence
from rest_framework.utils.encoders import JSONEncoder
from extras.choices import *
@ -759,25 +760,42 @@ class ImageAttachment(ChangeLoggedModel):
)
self.image.file.seek(0)
img = PillowImage.open(self.image.file)
img_format = img.format or 'PNG'
# Re-encode into a fresh buffer; any non-image trailer bytes
# are discarded. For animated GIFs, iterate through all frames
# via ImageSequence so animation is preserved.
img_format = img.format
# Validate that the detected format matches the declared extension to
# prevent silent content-type drift (e.g. PNG data with a .jpg extension).
detected_mime = PillowImage.MIME.get(img_format, '').lower()
expected_mime = IMAGE_ATTACHMENT_IMAGE_FORMATS.get(ext, '').lower()
if detected_mime and expected_mime and detected_mime != expected_mime:
raise ValidationError(
_("File content does not match the declared extension (.{ext}).").format(ext=ext)
)
# Re-encode into a fresh buffer; any non-image trailer bytes are discarded.
# Animated formats (GIF, WebP) use ImageSequence to preserve all frames.
buf = io.BytesIO()
if img_format == 'GIF':
from PIL import ImageSequence # noqa: PLC0415
if img_format in ('GIF', 'WEBP') and getattr(img, 'n_frames', 1) > 1:
frames = [frame.copy() for frame in ImageSequence.Iterator(img)]
if frames:
frames[0].save(
buf, format='GIF', save_all=True,
buf, format=img_format, save_all=True,
append_images=frames[1:] if len(frames) > 1 else [],
duration=img.info.get('duration', 100),
loop=img.info.get('loop', 0),
)
else:
img.save(buf, format='GIF')
img.save(buf, format=img_format)
else:
img.save(buf, format=img_format)
try:
img.save(buf, format=img_format)
except OSError as e:
# AVIF and JPEG 2000 require optional native codec libraries.
# Surface a specific message instead of the generic fallback.
if img_format in ('AVIF', 'JPEG2000'):
raise ValidationError(
_("{fmt} images require additional codec libraries that are not installed.").format(
fmt=img_format
)
) from e
raise
# Replace the in-memory upload with the sanitised content so
# Django writes the clean version to storage on model save.
self.image.file = ContentFile(buf.getvalue(), name=self.image.name)

View File

@ -235,6 +235,13 @@ class ImageAttachmentTestCase(TestCase):
img = Image.open(buf)
self.assertEqual(img.format, 'PNG')
def _make_animated_webp(self, filename, frames=2):
"""Return a SimpleUploadedFile containing a minimal animated WebP."""
buf = io.BytesIO()
images = [Image.new('RGB', (2, 2), color=(i * 80, 0, 0)) for i in range(frames)]
images[0].save(buf, format='WEBP', save_all=True, append_images=images[1:], loop=0, duration=100)
return SimpleUploadedFile(name=filename, content=buf.getvalue(), content_type='image/webp')
def test_clean_preserves_animated_gif_frames(self):
"""clean() re-encodes GIF uploads while keeping all animation frames."""
n_frames = 3
@ -242,21 +249,25 @@ class ImageAttachmentTestCase(TestCase):
ia.full_clean()
sanitised = ia.image.file.read()
buf = io.BytesIO(sanitised)
img = Image.open(buf)
img = Image.open(io.BytesIO(sanitised))
self.assertEqual(img.format, 'GIF')
# All frames must be preserved
frame_count = 0
try:
while True:
frame_count += 1
img.seek(img.tell() + 1)
except EOFError:
pass
self.assertEqual(frame_count, n_frames)
self.assertEqual(img.n_frames, n_frames)
def test_clean_preserves_animated_webp_frames(self):
"""clean() re-encodes animated WebP uploads while keeping all animation frames."""
n_frames = 3
ia = self._make_attachment(self._make_animated_webp('anim.webp', frames=n_frames))
ia.full_clean()
sanitised = ia.image.file.read()
img = Image.open(io.BytesIO(sanitised))
self.assertEqual(img.format, 'WEBP')
self.assertEqual(img.n_frames, n_frames)
def test_clean_rejects_disallowed_extension(self):
"""clean() raises ValidationError for file extensions not in IMAGE_ATTACHMENT_IMAGE_FORMATS."""
"""clean() raises ValidationError for file extensions not in IMAGE_ATTACHMENT_IMAGE_FORMATS.
Note: clean() is called directly (not full_clean()) for focused unit testing of this
specific validation path without triggering unrelated field-level validators."""
svg_content = b'<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>'
ia = self._make_attachment(
SimpleUploadedFile(name='evil.svg', content=svg_content, content_type='image/svg+xml')