-
Notifications
You must be signed in to change notification settings - Fork 21.9k
Add support for Cache-Control request directives #55033
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for Cache-Control request directives #55033
Conversation
b0eae55
to
e47f341
Compare
actionpack/CHANGELOG.md
Outdated
|
||
# Value directives | ||
request.cache_control_directives.max_age # => integer or nil | ||
request.cache_control_directives.max_stale # => integer or nil (or true for valueless max-stale) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit of a problem, users are likely to assume max_stale
if truthy is an Integer and not handle true
correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, I see max_stale_unlimited?
. I suppose that is acceptable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The interface look good to me. But I'd rather have an implementation that doesn't use a Hash.
actionpack/CHANGELOG.md
Outdated
|
||
# Value directives | ||
request.cache_control_directives.max_age # => integer or nil | ||
request.cache_control_directives.max_stale # => integer or nil (or true for valueless max-stale) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, I see max_stale_unlimited?
. I suppose that is acceptable
e47f341
to
5c9904d
Compare
Co-Authored-By: Jean Boussier <jean.boussier@gmail.com>
5c9904d
to
55b54a3
Compare
Motivation / Background
This Pull Request has been created because the HTTP Cache-Control request directives were not fully supported in Rails. According to RFC 9111, HTTP clients can use various cache control directives in their requests to control how caching is handled. This implementation adds comprehensive support for these directives, with special attention to the
max-stale
directive which can be used with or without a value.Detail
This Pull Request changes the following:
CacheControlDirectives
class that parses and provides access to HTTP Cache-Control request directivesno-cache
,no-store
,no-transform
,only-if-cached
)max-age
,min-fresh
,stale-if-error
)max-stale
directive which can appear both with a value (e.g.,max-stale=300
) or without a value (indicating unlimited staleness)This enhancement allows Rails applications to better integrate with HTTP caching mechanisms and provides more control over how requests interact with cached resources.
Additional information
The implementation follows RFC 9111 (HTTP Caching): https://www.rfc-editor.org/rfc/rfc9111.html#name-cache-control
The
max-stale
directive required special handling as it can appear without a value, in which case it indicates the client is willing to accept a response no matter how stale it is.This PR is a comprehensive implementation that incorporates feedback and suggestions from the previous PR Add support for Cache-Control: only-if-cached directive #55008. (@byroot )
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]