hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject svn commit: r1512441 - in /httpcomponents/httpclient/trunk: ./ httpclient-cache/src/main/java/org/apache/http/impl/client/cache/ httpclient-cache/src/test/java/org/apache/http/impl/client/cache/
Date Fri, 09 Aug 2013 18:50:43 GMT
Author: jonm
Date: Fri Aug  9 18:50:43 2013
New Revision: 1512441

URL: http://svn.apache.org/r1512441
Log:
HTTPCLIENT-1373: OPTIONS and TRACE should not invalidate cache
Contributed by James Leigh <james at 3roundstones dot com>

Modified:
    httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java
    httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheInvalidator.java

Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=1512441&r1=1512440&r2=1512441&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Fri Aug  9 18:50:43 2013
@@ -1,6 +1,8 @@
 
 Changes since release 4.3 BETA2
 -------------------
+* [HTTPCLIENT-1373] OPTIONS and TRACE should not invalidate cache
+  Contributed by James Leigh <james at 3roundstones dot com>
 
 * [HTTPCLIENT-1383] HttpClient enters an infinite loop during NTLM authentication if the
opposite 
   endpoint keeps responding with a type 2 NTLM response after type 3 MTLM message has already
been 

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java?rev=1512441&r1=1512440&r2=1512441&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java
(original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/BasicHttpCache.java
Fri Aug  9 18:50:43 2013
@@ -27,9 +27,12 @@
 package org.apache.http.impl.client.cache;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.Date;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -52,6 +55,10 @@ import org.apache.http.protocol.HTTP;
 import org.apache.http.util.EntityUtils;
 
 class BasicHttpCache implements HttpCache {
+    private static final Set<String> safeRequestMethods = new HashSet<String>(
+            Arrays.asList(HeaderConstants.HEAD_METHOD,
+                    HeaderConstants.GET_METHOD, HeaderConstants.OPTIONS_METHOD,
+                    HeaderConstants.TRACE_METHOD));
 
     private final CacheKeyGenerator uriExtractor;
     private final ResourceFactory resourceFactory;
@@ -83,12 +90,16 @@ class BasicHttpCache implements HttpCach
 
     public void flushCacheEntriesFor(final HttpHost host, final HttpRequest request)
             throws IOException {
-        final String uri = uriExtractor.getURI(host, request);
-        storage.removeEntry(uri);
+        if (!safeRequestMethods.contains(request.getRequestLine().getMethod())) {
+            final String uri = uriExtractor.getURI(host, request);
+            storage.removeEntry(uri);
+        }
     }
 
     public void flushInvalidatedCacheEntriesFor(final HttpHost host, final HttpRequest request,
final HttpResponse response) {
-        cacheInvalidator.flushInvalidatedCacheEntries(host, request, response);
+        if (!safeRequestMethods.contains(request.getRequestLine().getMethod())) {
+            cacheInvalidator.flushInvalidatedCacheEntries(host, request, response);
+        }
     }
 
     void storeInCache(

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java?rev=1512441&r1=1512440&r2=1512441&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java
(original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheInvalidator.java
Fri Aug  9 18:50:43 2013
@@ -202,11 +202,19 @@ class CacheInvalidator {
         if (reqURL == null) {
             return;
         }
-        final URL canonURL = getContentLocationURL(reqURL, response);
-        if (canonURL == null) {
-            return;
+        final URL contentLocation = getContentLocationURL(reqURL, response);
+        if (contentLocation != null) {
+            flushLocationCacheEntry(reqURL, response, contentLocation);
+        }
+        final URL location = getLocationURL(reqURL, response);
+        if (location != null) {
+            flushLocationCacheEntry(reqURL, response, location);
         }
-        final String cacheKey = cacheKeyGenerator.canonicalizeUri(canonURL.toString());
+    }
+
+    private void flushLocationCacheEntry(final URL reqURL,
+            final HttpResponse response, final URL location) {
+        final String cacheKey = cacheKeyGenerator.canonicalizeUri(location.toString());
         final HttpCacheEntry entry = getEntry(cacheKey);
         if (entry == null) {
             return;
@@ -222,7 +230,7 @@ class CacheInvalidator {
             return;
         }
 
-        flushUriIfSameHost(reqURL, canonURL);
+        flushUriIfSameHost(reqURL, location);
     }
 
     private URL getContentLocationURL(final URL reqURL, final HttpResponse response) {
@@ -238,6 +246,19 @@ class CacheInvalidator {
         return getRelativeURL(reqURL, contentLocation);
     }
 
+    private URL getLocationURL(final URL reqURL, final HttpResponse response) {
+        final Header clHeader = response.getFirstHeader("Location");
+        if (clHeader == null) {
+            return null;
+        }
+        final String location = clHeader.getValue();
+        final URL canonURL = getAbsoluteURL(location);
+        if (canonURL != null) {
+            return canonURL;
+        }
+        return getRelativeURL(reqURL, location);
+    }
+
     private boolean responseAndEntryEtagsDiffer(final HttpResponse response,
             final HttpCacheEntry entry) {
         final Header entryEtag = entry.getFirstHeader(HeaderConstants.ETAG);

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java?rev=1512441&r1=1512440&r2=1512441&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java
(original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestBasicHttpCache.java
Fri Aug  9 18:50:43 2013
@@ -48,13 +48,19 @@ import org.apache.http.HttpRequest;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
 import org.apache.http.HttpVersion;
+import org.apache.http.client.cache.HeaderConstants;
 import org.apache.http.client.cache.HttpCacheEntry;
 import org.apache.http.client.cache.Resource;
 import org.apache.http.client.methods.HttpDelete;
 import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpHead;
+import org.apache.http.client.methods.HttpOptions;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.methods.HttpTrace;
+import org.apache.http.client.utils.DateUtils;
 import org.apache.http.entity.BasicHttpEntity;
 import org.apache.http.entity.ByteArrayEntity;
-import org.apache.http.client.utils.DateUtils;
+import org.apache.http.message.BasicHeader;
 import org.apache.http.message.BasicHttpResponse;
 import org.apache.http.util.EntityUtils;
 import org.junit.Assert;
@@ -73,6 +79,105 @@ public class TestBasicHttpCache {
     }
 
     @Test
+    public void testDoNotFlushCacheEntriesOnGet() throws Exception {
+        final HttpHost host = new HttpHost("foo.example.com");
+        final HttpRequest req = new HttpGet("/bar");
+        final String key = (new CacheKeyGenerator()).getURI(host, req);
+        final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry();
+
+        backing.map.put(key, entry);
+
+        impl.flushCacheEntriesFor(host, req);
+
+        assertEquals(entry, backing.map.get(key));
+    }
+
+    @Test
+    public void testDoNotFlushCacheEntriesOnHead() throws Exception {
+        final HttpHost host = new HttpHost("foo.example.com");
+        final HttpRequest req = new HttpHead("/bar");
+        final String key = (new CacheKeyGenerator()).getURI(host, req);
+        final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry();
+
+        backing.map.put(key, entry);
+
+        impl.flushCacheEntriesFor(host, req);
+
+        assertEquals(entry, backing.map.get(key));
+    }
+
+    @Test
+    public void testDoNotFlushCacheEntriesOnOptions() throws Exception {
+        final HttpHost host = new HttpHost("foo.example.com");
+        final HttpRequest req = new HttpOptions("/bar");
+        final String key = (new CacheKeyGenerator()).getURI(host, req);
+        final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry();
+
+        backing.map.put(key, entry);
+
+        impl.flushCacheEntriesFor(host, req);
+
+        assertEquals(entry, backing.map.get(key));
+    }
+
+    @Test
+    public void testDoNotFlushCacheEntriesOnTrace() throws Exception {
+        final HttpHost host = new HttpHost("foo.example.com");
+        final HttpRequest req = new HttpTrace("/bar");
+        final String key = (new CacheKeyGenerator()).getURI(host, req);
+        final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry();
+
+        backing.map.put(key, entry);
+
+        impl.flushCacheEntriesFor(host, req);
+
+        assertEquals(entry, backing.map.get(key));
+    }
+
+    @Test
+    public void testFlushContentLocationEntryIfUnSafeRequest()
+            throws Exception {
+        final HttpHost host = new HttpHost("foo.example.com");
+        final HttpRequest req = new HttpPost("/foo");
+        final HttpResponse resp = HttpTestUtils.make200Response();
+        resp.setHeader("Content-Location", "/bar");
+        resp.setHeader(HeaderConstants.ETAG, "\"etag\"");
+        final String key = (new CacheKeyGenerator()).getURI(host, new HttpGet("/bar"));
+
+        final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(new Header[] {
+           new BasicHeader("Date", DateUtils.formatDate(new Date())),
+           new BasicHeader("ETag", "\"old-etag\"")
+        });
+
+        backing.map.put(key, entry);
+
+        impl.flushInvalidatedCacheEntriesFor(host, req, resp);
+
+        assertNull(backing.map.get(key));
+    }
+
+    @Test
+    public void testDoNotFlushContentLocationEntryIfSafeRequest()
+            throws Exception {
+        final HttpHost host = new HttpHost("foo.example.com");
+        final HttpRequest req = new HttpGet("/foo");
+        final HttpResponse resp = HttpTestUtils.make200Response();
+        resp.setHeader("Content-Location", "/bar");
+        final String key = (new CacheKeyGenerator()).getURI(host, new HttpGet("/bar"));
+
+        final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(new Header[] {
+           new BasicHeader("Date", DateUtils.formatDate(new Date())),
+           new BasicHeader("ETag", "\"old-etag\"")
+        });
+
+        backing.map.put(key, entry);
+
+        impl.flushInvalidatedCacheEntriesFor(host, req, resp);
+
+        assertEquals(entry, backing.map.get(key));
+    }
+
+    @Test
     public void testCanFlushCacheEntriesAtUri() throws Exception {
         final HttpHost host = new HttpHost("foo.example.com");
         final HttpRequest req = new HttpDelete("/bar");

Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheInvalidator.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheInvalidator.java?rev=1512441&r1=1512440&r2=1512441&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheInvalidator.java
(original)
+++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheInvalidator.java
Fri Aug  9 18:50:43 2013
@@ -51,6 +51,9 @@ import org.apache.http.message.BasicHead
 import org.apache.http.message.BasicHttpEntityEnclosingRequest;
 import org.apache.http.message.BasicHttpRequest;
 import org.apache.http.message.BasicHttpResponse;
+import org.easymock.EasyMock;
+import org.easymock.IAnswer;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -298,6 +301,28 @@ public class TestCacheInvalidator {
     }
 
     @Test
+    public void flushesEntryIfFresherAndSpecifiedByLocation()
+            throws Exception {
+        response.setStatusCode(201);
+        response.setHeader("ETag","\"new-etag\"");
+        response.setHeader("Date", DateUtils.formatDate(now));
+        final String theURI = "http://foo.example.com:80/bar";
+        response.setHeader("Location", theURI);
+
+        final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry(new Header[] {
+           new BasicHeader("Date", DateUtils.formatDate(tenSecondsAgo)),
+           new BasicHeader("ETag", "\"old-etag\"")
+        });
+
+        expect(mockStorage.getEntry(theURI)).andReturn(entry).anyTimes();
+        mockStorage.removeEntry(theURI);
+
+        replayMocks();
+        impl.flushInvalidatedCacheEntries(host, request, response);
+        verifyMocks();
+    }
+
+    @Test
     public void doesNotFlushEntryForUnsuccessfulResponse()
             throws Exception {
         response = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_BAD_REQUEST,
"Bad Request");
@@ -312,6 +337,13 @@ public class TestCacheInvalidator {
         });
 
         expect(mockStorage.getEntry(theURI)).andReturn(entry).anyTimes();
+        mockStorage.removeEntry(theURI);
+        EasyMock.expectLastCall().andAnswer(new IAnswer<Void>() {
+            public Void answer() {
+                Assert.fail();
+                return null;
+              }
+          }).anyTimes();
 
         replayMocks();
         impl.flushInvalidatedCacheEntries(host, request, response);
@@ -374,6 +406,13 @@ public class TestCacheInvalidator {
         });
 
         expect(mockStorage.getEntry(cacheKey)).andReturn(entry).anyTimes();
+        mockStorage.removeEntry(cacheKey);
+        EasyMock.expectLastCall().andAnswer(new IAnswer<Void>() {
+            public Void answer() {
+                Assert.fail();
+                return null;
+              }
+          }).anyTimes();
 
         replayMocks();
         impl.flushInvalidatedCacheEntries(host, request, response);
@@ -396,6 +435,13 @@ public class TestCacheInvalidator {
         });
 
         expect(mockStorage.getEntry(theURI)).andReturn(entry).anyTimes();
+        mockStorage.removeEntry(theURI);
+        EasyMock.expectLastCall().andAnswer(new IAnswer<Void>() {
+            public Void answer() {
+                Assert.fail();
+                return null;
+              }
+          }).anyTimes();
 
         replayMocks();
         impl.flushInvalidatedCacheEntries(host, request, response);
@@ -416,6 +462,13 @@ public class TestCacheInvalidator {
         });
 
         expect(mockStorage.getEntry(theURI)).andReturn(entry).anyTimes();
+        mockStorage.removeEntry(theURI);
+        EasyMock.expectLastCall().andAnswer(new IAnswer<Void>() {
+            public Void answer() {
+                Assert.fail();
+                return null;
+              }
+          }).anyTimes();
 
         replayMocks();
         impl.flushInvalidatedCacheEntries(host, request, response);
@@ -431,6 +484,13 @@ public class TestCacheInvalidator {
         response.setHeader("Content-Location", theURI);
 
         expect(mockStorage.getEntry(theURI)).andReturn(null).anyTimes();
+        mockStorage.removeEntry(theURI);
+        EasyMock.expectLastCall().andAnswer(new IAnswer<Void>() {
+            public Void answer() {
+                Assert.fail();
+                return null;
+              }
+          }).anyTimes();
 
         replayMocks();
         impl.flushInvalidatedCacheEntries(host, request, response);
@@ -451,6 +511,13 @@ public class TestCacheInvalidator {
         });
 
         expect(mockStorage.getEntry(theURI)).andReturn(entry).anyTimes();
+        mockStorage.removeEntry(theURI);
+        EasyMock.expectLastCall().andAnswer(new IAnswer<Void>() {
+            public Void answer() {
+                Assert.fail();
+                return null;
+              }
+          }).anyTimes();
 
         replayMocks();
         impl.flushInvalidatedCacheEntries(host, request, response);



Mime
View raw message