Github user vlsi commented on a diff in the pull request:

    https://github.com/apache/jmeter/pull/298#discussion_r122906537
  
    --- Diff: 
src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java ---
    @@ -180,65 +221,97 @@ private void setCache(String lastModified, String 
cacheControl, String expires,
             }
             Date expiresDate = null; // i.e. not using Expires
             if (useExpires) {// Check that we are processing 
Expires/CacheControl
    -            final String MAX_AGE = "max-age=";
    -            
    +            final String maxAge = "max-age=";
    +
                 if(cacheControl != null && cacheControl.contains("no-store")) {
                     // We must not store an CacheEntry, otherwise a 
                     // conditional request may be made
                     return;
                 }
                 if (expires != null) {
    -                try {
    -                    expiresDate = 
org.apache.http.client.utils.DateUtils.parseDate(expires);
    -                } catch (IllegalArgumentException e) {
    -                    if (log.isDebugEnabled()){
    -                        log.debug("Unable to parse Expires: '"+expires+"' 
"+e);
    -                    }
    -                    expiresDate = CacheManager.EXPIRED_DATE; // invalid 
dates must be treated as expired
    -                }
    +                expiresDate = extractExpiresDateFromExpires(expires);
                 }
                 // if no-cache is present, ensure that expiresDate remains 
null, which forces revalidation
                 if(cacheControl != null && !cacheControl.contains("no-cache")) 
{
    -                // the max-age directive overrides the Expires header,
    -                if(cacheControl.contains(MAX_AGE)) {
    -                    long maxAgeInSecs = Long.parseLong(
    -                            
cacheControl.substring(cacheControl.indexOf(MAX_AGE)+MAX_AGE.length())
    -                                .split("[, ]")[0] // Bug 51932 - allow for 
optional trailing attributes
    -                            );
    -                    expiresDate=new 
Date(System.currentTimeMillis()+maxAgeInSecs*1000);
    -
    -                } else if(expires==null) { // No max-age && No expires
    -                    if(!StringUtils.isEmpty(lastModified) && 
!StringUtils.isEmpty(date)) {
    -                        try {
    -                            Date responseDate = DateUtils.parseDate( date 
);
    -                            Date lastModifiedAsDate = DateUtils.parseDate( 
lastModified );
    -                            // see 
https://developer.mozilla.org/en/HTTP_Caching_FAQ
    -                            // see 
http://www.ietf.org/rfc/rfc2616.txt#13.2.4 
    -                            expiresDate=new Date(System.currentTimeMillis()
    -                                    
+Math.round((responseDate.getTime()-lastModifiedAsDate.getTime())*0.1));
    -                        } catch(IllegalArgumentException e) {
    -                            // date or lastModified may be null or in bad 
format
    -                            if(log.isWarnEnabled()) {
    -                                log.warn("Failed computing expiration date 
with following info:"
    -                                    +lastModified + "," 
    -                                    + cacheControl + ","
    -                                    + expires + "," 
    -                                    + etag + ","
    -                                    + url + ","
    -                                    + date);
    -                            }
    -                            // TODO Can't see anything in SPEC
    -                            expiresDate = new 
Date(System.currentTimeMillis()+ONE_YEAR_MS);
    -                        }
    -                    } else {
    -                        // TODO Can't see anything in SPEC
    -                        expiresDate = new 
Date(System.currentTimeMillis()+ONE_YEAR_MS);
    -                    }
    -                }  
    +                expiresDate = 
extractExpiresDateFromCacheControl(lastModified,
    +                        cacheControl, expires, etag, url, date, maxAge);
                     // else expiresDate computed in (expires!=null) condition 
is used
                 }
             }
    -        getCache().put(url, new CacheEntry(lastModified, expiresDate, 
etag));
    +        if (varyHeader != null) {
    +            log.debug("Set entry into cache for url {} and vary {} ({})", 
url,
    +                    varyHeader,
    +                    varyUrl(url, varyHeader.getLeft(), 
varyHeader.getRight()));
    --- End diff --
    
    Please add `isDebugEnabled` check to prevent `varyUrl` computation in debug 
disabled case.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to