You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by jo...@apache.org on 2013/08/09 20:50:43 UTC
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/
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);