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

ability to get source by image url, not by image field #194

Open 124bit opened 11 years ago

124bit commented 11 years ago

hello. Can you add ability to get source by image url, not by image field? I need it, because I'm using django-EAV and field, that isn't inherited from filefield (yawd-elfinder). I really like image-kit, but can't use it(

matthewwithanm commented 11 years ago

What do you want to get the URL from?

124bit commented 11 years ago

It would be greate not to use image_file as source. I want to use url string, shows the position of image at my server.

not

{% generateimage 'myapp:thumbnail' source=source_image %}

but

{% generateimage 'myapp:thumbnail' source=source_image_url %}

arski commented 11 years ago

Yea, when I read "But you can also generate processed image files directly in your template—from any image—without adding anything to your model." in the Readme - I guessed that this was already possible. However, judging by some testing around, code inspection, and this issue - it is not. It would be a very nice and obvious feature, or at the very least maybe you could update the Readme to reflect the actual state of things.

matthewwithanm commented 11 years ago

Hm…the README seems accurate to me; you can generate processed image files from any image—but you need to pass a file object (not a URL).

124bit commented 11 years ago

it can be done by changing def get_cachefile in imagekit/templatetags/imagekit.py to: (working on windows)

from django.utils.safestring import SafeText
def get_cachefile(context, generator_id, generator_kwargs, source=None):
    generator_id = generator_id.resolve(context)
    kwargs = dict((k, v.resolve(context)) for k, v in generator_kwargs.items())

    if type(kwargs['source'])==SafeText:
        with open(settings.PROJECT_PATH+ kwargs['source'].replace('/','\\'),'rb') as f:
            source=ImageFile(f)
        kwargs['source']=source

    generator = generator_registry.get(generator_id, **kwargs)
    return ImageCacheFile(generator)
matthewwithanm commented 11 years ago

@124bit My problem isn't that I don't know how to implement it; it's that I'm not yet sure if it should be implemented. I'm concerned about potential security implications with having the library load arbitrary URLs, and also unsure about the ambiguity between URLs and local paths (is '/a/b/c.jpg' a path or URL?).

Please note that what you've pasted here isn't doing what you originally asked for: this code accepts a file path—not a URL. Also, it's assuming that the file is local, using open and sidestepping the Django file storage API. Finally, the source file is opened every time with your code—regardless of whether the image needs to be generated or not.

What you've done here can be accomplished without modifying ImageKit—you just need to use another mechanism to load the source file. For example, you could pass the source file to the template from the view. Or, if you want to do all the work in templates, create a template tag to load an arbitrary file from your path:

{% get_file 'path/to/my/image.jpg' as f %}
{% generateimage 'myapp:thumbnail' source=f %}

I know that this is more code that you want to type, but I'm hesitant to add this new feature without having thought through it completely.

124bit commented 11 years ago

oh.. I only needed to use local urls =)). I thought about "every time opening" problem. But I don't know how to solve it. Maybe you can suggest something? I can't find a way to get django ImageFile object, without opening a real file.