search

matthiask / django-imagefield

You should probably use this image field instead of Django's built-in models.ImageField.
https://django-imagefield.readthedocs.io
BSD 3-Clause "New" or "Revised" License
101 stars 10 forks source link

Prevent a server error if the name of the ImageField is not a valid cache-key #4

Closed sacovo closed 4 years ago

sacovo commented 4 years ago

If any exception occurs while accessing the cache set the url to an empty string. If the form that contains the ImageField had any validation-errors, the name of the ImageField is not cleaned and could be an invalid key for caches like Memcached.

3

matthiask commented 4 years ago

Hmm, I suspect that you might not want to silently ignore cache errors. Maybe we should first try generating better cache keys?

sacovo commented 4 years ago

Under django.utils.text there is get_valid_filename, that is also used by the file storage backend.

matthiask commented 4 years ago

Thanks again.

I worry a bit that different images may now be mapped to the same cache key. Hashing the filename should work; it does unfortunately produce opaque cache keys but I think that's acceptable as a small downside.

Something like key = "imagefield-admin-thumb:%s" % hashlib.sha256(value.name).hexdigest() maybe? I'm sorry for the back and forth. I definitely think it's worth it fixing this bug. Thanks for your patience.

sacovo commented 4 years ago

Thanks for your input, hashing also has the advantage that the keys will always be the same length, memchached for example has a limit of 250 bytes for a key.