From b9e2bbc7783827d0e333e2af3be45592ce909da0 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Sat, 22 Apr 2023 21:21:31 +0200 Subject: [PATCH] Add Last-Modified header to 304 response when ETag is not present . This commit adds the Last-Modified header to the 304 Not Modified response when the ETag header is not present in the cache entry. This aligns the behavior with the recommendations in RFC 7232 and helps clients that rely on the Last-Modified header for cache updates when --- .../cache/CachedHttpResponseGenerator.java | 12 +++++++++ .../http/impl/cache/TestCachingExecChain.java | 27 +++++++++++++++++++ .../cache/TestProtocolRecommendations.java | 1 + 3 files changed, 40 insertions(+) diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedHttpResponseGenerator.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedHttpResponseGenerator.java index ff766136a..c827fdad0 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedHttpResponseGenerator.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedHttpResponseGenerator.java @@ -139,6 +139,18 @@ class CachedHttpResponseGenerator { response.addHeader(varyHeader); } + //Since the goal of a 304 response is to minimize information transfer + //when the recipient already has one or more cached representations, a + //sender SHOULD NOT generate representation metadata other than the + //above listed fields unless said metadata exists for the purpose of + //guiding cache updates (e.g., Last-Modified might be useful if the + //response does not have an ETag field). + if (etagHeader == null) { + final Header lastModifiedHeader = entry.getFirstHeader(HttpHeaders.LAST_MODIFIED); + if (lastModifiedHeader != null) { + response.addHeader(lastModifiedHeader); + } + } return response; } diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java index 7f1d3801c..d8bba3720 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java @@ -391,6 +391,7 @@ public class TestCachingExecChain { resp1.setHeader("Date", DateUtils.formatStandardDate(now)); resp1.setHeader("Cache-control", "max-age=600"); resp1.setHeader("Expires", DateUtils.formatStandardDate(inTenMinutes)); + resp1.setHeader("ETag", "\"etag\""); Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1); @@ -403,6 +404,32 @@ public class TestCachingExecChain { Mockito.verify(mockExecChain).proceed(Mockito.any(), Mockito.any()); } + @Test + public void testReturns304ForIfModifiedSinceHeaderIf304ResponseInCacheWithLastModified() throws Exception { + final Instant now = Instant.now(); + final Instant oneHourAgo = now.minus(1, ChronoUnit.HOURS); + final Instant inTenMinutes = now.plus(10, ChronoUnit.MINUTES); + final ClassicHttpRequest req1 = new HttpGet("http://foo.example.com/"); + req1.addHeader("If-Modified-Since", DateUtils.formatStandardDate(oneHourAgo)); + final ClassicHttpRequest req2 = new HttpGet("http://foo.example.com/"); + req2.addHeader("If-Modified-Since", DateUtils.formatStandardDate(oneHourAgo)); + + final ClassicHttpResponse resp1 = HttpTestUtils.make304Response(); + resp1.setHeader("Date", DateUtils.formatStandardDate(now)); + resp1.setHeader("Cache-control", "max-age=600"); + resp1.setHeader("Expires", DateUtils.formatStandardDate(inTenMinutes)); + + Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1); + + execute(req1); + + final ClassicHttpResponse result = execute(req2); + Assertions.assertEquals(HttpStatus.SC_NOT_MODIFIED, result.getCode()); + Assertions.assertTrue(result.containsHeader("Last-Modified")); + + Mockito.verify(mockExecChain).proceed(Mockito.any(), Mockito.any()); + } + @Test public void testReturns200ForIfModifiedSinceDateIsLess() throws Exception { final Instant now = Instant.now(); diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRecommendations.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRecommendations.java index a12de54e4..c1507bb65 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRecommendations.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRecommendations.java @@ -175,6 +175,7 @@ public class TestProtocolRecommendations { resp1.setHeader("Cache-Control","max-age=3600"); resp1.setHeader(validatorHeader, validator); resp1.setHeader(headerName, headerValue); + resp1.setHeader("ETag", "\"etag\""); Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1);