Return-Path: X-Original-To: apmail-hc-commits-archive@www.apache.org Delivered-To: apmail-hc-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 4BBC410A90 for ; Fri, 9 Aug 2013 18:51:10 +0000 (UTC) Received: (qmail 15124 invoked by uid 500); 9 Aug 2013 18:51:10 -0000 Delivered-To: apmail-hc-commits-archive@hc.apache.org Received: (qmail 15065 invoked by uid 500); 9 Aug 2013 18:51:06 -0000 Mailing-List: contact commits-help@hc.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "HttpComponents Project" Delivered-To: mailing list commits@hc.apache.org Received: (qmail 15053 invoked by uid 99); 9 Aug 2013 18:51:05 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 09 Aug 2013 18:51:05 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 09 Aug 2013 18:51:03 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id A714123888CD for ; Fri, 9 Aug 2013 18:50:43 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit 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 -0000 To: commits@hc.apache.org From: jonm@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20130809185043.A714123888CD@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org 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 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 * [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 safeRequestMethods = new HashSet( + 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() { + 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() { + 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() { + 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() { + 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() { + 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() { + public Void answer() { + Assert.fail(); + return null; + } + }).anyTimes(); replayMocks(); impl.flushInvalidatedCacheEntries(host, request, response);