Skip to content

Commit

Permalink
Fixed #35139 -- Prevented file read after ImageField is saved to stor…
Browse files Browse the repository at this point in the history
…age.
  • Loading branch information
john-parton authored and sarahboyce committed May 21, 2024
1 parent 4971a9a commit 9c5fe93
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 2 deletions.
11 changes: 10 additions & 1 deletion django/db/models/fields/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,13 @@ def open(self, mode="rb"):
# to further manipulate the underlying file, as well as update the
# associated model instance.

def _set_instance_attribute(self, name, content):
setattr(self.instance, self.field.attname, name)

def save(self, name, content, save=True):
name = self.field.generate_filename(self.instance, name)
self.name = self.storage.save(name, content, max_length=self.field.max_length)
setattr(self.instance, self.field.attname, self.name)
self._set_instance_attribute(self.name, content)
self._committed = True

# Save the object because it has changed, unless save is False
Expand Down Expand Up @@ -391,6 +394,12 @@ def __set__(self, instance, value):


class ImageFieldFile(ImageFile, FieldFile):
def _set_instance_attribute(self, name, content):
setattr(self.instance, self.field.attname, content)
# Update the name in case generate_filename() or storage.save() changed
# it, but bypass the descriptor to avoid re-reading the file.
self.instance.__dict__[self.field.attname] = self.name

def delete(self, save=True):
# Clear the image dimensions cache
if hasattr(self, "_dimensions_cache"):
Expand Down
5 changes: 5 additions & 0 deletions docs/releases/5.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,11 @@ Miscellaneous
:class:`~django.core.exceptions.FieldError` when saving a file without a
``name``.

* ``ImageField.update_dimension_fields(force=True)`` is no longer called after
saving the image to storage. If your storage backend resizes images, the
``width_field`` and ``height_field`` will not match the width and height of
the image.

.. _deprecated-features-5.1:

Features deprecated in 5.1
Expand Down
17 changes: 17 additions & 0 deletions tests/model_fields/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
from django.utils.functional import SimpleLazyObject
from django.utils.translation import gettext_lazy as _

from .storage import NoReadFileSystemStorage

try:
from PIL import Image
except ImportError:
Expand Down Expand Up @@ -373,6 +375,21 @@ class PersonTwoImages(models.Model):
width_field="headshot_width",
)

class PersonNoReadImage(models.Model):
"""
Model that defines an ImageField with a storage backend that does not
support reading.
"""

mugshot = models.ImageField(
upload_to="tests",
storage=NoReadFileSystemStorage(),
width_field="mugshot_width",
height_field="mugshot_height",
)
mugshot_width = models.IntegerField()
mugshot_height = models.IntegerField()


class CustomJSONDecoder(json.JSONDecoder):
def __init__(self, object_hook=None, *args, **kwargs):
Expand Down
6 changes: 6 additions & 0 deletions tests/model_fields/storage.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from django.core.files.storage.filesystem import FileSystemStorage


class NoReadFileSystemStorage(FileSystemStorage):
def open(self, *args, **kwargs):
raise AssertionError("This storage class does not support reading.")
28 changes: 27 additions & 1 deletion tests/model_fields/test_imagefield.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from .models import (
Person,
PersonDimensionsFirst,
PersonNoReadImage,
PersonTwoImages,
PersonWithHeight,
PersonWithHeightAndWidth,
Expand All @@ -30,7 +31,7 @@ class Person:
pass

PersonWithHeight = PersonWithHeightAndWidth = PersonDimensionsFirst = Person
PersonTwoImages = Person
PersonTwoImages = PersonNoReadImage = Person


class ImageFieldTestMixin(SerializeMixin):
Expand Down Expand Up @@ -469,3 +470,28 @@ def test_dimensions(self):
# Dimensions were recalculated, and hence file should have opened.
self.assertIs(p.mugshot.was_opened, True)
self.assertIs(p.headshot.was_opened, True)


@skipIf(Image is None, "Pillow is required to test ImageField")
class NoReadTests(ImageFieldTestMixin, TestCase):
def test_width_height_correct_name_mangling_correct(self):
instance1 = PersonNoReadImage()

instance1.mugshot.save("mug", self.file1)

self.assertEqual(instance1.mugshot_width, 4)
self.assertEqual(instance1.mugshot_height, 8)

instance1.save()

self.assertEqual(instance1.mugshot_width, 4)
self.assertEqual(instance1.mugshot_height, 8)

instance2 = PersonNoReadImage()
instance2.mugshot.save("mug", self.file1)
instance2.save()

self.assertNotEqual(instance1.mugshot.name, instance2.mugshot.name)

self.assertEqual(instance1.mugshot_width, instance2.mugshot_width)
self.assertEqual(instance1.mugshot_height, instance2.mugshot_height)

0 comments on commit 9c5fe93

Please sign in to comment.