/cc@jeremy

Nice! If we make this more conservative, we can probably get away with sneaking this into 4.2:

1. Add a new configuration option calledgenerate_weak_etags or something that defaults tofalse (similar tothis one)
2. In#etag=, check the value of that option. If it's truthy, processed with generating a weak etag (similar tohere)
3. Otherwise, generate a strong etag like we used to, but print a deprecation warning message saying that the defualt will flip in Rails 5 (similar tothis one)

This seems like an overall improvement to the situation and a good halfway point. Ideally though, we should also figure out the final API we would like to achieve with this (though I'm not sure if something like that is still appropriate for 4.2 at this time)...

1. Weak etag is a good default for the 90% cases (it seems quite difficult, at least at the controller level, to determine if the response would meet the byte-level equivalence requirement... given all the things that goes on in a typical Rails stack)
2. That said, there are probably cases that you would like to explicitly make the etag a strong one, such as thesend_file use case@jeremy brought up.

With the current API, I'm not sure what's the best way to achieve point 2, and I'd love to hear ideas 😄

"Sorry, Diff contents are not available for this pull request."

First time I'm seeing this, why?

Good question. I'm seeing this too.

Yeah, I have no idea, I had to pull the branch locally to check the diff

Same here, I checked the changes inhttps://github.com/rails/rails/pull/17573.diff

[Sorry about the late response.]
There was an error in the earlier code, hence the new commit.

I'm a bit of a noob here, but from what I've seen in the code, our current behavior resembles weak etags more. Even though the etags emitted look strong. I could not find any code that actually creates an etag based off the entire response body. Here's an example to illustrate -

In the classic blog post application, we can write thePostsController#show as follows to take advantage of etags -

def show  fresh_when(@post)end

And then here's what we see from the browser -

The same happens when using thestale? method and the controller wideetag. So it looks like etag generation and verification does not take the full response body into account. And If I am correct so far, we will need to introduce support for strong etags.

I too feel that strong etags are important for cases likesend_file. Thesend_file doesn't use any etags as of now, and I'm tempted to have them added, but am not sure if its a good idea.

I'm having a lot of cache misses because of this behavior. I'm using nginx as reverse proxy and when a page is requested withAccept-Encoding: deflate, gzip (i.e. always) it automatically changes the generated strong etag by adding the prefixW/. Then following requests sent by user with the same browser won't match rails default etag causing 200 instead of 304 even if the response is the same.

Rack::ETagalready do this and I think it's very important to have rails following this behavior for the reasons explainedhere

As a quick alternative theetag_matches? method can be changed to strip anyW/ prefix before matching, but I don't know what side effects this can cause.

I'm going to tryrails_weak_etags middleware to see if it solves my issues.

LGTM. What's missing to get this merged?

I don't think there needs to be a configuration options, because the way etags are calculated is already weak validation and Rack::ETag even weakens it's strong etags (a body digest is considered a strong validator based on RFC 7232 Section 2.1, while determining the etag from arbitrary data is considered a weak validator).

The current problem without this patch is that NGINX weakens strong ETags when it changes the content encoding (gzip). So right nowfresh_when andstale? are useless on NGINX with gzip enabled.

Another approach would be to consider a weakened etag equivalent to the strong etag sent by action dispatch (but I think sending weak etags as proposed here is the correct approach):

moduleActionDispatchmoduleHttpmoduleCachemoduleRequestNORMALIZE_ETAG=/^(?:W\/)?\"|\"$/.freezedefif_none_match_etags(if_none_match ?if_none_match.split(/\s*,\s*/) :[]).collectdo |etag|etag.gsub(NORMALIZE_ETAG,"")endenddefetag_matches?(etag)ifetagetag=etag.gsub(NORMALIZE_ETAG,"")if_none_match_etags.include?(etag)endendendendendend

I'm currently using the above code as a monkey patch with rails 4.2.4 to fix If-None-Match on NGINX 1.8.

I installedrails_weak_etags and everything work like a charm, no monkey patch needed, just a middleware.

In any case it would be very nice to have this merged (and backported to rails 4.x)

Is there any status update on this issue? Therails_weak_etags gem appears to solve the problem, but it seems like the current strong ETAG behavior is a bug in rails and should be fixed.

FWIW the last big rails projects I worked on, they all had a middleware to do something like this.

Please rebase.

Sorry I was a little out of touch off this. Allow me a bit to rebase.

Rebased.

Looks good to merge@chancancode. We'll need a guide update and some mention of how to use strong ETags now (set the header manually, I presume).

Thanks everyone for working on this, sorry it took so long! I opened a new issue to track the remaining work:#23148

Thanks@chancancode and others. Should this be back-ported to 4.2 as well?

This is a new feature/breaking change, so we cannot. We only backport bug fixes.

Thanks. Commenting for future Rails 4.2 users on CloudFlare because CF (silently) automatically convert strong ETags to weak ETags by prepending W/ in the header. Googling W/ is pretty ineffective and CloudFlare doesn't document this behaviour so hopefully this comment helps visibility. TLDR use Rails 5 or therails_weak_etags gem.