[httpcomponents-client] 01/04: Avoid updating Content-Length header in a 304 response.

[ol...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch 5.1.x
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git

commit 32d8e68b8da00fbd74026e73f0c758daabda55ae
Author: dirkhenselin <di...@vwgis.de>
AuthorDate: Tue Aug 18 12:24:22 2020 +0200

    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.
---
 .../client5/http/impl/cache/CacheUpdateHandler.java  |  6 ++++--
 .../http/impl/cache/TestCacheUpdateHandler.java      | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheUpdateHandler.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheUpdateHandler.java
index d0ea32c..2302148 100644
--- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheUpdateHandler.java
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheUpdateHandler.java
@@ -141,7 +141,8 @@ class CacheUpdateHandler {
         for (final Iterator<Header> it = response.headerIterator(); it.hasNext(); ) {
             final Header responseHeader = it.next();
             // Since we do not expect a content in a 304 response, should retain the original Content-Encoding header
-            if (HttpHeaders.CONTENT_ENCODING.equals(responseHeader.getName())) {
+            if (HttpHeaders.CONTENT_ENCODING.equals(responseHeader.getName())
+                    || HttpHeaders.CONTENT_LENGTH.equals(responseHeader.getName())) {
                 continue;
             }
             headerGroup.removeHeaders(responseHeader.getName());
@@ -159,7 +160,8 @@ class CacheUpdateHandler {
         for (final Iterator<Header> it = response.headerIterator(); it.hasNext(); ) {
             final Header responseHeader = it.next();
             // Since we do not expect a content in a 304 response, should update the cache entry with Content-Encoding
-            if (HttpHeaders.CONTENT_ENCODING.equals(responseHeader.getName())) {
+            if (HttpHeaders.CONTENT_ENCODING.equals(responseHeader.getName())
+                    || HttpHeaders.CONTENT_LENGTH.equals(responseHeader.getName())) {
                 continue;
             }
             headerGroup.addHeader(responseHeader);
diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheUpdateHandler.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheUpdateHandler.java
index 39816c0..26b441f 100644
--- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheUpdateHandler.java
+++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheUpdateHandler.java
@@ -266,6 +266,26 @@ public class TestCacheUpdateHandler {
         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 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.getHeaders();
+        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)) {