[MGNLCACHE-41] Magnolia Cache returns 200 for 'docroot' CachedPage if request includes If-None-Match header Created: 01/Jun/09 Updated: 04/Nov/15 Resolved: 04/Nov/15 |
|
| Status: | Closed |
| Project: | Cache Modules |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Neutral |
| Reporter: | Dean Arnold | Assignee: | Unassigned |
| Resolution: | Won't Do | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
Magnolia Community 4.0.1 on Tomcat 6.0.16 |
||
| Template: |
|
| Acceptance criteria: |
Empty
|
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
| Date of First Response: |
| Description |
|
The current AbstractExecutor.ifModifiedSince() method is always returning true if the request for a cached page includes the 'If-None-Match' header. The 'If-None-Match' header is/should be set by the browser as a request header if the prior response for a page/resource included the Etag header, which all 'docroot' cached responses from Magnolia (on Tomcat) do. Rather than returning 'true' to the IfModifiedSince() method, if the 'If-None-Match' header is present in the request, the cache should compare its value with that of the 'Etag' header that is stored with the CachedPage headers. If the ETag header is not present, the cache should fall back on last modified date. N.B. the Etag header being added to 'docroot' resources e.g. css/js/images etc. It is not added for Magnolia repository managed content. |
| Comments |
| Comment by Magnolia International [ 09/Sep/09 ] |
|
Thanks Dean; it looks like something that might require a slight api change - would you by chance have a patch for this ? |
| Comment by Jan Haderka [ 11/Mar/10 ] |
|
related bit of code from the old cache (just to keep it somewhere while getting rid of the old code) public boolean ifModifiedSince(HttpServletRequest request, long lastModified) { try { long headerValue = request.getDateHeader("If-Modified-Since"); if (headerValue != -1) { // If an If-None-Match header has been specified, if modified since // is ignored. if ((request.getHeader("If-None-Match") == null) && (lastModified > 0 && lastModified <= headerValue + 1000)) { return false; } } } catch (IllegalArgumentException illegalArgument) { return true; } return true; } |
| Comment by Magnolia International [ 11/Mar/10 ] |
|
Jan, that piece is now in the SetExpirationsHeader cache executor afaik; the missing piece is that we don't look at the etags.ß |
| Comment by Jan Haderka [ 06/Aug/10 ] |
|
umm, no. The SetExpirationsHeader does exactly what name implies. It sets the Cache-control header of the response. The code I quoted is in the AbstractExecutor (which admittedly is a superclass of the SetExpirationHeader. |
| Comment by Michael Mühlebach [ 04/Nov/15 ] |
|
Given the thousands of other issues we have open that are more highly requested, we won't be able to address this issue in the foreseeable future. Instead we will focus on issues with a higher impact, and more votes. |