matthewwithanm / django-imagekit

Automated image processing for Django. Currently v4.0
http://django-imagekit.rtfd.org/
BSD 3-Clause "New" or "Revised" License
2.27k stars 275 forks source link

If the file is not a image file or missing, the admin page crashes #254

Open nishimu opened 11 years ago

nishimu commented 11 years ago

The admin page for the model which has the ImageField crashes before rendering template if one of the files is not a image file(for some reason, a zero-size file) or missing.

matthewwithanm commented 10 years ago

@nishimu Sorry for the delay in responding. Can you share the full error and the version of Django you're using? I'm not seeing this.

tino commented 9 years ago

I have the same problem, but not only in the admin. The problem is that exceptions in generate are not caught (for example MissingSource: The spec '<imagekit.specs.DynamicSpec object at 0x7f26180aa320>' has no source file associated with it.). Therefore accessing an url in a template can cause a 500. The generateimages does catch those exceptions (https://github.com/matthewwithanm/django-imagekit/blob/master/imagekit/management/commands/generateimages.py#L31).

I am not sure where the exception should be caught, as the generation happens with signals?

matthewwithanm commented 9 years ago

In general, trying to access a URL for an image that doesn't have a source should throw an error. This is based on the behavior of Django itself—if you have an empty ImageField and try to access the URL, you get an error. I think if this didn't happen, you'd wind up with a bunch of surprisingly empty img src attributes. The usual way to avoid this would be to check if the image exists first:

{% if model.image %}
<img src="{{ model.image.url }}" />
{% endif %}

There are also some places where it makes sense to trap this error. For example, the admin template should never cause an error, and probably also the generateimage template tag.

@tino Can you show the code that's causing the error? Which category does it fall under?

tino commented 9 years ago

Ah, I guess I will have to check for image existence in the template then. I was thinking that accessing a template variable should never blow up in an error, but I guess if Django's own ImageField does this...

On Wed, May 6, 2015 at 4:03 PM, Matthew Dapena-Tretter < notifications@github.com> wrote:

In general, trying to access a URL for an image that doesn't have a source should throw an error. This is based on the behavior of Django itself—if you have an empty ImageField and try to access the URL, you get an error. I think if this didn't happen, you'd wind up with a bunch of surprisingly empty img src attributes. The usual way to avoid this would be to check if the image exists first:

{% if model.image %}

{% endif %}

There are also some places where it makes sense to trap this error. For example, the admin template should never cause an error, and probably also the generateimage template tag.

@tino https://github.com/tino Can you show the code that's causing the error? Which category does it fall under?

— Reply to this email directly or view it on GitHub https://github.com/matthewwithanm/django-imagekit/issues/254#issuecomment-99476101 .

matthewwithanm commented 9 years ago

Yeah, this is a common issue. It makes sense when you think about it, though: you can pass any variables into the template so the only options for suppressing errors are to make different APIs to be used in the template (which is icky) or catch them in the template code (which Django doesn't do).

ceefour commented 6 years ago

Is this still an issue? It seems related to #339 , but this pertains to "admin" so if this is no longer an issue then should be closed.

vstoykov commented 6 years ago

I'm not sure if it is an issue with ImageKit at all. There are certain conditions in with Django itself will blow up if there is a problem with the file (at least in DEBUG, in production sometimes the same error is ignored).

The issue is very old and there is no response from the original author. About the relation with #339 they are different. In the other issue the source file is not set (it's None). In this case the source file is set but there are problems with the content of the file:

In normal operation of django this should never happens. If it will happens then there is some problem with the logic of the application: source file is not an ImageField, some background process is messing with images and so on. This should not be silenced and application developer should be notified but this is specific to the application implementation and not generic for all use cases of ImageKit.

I understand that problem in a template should not bring the site down and this is bad user experience (even if it is in the administration). For that reason I will be really happy if someone came up with some idea, hopefully with a patch (pull request) with fix and some tests, in order to improve the situation with exception safety in the template.

ayushin commented 5 years ago

Can't believe this is still not resolved! Can we discuss and fix it?

ghost commented 4 years ago

Having the same problem. If the image field of my user references an image that is missing and I try to use ImageKit to create a thumbnail from it it will throw an error.

liuxue0905 commented 4 years ago
File "XXXXXXXXX/site-packages/django/db/models/fields/files.py", line 449, in update_dimension_fields
    width = file.width
  File "XXXXXXXXX/site-packages/django/core/files/images.py", line 19, in width
    return self._get_image_dimensions()[0]
  File "XXXXXXXXX/site-packages/django/core/files/images.py", line 28, in _get_image_dimensions
    self.open()
  File "XXXXXXXXX/django/db/models/fields/files.py", line 74, in open
    self.file = self.storage.open(self.name, mode)
  File "XXXXXXXXX/site-packages/django/core/files/storage.py", line 36, in open
    return self._open(name, mode)
  File "XXXXXXXXX/site-packages/django/core/files/storage.py", line 231, in _open
    return File(open(self.path(name), mode))
FileNotFoundError: [Errno 2] No such file or directory: 'XXXXXXXXX'

"django/db/models/fields/files.py"

class ImageField(FileField):
    .........
    def update_dimension_fields(self, instance, force=False, *args, **kwargs):
        .........
        # file should be an instance of ImageFieldFile or should be None.
        if file:
            width = file.width
            height = file.height
        else:
            # No file, so clear dimensions fields.
            width = None
            height = None
        .........
    .........

"django/core/files/images.py"

class ImageFile(File):
    .........
    @property
    def width(self):
        return self._get_image_dimensions()[0]

    @property
    def height(self):
        return self._get_image_dimensions()[1]

    def _get_image_dimensions(self):
        if not hasattr(self, '_dimensions_cache'):
            close = self.closed
            self.open()
            self._dimensions_cache = get_image_dimensions(self, close=close)
        return self._dimensions_cache

ImageFile _get_image_dimensions self.open() This line will throw exception, Image file deleted before this time, so should check file exist or not exist before take width/height. ImageField if file: add existence checking. Both ImageField ImageFile can solve it by adding checking. May be, we can inherit the ImageField, try except the exception.

liuxue0905 commented 4 years ago

Not only admin page, where ever use the model which include this ImageField cause this exception. Especially contain additional width_field height_field fields.

class ModelHasImageField(models.Model):
    .........
    image_field = models.ImageField(width_field='width', height_field='height', .........)
    width = models.PositiveIntegerField(blank=True, null=True)
    height = models.PositiveIntegerField(blank=True, null=True)
    .........
ModelHasImageField.objects.get()
ModelHasImageField.objects.all()
model: XXModel
model.sub_model_has_image_field
anukhatra commented 3 years ago

In models.py

class Product(models.Model): name = models.CharField(max_length=200) image = models.ImageField(upload_to="products",null=True,blank=True)

def __str__(self):
    return self.name

@property
def imageURL(self):
    try:
        url = self.image.url
    except:
        url = ''
    return url

And in product.html img class="thumbnail" src="{{product.imageURL}}"