When acquiring a new image generation callback URL image_style_url() sets an access token that is then verified when the newly acquired URL is accessed.

 // Set a cache entry to grant access to this style/image path. This will be
// checked by image_style_generate().
cache_set('access:' . $style_name . ':' . md5($path), 1, 'cache_image', REQUEST_TIME + 600); 

image_style_generate() checks for the existence of that access token before generating the image but then never removes the token after successfully generating the image.

 // Check that it's a defined style and that access was granted by
// image_style_url().
if (!$style || !cache_get('access:' . $style_name . ':' . $path_md5, 'cache_image')) {
  drupal_access_denied();
  drupal_exit();
} 

This means that the acquired image generation URL remains valid until the cache_image table is wiped. Is this intentional? Or should image_style_generate() remove the access token after generating the image?

Comments

quicksketch’s picture

It's not a critical problem that the cache entry is not manually removed because the time to live is only 5 minutes (600 seconds), so the next cron run this cache entry will be cleaned up. However it certainly won't hurt to simply remove the cache entry immediately when the image is generated.

Completely separate from the cache issue however, we might consider using the new lock system (http://api.drupal.org/api/group/lock/7) instead and not use cache tables at all. I haven't looked into this enough, but it certainly seems like a possible alternative and much more appropriate for this sort of temporary access.

eojthebrave’s picture

We're already using the lock system to prevent multiple requests from generating the same image. See image_style_generate().

I believe the token used here is to prevent the use of image generation callback paths without first calling image_style_url() to generate the path. Ensuring that only Drupal can request that an image be generated. To be honest, I'm not sure I completely understand why this is here. Could someone explain this? Do we even need this?

Either way, the lock system would not allow for the same functionality.