Simplify Cache-Control Header

[Hamms]

In preparation for an eventual upgrade to Rails 6.1, in which additional logic was added to the existing Cache-Control header normalization logic which special-cases the no-store header: rails/rails#39461

We previously wanted to ensure that must-revalidate was present to prevent issues with older Safari mobile browsers (see #43776 for context), but our belief at this point is that those old versions are no longer relevant.

Links

This logic was originally added in #3724

Testing story

Open to suggestions on the best way to test this. In particular, I'm not sure how to test broad browser compatibility. We know that this might cause issues with older mobile safari browsers, but aren't sure how old you have to go for this to become and issue and we have very limited ability to access older versions.

Caching

In general, the danger here is that even though no-store is the recommended way to avoid caching content, older browsers might have implemented that incorrectly and require workarounds like max-age or must-revalidate to achieve the functionality that's supposed to be offered by no-store. I'm not sure how to identify those possible violations, however, or even what we would do if we did, given that the new Rails logic is quite strict about not mixing other directives with no-store.

PR Checklist:

• Tests provide adequate coverage
• Privacy and Security impacts have been assessed
• Code is well-commented
• New features are translatable or updates will not break translations
• Relevant documentation has been added or updated
• User impact is well-understood and desirable
• Pull Request is labeled appropriately
• Follow-up work items (including potential tech debt) are tracked and linked

[cat5inthecradle]

I'm in favor of this change. This article says that CloudFront will respect the cache-control header unless we explicitly set the minimum TTL to something other than zero. Currently, all behaviors in our cloudfront distribution have MinTTL set to 0.

https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/Expiration.html

[Hamms]

Thanks for that additional context! I'll also a link to this slack thread with some more context and discussion that folks may find useful.

[sureshc]

Feels like it’s worth reviewing with other teams, since this impacts browsers. Should student learning and teacher tools take a look to assess impact on their features? The fact that browser tests are all passing, gives us significant confidence, though.

[Hamms]

I contacted both teams on Wednesday to explain this proposed change and ask them to let me know if they know of any way in which it might negatively impact their features, and so far haven't heard anything.

I think the best way to validate the potential impact of this change at this point is just to merge it; it'll be easy to revert if we discover a problem (at least until we actually do update to Rails 6.1), and the sooner we merge the longer we'll have to test it out in production before the Rails update.