Then the current implementation is wrong, because it considers the
request fresh with one OR the other is fresh:
def fresh?(response)
not_modified?(response.last_modified) || etag_matches?
(response.etag)
end
The correct implementation would be:
def not_modified?(modified_at)
modified_at && if_modified_since >= modified_at
end
def etag_matches?(etag)
if_none_match == etag
end
def fresh?(response)
if if_modified_since && if_none_match
not_modified?(response.last_modified) && etag_matches?
(response.etag)
if if_modified_since
not_modified?(response.last_modified)
if if_none_match
etag_matches?(response.etag)
else
false
end
end
Why bother? Sometimes I want to cache a resource considering its
timestamp, but this cache is only valid for the current user (works
like scoping):
def show
response.last_modified = resource.updated_at
response.etag = current_user
if request.fresh?(response)
head :not_modified
else
# ...
end
end
In the current implementation, even the first example given by Ryan
Daigle on his blog will fail:
I'm not sure you're reading that paragraph correctly. It's intended
to supporting both http 1.0 and 1.1 caches, if we made the change
you're suggesting any 1.0 caches will ignore the etags and serve your
old data. Which would defeat the purpose of its inclusion.
I believe the current behaviour is valid, it uses both validators,
merely shortcircuiting once it's found a match.
def show
response.last_modified = resource.updated_at
response.etag = current_user
if request.fresh?(response)
head :not_modified
else
# ...
end
end
In the current implementation, even the first example given by Ryan
Daigle on his blog will fail:
You should include the user id in the etag along with the other
freshness information.
According to the specification, if both Last-Modified date and Entity
Tags are sent, both must be validated to consider the request fresh:
You're right. The relevant part from the spec is: "An HTTP/1.1 caching
proxy, upon receiving a conditional request that includes both a Last-
Modified date and one or more entity tags as cache validators, MUST
NOT return a locally cached response to the client unless that cached
response is consistent with all of the conditional header fields in
the request."
Considering this, we also have to change automatic etagging.
We should not automatically set etag (with body digest) when Last-
Modified field is set, because to check this etag, we would have to
render the action and this is what we want to avoid in first place.
And the current implementation of conditional get
(handler_conditional_get!) automatically replies 304 whenever the etag
is matching (even if it's not body digest).
I believe this might be the right implementation:
def handle_conditional_get!
if etag? || last_modified?
set_conditional_cache_control!
elsif nonempty_ok_response?
self.etag = body
if request && request.etag_matches?(etag)
self.status = '304 Not Modified'
self.body = ''
end
set_conditional_cache_control!
end
end
Considering this, we also have to change automatic etagging.
We should not automatically set etag (with body digest) when Last-
Modified field is set, because to check this etag, we would have to
render the action and this is what we want to avoid in first place.
You're right. I've added your implementation and tests for it. Thanks
for helping to sanity check all this.
protected
def list
@list ||= List.find(params[:id])
end
def current_user
@current_user ||= User.find(params[:user_id])
end
end
It's very lightweight (100 lines) and accepts also Procs, Methods and
Symbols as parameters.
If there is some interest in merging into Rails Core, I will be glad
to help again.
