[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.
Thanks for taking the time to raise this issue. As you are no doubt aware this issue has been on our backlog for some time now with very little movement.
I'm going to close this to set expectations so the issue doesn't stay open for years with few updates. If the issue is still relevant please feel free to reopen it or create a new issue.

Generated at Sun Feb 11 23:51:37 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.