Improve 304 response so that browser will cache on subsequent calls

[danmanirl]

In the 304 response for images, PhotoShow sends the default "no-cache" Cache-Control header, causing the browser to omit the If-Modified-Since header. This fixes that, so that it will cache every time.

The value "240" is included in case the browser does not "do the right thing" and use the expiry date from the original request.

Tested on "Mozilla Firefox for Ubuntu version 56"

[gboudreau]

Why 240 ? Why not use the user-chosen setting Settings::$cache_max_age ?

[danmanirl]

I'm not sure if all browsers handle it correctly and use the expiry from the original request rather than the new expiry. If it is the latter the cache may never expire? I don't have a huge amount of experience with http caching.

It's probably over-paranoid and I can set it to cache-max-age if you think it's better.

[gboudreau]

The CC max-age is a number of seconds starting now, not when the file was last modified, or when the browser sent the first query. Returning the same value for 200 and 304 responses is the right way to do this.

Example: if the user selected a 1h cache_max_age setting, that tells the browser to re-use the cached version of the file for 1h before checking with the server again to see if the file changed. After 1h has passed, the browser will query the server again, and if the server replies with a 304 Not Modified, it should again indicate that the browser should check again in 1h.

I would suggest moving lines that send the Cache-Control and Expires headers before the if statement on line 443. That should fix the issue with 304 responses.

Also, I suggest adding a Pragma: private HTTP header.

[danmanirl]

As far as I can tell from experimenting with the firefox browser, this is not the behaviour.

The browser stores the original expiry of the image and uses that to decide whether to add the header If-Modified-Since. This can be shown by setting the value above to 5 instead of 240. Go to a "big" image page twice using the browser back button and not page refresh, once a 304 is received with an expiry in 14 days time, then wait 10 seconds and try again. It sends the If-Modified-Since header and receives a 304 response.

Having said that, I agree that sending the same header values for both responses is correct. Changes made.

[gboudreau]

What you describe is caused by the Cache-control' max-age value having priority over the Expires header. When the server sends both, the max-age value takes priority, and the Expires header is ignored.

[thibaud-rohmer]

Well, if it's been approved by Guillaume, looks like I have no choice.... :smile: