From 8151d9e51a7e06051f2c561ff04a575261acf46d Mon Sep 17 00:00:00 2001
From: dirkhenselin <dirk.henselin@vwgis.de>
Date: Tue, 18 Aug 2020 12:24:22 +0200
Subject: [PATCH] Avoid updating Content-Length header in a 304 response.

I observed the following problem: `Transfer-Encoding` and
`Content-Length` headers should be mutually exclusive and because I use
chunked transfer, the `Transfer-Encoding` header is set in the response
while the `Content-Length` header is not. In case of a 304 during a
revalidation, the header contains Content-Length=0. Probably a proxy is
responsible for this, just like the comment "Some well-known proxies
respond with Content-Length=0, when returning 304" in the method
CachedHttpResponseGenerator::addMissingContentLengthHeader is saying. In
CacheEntryUpdater::mergeHeaders the Content-Length=0 is merged into the
cached entry, but the cached entry contains also a `Transfer-Encoding`
header, so in the cached entry these headers aren't mutually exclusive
anymore. Because of the `Transfer-Encoding` header the method
CachedHttpResponseGenerator::addMissingContentLengthHeader isn't fixing
the `Content-Length` header and Content-Length=0 causes returning null
instead of the cached content. IMHO the `Content-Length` header should
not be merged into the cached response in case of a 304, at least if the
cached entry contains a `Transfer-Encoding` header.
---
 .../impl/client/cache/CacheEntryUpdater.java  | 10 +++++----
 .../client/cache/TestCacheEntryUpdater.java   | 21 +++++++++++++++++++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheEntryUpdater.java b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheEntryUpdater.java
index f18c73cbe6..e5a57cc48f 100644
--- a/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheEntryUpdater.java
+++ b/httpclient-cache/src/main/java/org/apache/http/impl/client/cache/CacheEntryUpdater.java
@@ -111,8 +111,9 @@ && entryDateHeaderNewerThenResponse(entry, response)) {
         // Remove cache headers that match response
         for (final HeaderIterator it = response.headerIterator(); it.hasNext(); ) {
             final Header responseHeader = it.nextHeader();
-            // Since we do not expect a content in a 304 response, should retain the original Content-Encoding header
-            if (HTTP.CONTENT_ENCODING.equals(responseHeader.getName())) {
+            // Since we do not expect a content in a 304 response, should retain the original Content-Encoding and Content-Length header
+            if (HTTP.CONTENT_ENCODING.equals(responseHeader.getName())
+                    || HTTP.CONTENT_LEN.equals(responseHeader.getName())) {
                 continue;
             }
             final Header[] matchingHeaders = headerGroup.getHeaders(responseHeader.getName());
@@ -133,8 +134,9 @@ && entryDateHeaderNewerThenResponse(entry, response)) {
         }
         for (final HeaderIterator it = response.headerIterator(); it.hasNext(); ) {
             final Header responseHeader = it.nextHeader();
-            // Since we do not expect a content in a 304 response, should avoid updating Content-Encoding header
-            if (HTTP.CONTENT_ENCODING.equals(responseHeader.getName())) {
+            // Since we do not expect a content in a 304 response, should avoid updating Content-Encoding and Content-Length header
+            if (HTTP.CONTENT_ENCODING.equals(responseHeader.getName())
+                    || HTTP.CONTENT_LEN.equals(responseHeader.getName())) {
                 continue;
             }
             headerGroup.addHeader(responseHeader);
diff --git a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheEntryUpdater.java b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheEntryUpdater.java
index a0a873742d..0f0ea51c0c 100644
--- a/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheEntryUpdater.java
+++ b/httpclient-cache/src/test/java/org/apache/http/impl/client/cache/TestCacheEntryUpdater.java
@@ -260,6 +260,27 @@ public void testContentEncodingHeaderIsNotUpdatedByMerge() throws IOException {
         headersNotContain(updatedHeaders, "Content-Encoding", "gzip");
     }
 
+    @Test
+    public void testContentLengthIsNotAddedWhenTransferEncodingIsPresent() throws IOException {
+        final Header[] headers = {
+                new BasicHeader("Date", DateUtils.formatDate(requestDate)),
+                new BasicHeader("ETag", "\"etag\""),
+                new BasicHeader("Transfer-Encoding", "chunked")};
+
+        entry = HttpTestUtils.makeCacheEntry(headers);
+        response.setHeaders(new Header[]{
+                new BasicHeader("Last-Modified", DateUtils.formatDate(responseDate)),
+                new BasicHeader("Cache-Control", "public"),
+                new BasicHeader("Content-Length", "0")});
+
+        final HttpCacheEntry updatedEntry = impl.updateCacheEntry(null, entry,
+                new Date(), new Date(), response);
+
+        final Header[] updatedHeaders = updatedEntry.getAllHeaders();
+        headersContain(updatedHeaders, "Transfer-Encoding", "chunked");
+        headersNotContain(updatedHeaders, "Content-Length", "0");
+    }
+
     private void headersContain(final Header[] headers, final String name, final String value) {
         for (final Header header : headers) {
             if (header.getName().equals(name)) {