You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by fs...@apache.org on 2020/11/16 12:24:07 UTC

[jmeter] branch master updated: JMeter Cache Manager misbehaving when "Use Cache-Control/Expires header" is checked

This is an automated email from the ASF dual-hosted git repository.

fschumacher pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jmeter.git


The following commit(s) were added to refs/heads/master by this push:
     new 589511c  JMeter Cache Manager misbehaving when "Use Cache-Control/Expires header" is checked
589511c is described below

commit 589511cf254b894353494546b82df6f45a6e1066
Author: Felix Schumacher <fe...@internetallee.de>
AuthorDate: Mon Nov 16 13:22:57 2020 +0100

    JMeter Cache Manager misbehaving when "Use Cache-Control/Expires header" is checked
    
    Don't store http responses in the cache, if they have no caching related headers.
    
    Bugzilla Id: 64915
---
 .../jmeter/protocol/http/control/CacheManager.java | 21 ++++++++++++----
 .../http/control/TestCacheManagerBase.java         | 12 ++++++++++
 .../protocol/http/control/TestCacheManagerHC4.java | 24 ++++++++++++++++---
 .../control/TestCacheManagerUrlConnection.java     |  6 +++++
 .../control/TestCacheManagerUrlConnectionBase.java | 28 +++++++++++++++-------
 xdocs/changes.xml                                  |  1 +
 6 files changed, 76 insertions(+), 16 deletions(-)

diff --git a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java
index c2f4fe3..8b26d70 100644
--- a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java
+++ b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java
@@ -191,8 +191,19 @@ public class CacheManager extends ConfigTestElement implements TestStateListener
             String url = conn.getURL().toString();
             String cacheControl = conn.getHeaderField(HTTPConstants.CACHE_CONTROL);
             String date = conn.getHeaderField(HTTPConstants.DATE);
-            setCache(lastModified, cacheControl, expires, etag, url, date, getVaryHeader(varyHeader, asHeaders(res.getRequestHeaders())));
+            if (anyNotBlank(lastModified, expires, etag, cacheControl)) {
+                setCache(lastModified, cacheControl, expires, etag, url, date, getVaryHeader(varyHeader, asHeaders(res.getRequestHeaders())));
+            }
+        }
+    }
+
+    private boolean anyNotBlank(String... values) {
+        for (String value: values) {
+            if (StringUtils.isNotBlank(value)) {
+                return true;
+            }
         }
+        return false;
     }
 
     private Pair<String, String> getVaryHeader(String headerName, Header[] reqHeaders) {
@@ -230,9 +241,11 @@ public class CacheManager extends ConfigTestElement implements TestStateListener
             String etag = getHeader(method ,HTTPConstants.ETAG);
             String cacheControl = getHeader(method, HTTPConstants.CACHE_CONTROL);
             String date = getHeader(method, HTTPConstants.DATE);
-            setCache(lastModified, cacheControl, expires, etag,
-                    res.getUrlAsString(), date, getVaryHeader(varyHeader,
-                            asHeaders(res.getRequestHeaders()))); // TODO correct URL?
+            if (anyNotBlank(lastModified, expires, etag, cacheControl)) {
+                setCache(lastModified, cacheControl, expires, etag,
+                        res.getUrlAsString(), date, getVaryHeader(varyHeader,
+                                asHeaders(res.getRequestHeaders()))); // TODO correct URL?
+            }
         }
     }
 
