Although this module is supposed to just make resized images nice and crisp when resized, IMO a feature that is just as important is the fact that remote images are copied to your server to make them local. This solves the problems associated with hotlinking to images. Eg. On our school website with children linking to images around the Net, I'm going to have to be very aware of other webmasters deciding to serve us porn or sick images instead of the hotlinked images - this is easily done in htaccess.

However, at the moment the copying is done transparently - if the user doesn't resize the image, a local copy isn't created on the server. They're making a decision about the image without even knowing it. So I was wondering if it was possible by adding an option to 'always make a copy of remote images', so it wouldn't be necessary to resize them first? This would still be a transparent action to the user, but puts the decision back into the website owner's hands.

Thoughts?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quicksketch’s picture

I think this fundamentally falls outside the scope of what this module accomplishes. The ability to resize external images was meant to be a feature that adds consistency to the resizing functionality, it was never meant to be a central feature of the module. I'm generally opposed to adding this, since hosting all remote images was never the purpose of this module and it certainly would not be an expected feature to find in a module that advertises itself as an image resizing utility.

quicksketch’s picture

Status: Active » Needs review
FileSize
3.69 KB

Thinking about this further, I definitely see where you're coming from. It's odd that a resized image would be considered "safe" yet an unresized one might cause trouble. The problem I have generally is that an option to "copy remote images locally" could be a module all in itself. However the logic used by Image Resize Filter is rather extensive in terms of finding all the images, determining if it's local or remote, then copying that image locally.

So now I'm weighing the two possible sides of this: Copying images locally is not intended to be a central feature or... copying images locally is inconsistent from the user-perspective depending on if they resize the image or not.

Here's a patch which I'm putting out there for feedback, which does not add any new options to Image Resize Filter. Instead it simply assumes all remote images should always be copied; resized or not. In other words the administrator doesn't even get an option to decide if the behavior should be consistent or not, all images are copied locally no matter what if the option is enabled. I added a single exception for 1x1 images, which are frequently used in tracking scripts as the non-JavaScript fallback.

-Anti-’s picture

Thanks for following this up.
I would also be interested to hear other opinions with regard to 'hotlinked vs localised' images.

I'll try to test out the patch in the next couple of weeks.

helmo’s picture

I just applied the patch and experimented a little. It works as expected for me.
About caching all remote images: It seems sensible and reduces complexity for the admin.

Since I had recently wished for such functionality I am really glad you implemented this, and hope it can be committed soon.

helmo’s picture

After debugging this a bit more I noticed a nasty side-effect. Since a md5 of the whole image was used to generate the local filename, this original image data was needed on every single pageload. Just to find out what the local filename would be.

The attached patch should solve this by using md5($image['original']) and refactor the code some more. It is based on the cvs HEAD.
Only when the remote url does not show a filetype do we fetch the image a second time to determine the extension.

Zach Harkey’s picture

Brilliant.

+1 for caching local copies of ALL external images, regardless of being resized.

I had resorted to super kludge of resizing external images by 1px just to force this behavior.

The 1x1 pixel exception is great thinking too.

Might also be a good idea to ignore images with class="no-resize" or something.

bryancasler’s picture

This is fantastic, but I am running into one remaining problem. I use CKEditor, which allows image embedding. However sometimes the images don't automatically populate their width and height attributes. This results in the following problem.

Behaves when width and height attributes are set
Input:---<img alt="" height="84" src="http://drupal.org/sites/all/themes/bluebeach/logos/drupal.org.png" width="264" />
Output:-<img width="264" height="84" src="/sites/default/files/resize/remote/adc18e5313250c6cee9894e9fe341e16-264x84.png" alt="">

Breaks when they are not
Input:---<img alt="" src="http://drupal.org/sites/all/themes/bluebeach/logos/drupal.org.png" />
Output:-<img src="http://drupal.org/sites/all/themes/bluebeach/logos/drupal.org.png" alt="">

quicksketch’s picture

animelion, that's not related to this issue. Maybe check out #728346: Resize images based on predefined size. or open a new issue.

adub’s picture

Also interested in resizing and storing remote images locally. Is it possible that a separate input filter module could resize image attributes not to break layout which would then trigger this module to move and resize the files? Or is that misusing this module?

update: I've just hacked something together which does it using customfilter and this regex - http://www.experts-exchange.com/Web_Development/Web_Languages-Standards/... Doesn't seem like a production solution but might be handy for someone working on this.

quicksketch’s picture

Is it possible that a separate input filter module could resize image attributes not to break layout which would then trigger this module to move and resize the files?

Yes, as long as the image size has been changed by the time Image Resize Filter runs, it will then save and resize it. That's where the Order comes in underneath the Input Format configuration.

quicksketch’s picture

Version: 6.x-1.5 » 6.x-1.6
Status: Needs review » Fixed
FileSize
3.87 KB

Things have changes quite a bit since I first made the patch in #2 and it was updated in #5. I rerolled it with the latest code and included it. It doesn't seem that there was any opposition to this change, and it all makes sense to me. I've included this patch in CVS and it will be in the 1.7 version.

quicksketch’s picture

Title: hotlinking solution » Download all remote images locally (a hotlinking solution)
quicksketch’s picture

I had quite an error in #11, which caused all remote images to not do anything at all. Fixed with this additional patch on top of #11.

quicksketch’s picture

Since we're now doing things to images that aren't even resized, I opened and committed #765194: Always add height and width attributes even when not resizing as well, which is a similar "fix images" solution.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.