Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpKernel] Correctly mergemax-age/s-maxage andExpires headers#58376
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| yield'merge max-age and s-maxage' => [ | ||
| ['public' =>true,'max-age' =>'60'], | ||
| ['public' =>true,'max-age' =>null,'s-maxage' =>'60'], |
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.
If one response hasmax-age and the other hass-maxage, the final response should only have as-maxage header and nomax-age (since we don't know the "private" value for one of the responses).
BTW this will cause upstream merge issues because the |
Failing test is unrelated (5.4 branch is failing without this PR as well) |
max-age/s-maxage andExpires headerse6aec6b tod3e65d6CompareThank you@aschempp. |
908a91f intosymfony:5.4Uh oh!
There was an error while loading.Please reload this page.
Thank you Andreas |
The
ResponseCacheStrategydoes not currently mergeExpiresandCache-Control: max-age/s-maxageheaders. Before#41665 this was not an issue, because if not all respones had all headers, they were not added to the final reponse. And we could assume a response itself is consistent betweenExpiresandmax-age.@mpdude addedheuristic caching of public responses in#41665. Unfortunately, it only looks at
Cache-Control: publicbut if should also check if no cache information (max-age/s-maxage/Expires) is present. If that were the case, the behavior would not have changed. But it now leads to inconsistent header values because it independently keepsExpiresandmax-age/s-maxage.This PR does not only fix theheuristic caching, but also merges
ExpiresandCache-Controlheaders to make sure only the lowest value is retained across all headers. For semi-BC reasons I also made sure to only add anExpiresheader if any of the responses contains one.