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 2E3CDEAF9 for ; Tue, 8 Jan 2013 12:16:27 +0000 (UTC) Received: (qmail 64715 invoked by uid 500); 8 Jan 2013 12:16:27 -0000 Delivered-To: apmail-hc-commits-archive@hc.apache.org Received: (qmail 64642 invoked by uid 500); 8 Jan 2013 12:16:25 -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 64611 invoked by uid 99); 8 Jan 2013 12:16:25 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 08 Jan 2013 12:16:25 +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; Tue, 08 Jan 2013 12:16:21 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 2CCC823889ED for ; Tue, 8 Jan 2013 12:16:01 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1430245 - 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: Tue, 08 Jan 2013 12:16:00 -0000 To: commits@hc.apache.org From: olegk@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20130108121601.2CCC823889ED@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: olegk Date: Tue Jan 8 12:16:00 2013 New Revision: 1430245 URL: http://svn.apache.org/viewvc?rev=1430245&view=rev Log: HTTPCLIENT-1290: 304 cached response never reused with If-modified-since conditional requests Contributed by Francois-Xavier Bonnet Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=1430245&r1=1430244&r2=1430245&view=diff ============================================================================== --- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original) +++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Tue Jan 8 12:16:00 2013 @@ -1,6 +1,10 @@ Changes in trunk ------------------- +* [HTTPCLIENT-1290] 304 cached response never reused with If-modified-since conditional + requests. + Contributed by Francois-Xavier Bonnet + * [HTTPCLIENT-1291] Absolute request URIs without an explicitly specified path are rewritten to have "/" path). Contributed by Oleg Kalnichevski @@ -20,8 +24,8 @@ Changes in trunk route differs from the host name specified in the request URI. Contributed by Oleg Kalnichevski -* Kerberos and SPNego auth schemes use incorrect authorization header name when authenticating - with a proxy. +* [HTTPCLIENT-1293] Kerberos and SPNego auth schemes use incorrect authorization header name + when authenticating with a proxy. Contributed by Oleg Kalnichevski * [HTTPCLIENT-1283] NTLM needs to use Locale-independent form of Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java?rev=1430245&r1=1430244&r2=1430245&view=diff ============================================================================== --- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java (original) +++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingExec.java Tue Jan 8 12:16:00 2013 @@ -800,6 +800,7 @@ public class CachingExec implements Clie responseCache.flushInvalidatedCacheEntriesFor(target, request, backendResponse); if (cacheable && !alreadyHaveNewerCacheEntry(target, request, backendResponse)) { try { + storeRequestIfModifiedSinceFor304Response(request, backendResponse); return Proxies.enhanceResponse(responseCache.cacheAndReturnResponse( target, request, backendResponse, requestDate, responseDate)); } finally { @@ -816,6 +817,24 @@ public class CachingExec implements Clie return backendResponse; } + /** + * For 304 Not modified responses, adds a "Last-Modified" header with the + * value of the "If-Modified-Since" header passed in the request. This + * header is required to be able to reuse match the cache entry for + * subsequent requests but as defined in http specifications it is not + * included in 304 responses by backend servers. This header will not be + * included in the resulting response. + */ + private void storeRequestIfModifiedSinceFor304Response( + HttpRequest request, HttpResponse backendResponse) { + if (backendResponse.getStatusLine().getStatusCode() == HttpStatus.SC_NOT_MODIFIED) { + Header h = request.getFirstHeader("If-Modified-Since"); + if (h != null) { + backendResponse.addHeader("Last-Modified", h.getValue()); + } + } + } + private boolean alreadyHaveNewerCacheEntry(HttpHost target, HttpRequestWrapper request, HttpResponse backendResponse) { HttpCacheEntry existing = null; Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java?rev=1430245&r1=1430244&r2=1430245&view=diff ============================================================================== --- httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java (original) +++ httpcomponents/httpclient/trunk/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CachingHttpClient.java Tue Jan 8 12:16:00 2013 @@ -928,6 +928,7 @@ public class CachingHttpClient implement if (cacheable && !alreadyHaveNewerCacheEntry(target, request, backendResponse)) { try { + storeRequestIfModifiedSinceFor304Response(request, backendResponse); return responseCache.cacheAndReturnResponse(target, request, backendResponse, requestDate, responseDate); } catch (IOException ioe) { @@ -944,7 +945,25 @@ public class CachingHttpClient implement return backendResponse; } - private boolean alreadyHaveNewerCacheEntry(HttpHost target, HttpRequestWrapper request, + /** + * For 304 Not modified responses, adds a "Last-Modified" header with the + * value of the "If-Modified-Since" header passed in the request. This + * header is required to be able to reuse match the cache entry for + * subsequent requests but as defined in http specifications it is not + * included in 304 responses by backend servers. This header will not be + * included in the resulting response. + */ + private void storeRequestIfModifiedSinceFor304Response( + HttpRequest request, HttpResponse backendResponse) { + if (backendResponse.getStatusLine().getStatusCode() == HttpStatus.SC_NOT_MODIFIED) { + Header h = request.getFirstHeader("If-Modified-Since"); + if (h != null) { + backendResponse.addHeader("Last-Modified", h.getValue()); + } + } + } + + private boolean alreadyHaveNewerCacheEntry(HttpHost target, HttpRequest request, HttpResponse backendResponse) { HttpCacheEntry existing = null; try { Modified: httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java?rev=1430245&r1=1430244&r2=1430245&view=diff ============================================================================== --- httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java (original) +++ httpcomponents/httpclient/trunk/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCachingExec.java Tue Jan 8 12:16:00 2013 @@ -52,6 +52,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import junit.framework.AssertionFailedError; + import org.apache.http.Header; import org.apache.http.HttpHost; import org.apache.http.HttpRequest; @@ -740,6 +742,43 @@ public class TestCachingExec { } @Test + public void testReturns304ForIfModifiedSinceHeaderIf304ResponseInCache() throws Exception { + Date now = new Date(); + Date oneHourAgo = new Date(now.getTime() - 3600 * 1000L); + Date inTenMinutes = new Date(now.getTime() + 600 * 1000L); + impl = new CachingExec(mockBackend, new BasicHttpCache(), CacheConfig.DEFAULT); + HttpRequestWrapper req1 = HttpRequestWrapper.wrap(new HttpGet("http://foo.example.com/")); + req1.addHeader("If-Modified-Since", DateUtils.formatDate(oneHourAgo)); + HttpRequestWrapper req2 = HttpRequestWrapper.wrap(new HttpGet("http://foo.example.com/")); + req2.addHeader("If-Modified-Since", DateUtils.formatDate(oneHourAgo)); + + HttpResponse resp1 = new BasicHttpResponse(HttpVersion.HTTP_1_1, HttpStatus.SC_NOT_MODIFIED, "Not modified"); + resp1.setHeader("Date", DateUtils.formatDate(now)); + resp1.setHeader("Cache-control", "max-age=600"); + resp1.setHeader("Expires", DateUtils.formatDate(inTenMinutes)); + + expect(mockBackend.execute( + same(route), + isA(HttpRequestWrapper.class), + isA(HttpClientContext.class), + (HttpExecutionAware) isNull())).andReturn(Proxies.enhanceResponse(resp1)).once(); + + expect(mockBackend.execute( + same(route), + isA(HttpRequestWrapper.class), + isA(HttpClientContext.class), + (HttpExecutionAware) isNull())).andThrow( + new AssertionFailedError("Should have reused cached 304 response")).anyTimes(); + + replayMocks(); + impl.execute(route, req1); + HttpResponse result = impl.execute(route, req2); + verifyMocks(); + Assert.assertEquals(HttpStatus.SC_NOT_MODIFIED, result.getStatusLine().getStatusCode()); + Assert.assertFalse(result.containsHeader("Last-Modified")); + } + + @Test public void testReturns200ForIfModifiedSinceDateIsLess() throws Exception { Date now = new Date(); Date tenSecondsAgo = new Date(now.getTime() - 10 * 1000L);