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
100 stars 10 forks source link

Specify path to store processed images #8

Closed hleroy closed 3 years ago

hleroy commented 3 years ago

Hello Matthias,

In my model, I'm using a custom upload path in order to serve photos privately. Would it be possible for django-imagefield to store processed images in the same "upload_to" location as the initial image ? Or call a function which will provide the secure path ?

image = ImageField(auto_add_fields=True, upload_to=get_image_path, storage=secure_fs)
def get_image_path(instance, filename):
    """Return image path formatted as PRIVATE_ROOT/user_id/."""
    path = '{path}/{id}/'.format(path=settings.PRIVATE_ROOT, id=instance.user.id)
asencis commented 3 years ago

@matthiask This one still needs answering...

matthiask commented 3 years ago

Hi,

Processed images should already be saved using the secure_fs storage so you shouldn't have to change anything if all you want is to make images not accessible using the default Django filesystem storage.

However, changing the path name (everything after the storage root) wouldn't be backwards compatible. The path is determined here: https://github.com/matthiask/django-imagefield/blob/26f126f3a91b94b2e23dbc6b5da86fdb81ed0bee/imagefield/fields.py#L151-L182 If you know 1) the original filename of uploaded images and 2) the processors in use you can deterministically calculate the resulting processed image name. That is by design and the reason, why django-imagefield works well without a cache.

What you could do is insert a hard-to-guess path component into uploaded images. This may help. Maybe something like lambda instance, filename: f"{root}/{instance.user_id}/{uuid.uuid4()}-{filename}". This is still security-by-obscurity and may not be sufficient for you. (Btw, I think your get_image_path has a bug – I think callable upload_to values should also return the filename.)

(Thanks for the reminder @asencis , however, since I'm not being paid for this an answer cannot be expected.)

asencis commented 3 years ago

@matthiask Hi Matt, no worries - and thank you for this answer. I was just going through this package and noticed that there was a "stale" open question and thought it was probably just something that slipped through the net. Appreciate that you don't get paid for this. We're looking to help with financials for the Django community and will keep this package on our radar.

hleroy commented 3 years ago

@matthiask Thank you for your detailed answer. I really need the same behavior as Django's ImageField. But I understand it wouldn't be backward compatible to implement it.

Regarding get_image_path. You're right, get_image_path must also return the filename. I didn't copy my function in its entirety:

def get_image_path(instance, filename):
    """
    Return image path formatted as PRIVATE_ROOT/user_id/year/month/day/filename
    """
    path = '{path}/{id}/{year}/{month}/{day}/'.format(path=settings.PRIVATE_ROOT,
                                                      id=instance.note.user.id,
                                                      year=instance.note.datetime.year,
                                                      month=instance.note.datetime.month,
                                                      day=instance.note.datetime.day)
    # Create path if it doesn't exist
    if not os.path.isdir(path):
        os.makedirs(path, mode=0o755, exist_ok=True)
    return path + filename