HTTPCLIENT-1373: OPTIONS and TRACE should not invalidate cache
Contributed by James Leigh <james at 3roundstones dot com> git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1512441 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
parent
888b783538
commit
0364f0156b
|
@ -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
|
||||
|
|
|
@ -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,13 +90,17 @@ class BasicHttpCache implements HttpCache {
|
|||
|
||||
public void flushCacheEntriesFor(final HttpHost host, final HttpRequest request)
|
||||
throws IOException {
|
||||
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) {
|
||||
if (!safeRequestMethods.contains(request.getRequestLine().getMethod())) {
|
||||
cacheInvalidator.flushInvalidatedCacheEntries(host, request, response);
|
||||
}
|
||||
}
|
||||
|
||||
void storeInCache(
|
||||
final HttpHost target, final HttpRequest request, final HttpCacheEntry entry) throws IOException {
|
||||
|
|
|
@ -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 String cacheKey = cacheKeyGenerator.canonicalizeUri(canonURL.toString());
|
||||
final URL location = getLocationURL(reqURL, response);
|
||||
if (location != null) {
|
||||
flushLocationCacheEntry(reqURL, response, location);
|
||||
}
|
||||
}
|
||||
|
||||
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);
|
||||
|
|
|
@ -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;
|
||||
|
@ -72,6 +78,105 @@ public class TestBasicHttpCache {
|
|||
impl = new BasicHttpCache(new HeapResourceFactory(), backing, CacheConfig.DEFAULT);
|
||||
}
|
||||
|
||||
@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");
|
||||
|
|
|
@ -51,6 +51,9 @@ import org.apache.http.message.BasicHeader;
|
|||
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;
|
||||
|
||||
|
@ -297,6 +300,28 @@ public class TestCacheInvalidator {
|
|||
verifyMocks();
|
||||
}
|
||||
|
||||
@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 {
|
||||
|
@ -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);
|
||||
|
|
Loading…
Reference in New Issue