diff --git a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerBase.java b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerBase.java
index 2c73cea..85beca3 100644
--- a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerBase.java
+++ b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerBase.java
@@ -77,6 +77,8 @@ public abstract class TestCacheManagerBase extends JMeterTestCase {
 
     protected abstract void cacheResult(HTTPSampleResult result) throws Exception;
 
+    protected abstract void cacheResult(HTTPSampleResult sampleResultOK, boolean hasCachingHeaders) throws Exception;
+
     protected abstract void setLastModified(String lastModified);
 
     protected abstract void checkRequestHeader(String requestHeader, String expectedValue);
@@ -323,6 +325,16 @@ public abstract class TestCacheManagerBase extends JMeterTestCase {
     }
 
     @Test
+    public void testNoCachingHeadersBug64915() throws Exception {
+        this.cacheManager.setUseExpires(true);
+        this.cacheManager.testIterationStart(null);
+        assertNoSuchEntry();
+        cacheResult(sampleResultOK, false);
+        assertNoSuchEntry();
+    }
+
+
+    @Test
     public void testCacheHttpClientBug51932() throws Exception {
         this.cacheManager.setUseExpires(true);
         this.cacheManager.testIterationStart(null);
diff --git a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerHC4.java b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerHC4.java
index 0cefd3b..2870f3e 100644
--- a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerHC4.java
+++ b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerHC4.java
@@ -62,10 +62,19 @@ public class TestCacheManagerHC4 extends TestCacheManagerBase {
         private List<org.apache.http.Header> headers;
 
         public HttpResponseStub() {
+            this(true);
+        }
+
+        public HttpResponseStub(boolean cachingHeaders) {
             this.headers = new ArrayList<>();
-            this.lastModifiedHeader = new BasicHeader(HTTPConstants.LAST_MODIFIED, currentTimeInGMT);
             this.dateHeader = new BasicHeader(HTTPConstants.DATE, currentTimeInGMT);
-            this.etagHeader = new BasicHeader(HTTPConstants.ETAG, EXPECTED_ETAG);
+            if (cachingHeaders) {
+                this.lastModifiedHeader = new BasicHeader(HTTPConstants.LAST_MODIFIED, currentTimeInGMT);
+                this.etagHeader = new BasicHeader(HTTPConstants.ETAG, EXPECTED_ETAG);
+            } else {
+                this.lastModifiedHeader = null;
+                this.etagHeader = null;
+            }
         }
 
         /* (non-Javadoc)
@@ -236,7 +245,16 @@ public class TestCacheManagerHC4 extends TestCacheManagerBase {
 
     @Override
     protected void cacheResult(HTTPSampleResult result) {
-        this.cacheManager.saveDetails(httpResponse, result);
+        cacheResult(result, true);
+    }
+
+    @Override
+    protected void cacheResult(HTTPSampleResult result, boolean hasCachingHeaders) {
+        if (hasCachingHeaders) {
+            this.cacheManager.saveDetails(httpResponse, result);
+        } else {
+            this.cacheManager.saveDetails(new HttpResponseStub(false), result);
+        }
     }
 
     @Override
diff --git a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnection.java b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnection.java
index eb74191..fb1c3bd 100644
--- a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnection.java
+++ b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnection.java
@@ -30,6 +30,12 @@ public class TestCacheManagerUrlConnection extends TestCacheManagerUrlConnection
 
     @Override
     protected void cacheResult(HTTPSampleResult result) throws Exception {
+        cacheResult(result, true);
+    }
+
+    @Override
+    protected void cacheResult(HTTPSampleResult result, boolean hasCachingHeaders) throws Exception {
+        ((URLConnectionStub) this.urlConnection).setCachingHeaders(hasCachingHeaders);
         this.cacheManager.saveDetails(this.urlConnection, result);
     }
 
diff --git a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnectionBase.java b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnectionBase.java
index 728d022..8d74867 100644
--- a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnectionBase.java
+++ b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnectionBase.java
@@ -29,6 +29,8 @@ import org.junit.jupiter.api.BeforeEach;
 public abstract class TestCacheManagerUrlConnectionBase extends TestCacheManagerBase {
     protected class URLConnectionStub extends HttpURLConnection {
 
+        private boolean cachingHeaders = true;
+
         protected URLConnectionStub(URL url) {
             super(url);
         }
@@ -47,15 +49,18 @@ public abstract class TestCacheManagerUrlConnectionBase extends TestCacheManager
 
         @Override
         public String getHeaderField(String name) {
-            if (HTTPConstants.LAST_MODIFIED.equals(name)) {
-                return lastModifiedHeader;
-            } else if (HTTPConstants.ETAG.equals(name)) {
-                return EXPECTED_ETAG;
-            } else if (HTTPConstants.EXPIRES.equals(name)) {
-                return expires;
-            } else if (HTTPConstants.CACHE_CONTROL.equals(name)) {
-                return cacheControl;
-            } else if (HTTPConstants.DATE.equals(name)) {
+            if (cachingHeaders) {
+                if (HTTPConstants.LAST_MODIFIED.equals(name)) {
+                    return lastModifiedHeader;
+                } else if (HTTPConstants.ETAG.equals(name)) {
+                    return EXPECTED_ETAG;
+                } else if (HTTPConstants.EXPIRES.equals(name)) {
+                    return expires;
+                } else if (HTTPConstants.CACHE_CONTROL.equals(name)) {
+                    return cacheControl;
+                }
+            }
+            if (HTTPConstants.DATE.equals(name)) {
                 return currentTimeInGMT;
             } else if (HTTPConstants.VARY.equals(name)) {
                 return vary;
@@ -76,7 +81,12 @@ public abstract class TestCacheManagerUrlConnectionBase extends TestCacheManager
         public boolean usingProxy() {
             return false;
         }
+
+        protected void setCachingHeaders(boolean useSpecialHeaders) {
+            this.cachingHeaders = useSpecialHeaders;
+        }
     }
+
     protected URLConnection urlConnection;
 
     @Override
diff --git a/xdocs/changes.xml b/xdocs/changes.xml
index 1a48b05..df80a05 100644
--- a/xdocs/changes.xml
+++ b/xdocs/changes.xml
@@ -184,6 +184,7 @@ applications when JMeter is starting up.</p>
 <h3>Timers, Assertions, Config, Pre- &amp; Post-Processors</h3>
 <ul>
     <li><bug>64638</bug>JSON JMESPath Assertion / JSON Assertion: Opening GUI shows a horizontal scrollbar that keeps sliding</li>
+    <li><bug>64915</bug>JMeter Cache Manager misbehaving when "Use Cache-Control/Expires header" is checked</li>
 </ul>
 
 <h3>Functions</h3>