When sending a 304 response, imagecache uses the default drupal headers if the files system is private.
Drupal default headers:

Cache-Control:store, no-cache, must-revalidate
post-check=0, pre-check=0
Connection:close
Date:Wed, 06 Jul 2011 16:30:35 GMT
Etag:"774eac11f1bd23bf6c3d157bf5c044ec"
Expires:Sun, 19 Nov 1978 05:00:00 GMT
Server:Apache/2.2.3 (CentOS)

The Cache-Control and Expires lines prevent caching of the images in some browsers.
I propose sending the same headers in the 304 as in the 200 response

Cache-Control:max-age=1209600, private, must-revalidate
Connection:close
Date:Wed, 06 Jul 2011 16:29:55 GMT
Etag:"774eac11f1bd23bf6c3d157bf5c044ec"
Expires:Wed, 20 Jul 2011 16:29:55 GMT
Server:Apache/2.2.3 (CentOS)

These headers are set in imagecache_transfer(), but _imagecache_cache_set_cache_headers() exits without applying them.
I would like to include the additional headers by inserting the code from file_transfer() just before the "exit;" in _imagecache_cache_set_cache_headers() line 580 in the dev version.

foreach ($headers as $header) {
    // To prevent HTTP header injection, we delete new lines that are
    // not followed by a space or a tab.
    // See http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2
    $header = preg_replace('/\r?\n(?!\t| )/', '', $header);
    drupal_set_header($header);
  }

or expanding the call to drupal_set_header() to

foreach ($headers as $header) {
    // To prevent HTTP header injection, we delete new lines that are
    // not followed by a space or a tab.
    // See http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2
    $header = preg_replace('/\r?\n(?!\t| )/', '', $header);
    if (strlen($header)) {
        header($header);
    }
}

I am not sure what security/usability issues there might be from these changes, so I am not providing a patch. I believe it should be safe to change from the drupal default headers to the imagecache default headers.
I have applied the changes on my site, which allows caching of the images served from a private file system.

CommentFileSizeAuthor
#1 304-headers-1210920-0.patch511 bytessandrewj
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sandrewj’s picture

Status: Active » Needs review
FileSize
511 bytes

I've been using this for a while with no problems. Please review.

fizk’s picture

Issue tags: +ImageCache 3

Marking as ImageCache 3.x Todo.