From 65ec9a8e7a447cfd2a9dffdabcafdb8be1f70c62 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Fri, 9 Jun 2023 15:50:39 +0200 Subject: [PATCH] HTTPCLIENT-2277: do not store hop-by-hop and connection specific headers in cache (RFC 9111 3.1) --- .../http/cache/CacheHeaderSupport.java | 93 +++++++++++++++++++ .../hc/client5/http/cache/HttpCacheEntry.java | 12 ++- .../http/impl/cache/CacheUpdateHandler.java | 11 ++- .../http/impl/cache/CacheValidityPolicy.java | 26 ------ .../cache/CachedHttpResponseGenerator.java | 10 +- .../CachedResponseSuitabilityChecker.java | 5 - .../http/cache/TestCacheHeaderSupport.java | 67 +++++++++++++ .../http/impl/cache/HttpTestUtils.java | 40 +------- .../impl/cache/TestCacheUpdateHandler.java | 19 ---- .../impl/cache/TestCacheValidityPolicy.java | 28 ------ .../TestCachedHttpResponseGenerator.java | 26 ------ .../TestCachedResponseSuitabilityChecker.java | 74 +++------------ .../impl/cache/TestProtocolRequirements.java | 58 ------------ 13 files changed, 197 insertions(+), 272 deletions(-) create mode 100644 httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/CacheHeaderSupport.java create mode 100644 httpclient5-cache/src/test/java/org/apache/hc/client5/http/cache/TestCacheHeaderSupport.java diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/CacheHeaderSupport.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/CacheHeaderSupport.java new file mode 100644 index 000000000..0cf6ae72a --- /dev/null +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/CacheHeaderSupport.java @@ -0,0 +1,93 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ +package org.apache.hc.client5.http.cache; + +import java.util.Collections; +import java.util.Set; +import java.util.TreeSet; + +import org.apache.hc.core5.annotation.Internal; +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HeaderElements; +import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.MessageHeaders; +import org.apache.hc.core5.http.message.MessageSupport; + +@Internal +public final class CacheHeaderSupport { + + private final static Set HOP_BY_HOP; + + static { + final TreeSet set = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + set.add(HttpHeaders.CONNECTION); + set.add(HttpHeaders.CONTENT_LENGTH); + set.add(HttpHeaders.TRANSFER_ENCODING); + set.add(HttpHeaders.HOST); + set.add(HttpHeaders.KEEP_ALIVE); + set.add(HttpHeaders.TE); + set.add(HttpHeaders.UPGRADE); + set.add(HttpHeaders.PROXY_AUTHORIZATION); + set.add("Proxy-Authentication-Info"); + set.add(HttpHeaders.PROXY_AUTHENTICATE); + HOP_BY_HOP = Collections.unmodifiableSet(set); + } + + public static boolean isHopByHop(final String headerName) { + if (headerName == null) { + return false; + } + return HOP_BY_HOP.contains(headerName); + } + + public static boolean isHopByHop(final Header header) { + if (header == null) { + return false; + } + return isHopByHop(header.getName()); + } + + /** + * This method should be provided by the core + */ + public static Set hopByHopConnectionSpecific(final MessageHeaders headers) { + final Header connectionHeader = headers.getFirstHeader(HttpHeaders.CONNECTION); + final String connDirective = connectionHeader != null ? connectionHeader.getValue() : null; + // Disregard most common 'Close' and 'Keep-Alive' tokens + if (connDirective != null && + !connDirective.equalsIgnoreCase(HeaderElements.CLOSE) && + !connDirective.equalsIgnoreCase(HeaderElements.KEEP_ALIVE)) { + final TreeSet result = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + result.addAll(HOP_BY_HOP); + result.addAll(MessageSupport.parseTokens(connectionHeader)); + return result; + } else { + return HOP_BY_HOP; + } + } + +} diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntry.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntry.java index c412feee6..f3a6ca9a9 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntry.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HttpCacheEntry.java @@ -32,7 +32,9 @@ import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.Iterator; +import java.util.Locale; import java.util.Map; +import java.util.Set; import org.apache.hc.client5.http.utils.DateUtils; import org.apache.hc.core5.annotation.Contract; @@ -98,16 +100,22 @@ public class HttpCacheEntry implements MessageHeaders, Serializable { Args.notNull(request, "Request"); Args.notNull(response, "Origin response"); + final Set requestHopByHop = CacheHeaderSupport.hopByHopConnectionSpecific(request); final HeaderGroup requestHeaders = new HeaderGroup(); for (final Iterator
it = request.headerIterator(); it.hasNext(); ) { final Header header = it.next(); - requestHeaders.addHeader(header); + if (!requestHopByHop.contains(header.getName().toLowerCase(Locale.ROOT))) { + requestHeaders.addHeader(header); + } } + final Set responseHopByHop = CacheHeaderSupport.hopByHopConnectionSpecific(request); final HeaderGroup responseHeaders = new HeaderGroup(); for (final Iterator
it = response.headerIterator(); it.hasNext(); ) { final Header header = it.next(); - responseHeaders.addHeader(header); + if (!responseHopByHop.contains(header.getName().toLowerCase(Locale.ROOT))) { + responseHeaders.addHeader(header); + } } return new HttpCacheEntry( 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 4bb23e62f..0f0677322 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 @@ -29,8 +29,11 @@ package org.apache.hc.client5.http.impl.cache; import java.time.Instant; import java.util.HashMap; import java.util.Iterator; +import java.util.Locale; import java.util.Map; +import java.util.Set; +import org.apache.hc.client5.http.cache.CacheHeaderSupport; import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.client5.http.cache.Resource; import org.apache.hc.client5.http.cache.ResourceFactory; @@ -156,8 +159,6 @@ class CacheUpdateHandler { // Since we do not expect a content in a 304 response, should retain the original Content-Encoding header if (headerName.equalsIgnoreCase(HttpHeaders.CONTENT_ENCODING)) { headerGroup.addHeader(entryHeader); - } else if (headerName.equalsIgnoreCase(HttpHeaders.CONTENT_LENGTH)) { - headerGroup.addHeader(entryHeader); } else if (headerName.equalsIgnoreCase(HttpHeaders.WARNING)) { // remove cache entry 1xx warnings final String warningValue = entryHeader.getValue(); @@ -165,18 +166,18 @@ class CacheUpdateHandler { headerGroup.addHeader(entryHeader); } } else { - // drop headers present in the response if (!response.containsHeader(headerName)) { headerGroup.addHeader(entryHeader); } } } + final Set responseHopByHop = CacheHeaderSupport.hopByHopConnectionSpecific(response); for (final Iterator
it = response.headerIterator(); it.hasNext(); ) { final Header responseHeader = it.next(); final String headerName = responseHeader.getName(); // Since we do not expect a content in a 304 response, should update the cache entry with Content-Encoding - if (!headerName.equalsIgnoreCase(HttpHeaders.CONTENT_ENCODING) - && !headerName.equalsIgnoreCase(HttpHeaders.CONTENT_LENGTH)) { + if (!headerName.equalsIgnoreCase(HttpHeaders.CONTENT_ENCODING) && + !responseHopByHop.contains(headerName.toLowerCase(Locale.ROOT))) { headerGroup.addHeader(responseHeader); } } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java index 633506a02..1f39716e8 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheValidityPolicy.java @@ -30,7 +30,6 @@ import java.time.Duration; import java.time.Instant; import org.apache.hc.client5.http.cache.HttpCacheEntry; -import org.apache.hc.client5.http.cache.Resource; import org.apache.hc.client5.http.utils.DateUtils; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpHeaders; @@ -138,31 +137,6 @@ class CacheValidityPolicy { staleness.compareTo(TimeValue.ofSeconds(responseCacheControl.getStaleIfError())) <= 0; } - /** - * This matters for deciding whether the cache entry is valid to serve as a - * response. If these values do not match, we might have a partial response - * - * @param entry The cache entry we are currently working with - * @return boolean indicating whether actual length matches Content-Length - */ - protected boolean contentLengthHeaderMatchesActualLength(final HttpCacheEntry entry) { - final Header h = entry.getFirstHeader(HttpHeaders.CONTENT_LENGTH); - if (h != null) { - try { - final long responseLen = Long.parseLong(h.getValue()); - final Resource resource = entry.getResource(); - if (resource == null) { - return false; - } - final long resourceLen = resource.length(); - return responseLen == resourceLen; - } catch (final NumberFormatException ex) { - return false; - } - } - return true; - } - protected TimeValue getApparentAge(final HttpCacheEntry entry) { final Instant dateValue = entry.getInstant(); if (dateValue == null) { 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 f9c27ff13..a17cc49b2 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 @@ -75,7 +75,7 @@ class CachedHttpResponseGenerator { final Header h = entry.getFirstHeader(HttpHeaders.CONTENT_TYPE); final ContentType contentType = h != null ? ContentType.parse(h.getValue()) : null; final byte[] content = resource.get(); - addMissingContentLengthHeader(response, content); + generateContentLength(response, content); response.setBody(content, contentType); } @@ -154,12 +154,8 @@ class CachedHttpResponseGenerator { return response; } - private void addMissingContentLengthHeader(final HttpResponse response, final byte[] body) { - if (transferEncodingIsPresent(response)) { - return; - } - // Some well known proxies respond with Content-Length=0, when returning 304. For robustness, always - // use the cached entity's content length, as modern browsers do. + private void generateContentLength(final HttpResponse response, final byte[] body) { + response.removeHeaders(HttpHeaders.TRANSFER_ENCODING); response.setHeader(HttpHeaders.CONTENT_LENGTH, Integer.toString(body.length)); } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedResponseSuitabilityChecker.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedResponseSuitabilityChecker.java index e63cd7da7..3c27768f5 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedResponseSuitabilityChecker.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachedResponseSuitabilityChecker.java @@ -118,11 +118,6 @@ class CachedResponseSuitabilityChecker { return false; } - if (isGet(request) && !validityStrategy.contentLengthHeaderMatchesActualLength(entry)) { - LOG.debug("Cache entry Content-Length and header information do not match"); - return false; - } - if (hasUnsupportedConditionalHeaders(request)) { LOG.debug("Request contains unsupported conditional headers"); return false; diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/cache/TestCacheHeaderSupport.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/cache/TestCacheHeaderSupport.java new file mode 100644 index 000000000..dbfaf33b4 --- /dev/null +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/cache/TestCacheHeaderSupport.java @@ -0,0 +1,67 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ +package org.apache.hc.client5.http.cache; + +import java.util.Set; + +import org.apache.hc.core5.http.ContentType; +import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.HttpStatus; +import org.apache.hc.core5.http.support.BasicResponseBuilder; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class TestCacheHeaderSupport { + + @Test + public void testHopByHopHeaders() { + Assertions.assertTrue(CacheHeaderSupport.isHopByHop("Connection")); + Assertions.assertTrue(CacheHeaderSupport.isHopByHop("connection")); + Assertions.assertTrue(CacheHeaderSupport.isHopByHop("coNNection")); + Assertions.assertFalse(CacheHeaderSupport.isHopByHop("Content-Type")); + Assertions.assertFalse(CacheHeaderSupport.isHopByHop("huh")); + } + + @Test + public void testHopByHopHeadersConnectionSpecific() { + final HttpResponse response = BasicResponseBuilder.create(HttpStatus.SC_OK) + .addHeader(HttpHeaders.CONNECTION, "blah, blah, this, that") + .addHeader(HttpHeaders.CONTENT_TYPE, ContentType.TEXT_PLAIN.toString()) + .build(); + final Set hopByHopConnectionSpecific = CacheHeaderSupport.hopByHopConnectionSpecific(response); + Assertions.assertTrue(hopByHopConnectionSpecific.contains("Connection")); + Assertions.assertTrue(hopByHopConnectionSpecific.contains("connection")); + Assertions.assertTrue(hopByHopConnectionSpecific.contains("coNNection")); + Assertions.assertFalse(hopByHopConnectionSpecific.contains("Content-Type")); + Assertions.assertTrue(hopByHopConnectionSpecific.contains("blah")); + Assertions.assertTrue(hopByHopConnectionSpecific.contains("Blah")); + Assertions.assertTrue(hopByHopConnectionSpecific.contains("This")); + Assertions.assertTrue(hopByHopConnectionSpecific.contains("That")); + } + +} diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/HttpTestUtils.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/HttpTestUtils.java index 3c70feac5..1d626900d 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/HttpTestUtils.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/HttpTestUtils.java @@ -36,6 +36,7 @@ import java.util.Objects; import java.util.Random; import org.apache.hc.client5.http.cache.HttpCacheEntry; +import org.apache.hc.client5.http.cache.CacheHeaderSupport; import org.apache.hc.client5.http.cache.Resource; import org.apache.hc.client5.http.utils.DateUtils; import org.apache.hc.core5.http.ClassicHttpRequest; @@ -62,25 +63,6 @@ import org.junit.jupiter.api.Assertions; public class HttpTestUtils { - /* - * "The following HTTP/1.1 headers are hop-by-hop headers..." - * - * @see http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.5.1 - */ - private static final String[] HOP_BY_HOP_HEADERS = { "Connection", "Keep-Alive", "Proxy-Authenticate", - "Proxy-Authorization", "TE", "Trailers", "Transfer-Encoding", "Upgrade" }; - - /* - * "Multiple message-header fields with the same field-name MAY be present - * in a message if and only if the entire field-value for that header field - * is defined as a comma-separated list [i.e., #(values)]." - * - * @see http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2 - */ - private static final String[] MULTI_HEADERS = { "Accept", "Accept-Charset", "Accept-Encoding", - "Accept-Language", "Allow", "Cache-Control", "Connection", "Content-Encoding", - "Content-Language", "Expect", "Pragma", "Proxy-Authenticate", "TE", "Trailer", - "Transfer-Encoding", "Upgrade", "Via", HttpHeaders.WARNING, "WWW-Authenticate" }; private static final String[] SINGLE_HEADERS = { "Accept-Ranges", "Age", "Authorization", "Content-Length", "Content-Location", "Content-MD5", "Content-Range", "Content-Type", "Date", "ETag", "Expires", "From", "Host", "If-Match", "If-Modified-Since", @@ -88,21 +70,6 @@ public class HttpTestUtils { "Max-Forwards", "Proxy-Authorization", "Range", "Referer", "Retry-After", "Server", "User-Agent", "Vary" }; - /* - * Determines whether the given header name is considered a hop-by-hop - * header. - * - * @see http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.5.1 - */ - public static boolean isHopByHopHeader(final String name) { - for (final String s : HOP_BY_HOP_HEADERS) { - if (s.equalsIgnoreCase(name)) { - return true; - } - } - return false; - } - /* * Determines whether a given header name may only appear once in a message. */ @@ -164,7 +131,7 @@ public class HttpTestUtils { */ public static boolean isEndToEndHeaderSubset(final HttpMessage r1, final HttpMessage r2) { for (final Header h : r1.getHeaders()) { - if (!isHopByHopHeader(h.getName())) { + if (!CacheHeaderSupport.isHopByHop(h)) { final String r1val = getCanonicalHeaderValue(r1, h.getName()); final String r2val = getCanonicalHeaderValue(r2, h.getName()); if (!r1val.equals(r2val)) { @@ -291,8 +258,7 @@ public class HttpTestUtils { public static Header[] getStockHeaders(final Instant when) { return new Header[] { new BasicHeader("Date", DateUtils.formatStandardDate(when)), - new BasicHeader("Server", "MockServer/1.0"), - new BasicHeader("Content-Length", "128") + new BasicHeader("Server", "MockServer/1.0") }; } 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 e4e546af5..67b17e87d 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 @@ -263,23 +263,4 @@ public class TestCacheUpdateHandler { assertThat(updatedEntry, Matchers.not(ContainsHeaderMatcher.contains("Content-Encoding", "gzip"))); } - @Test - public void testContentLengthIsNotAddedWhenTransferEncodingIsPresent() throws IOException { - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(requestDate)), - new BasicHeader("ETag", "\"etag\""), - new BasicHeader("Transfer-Encoding", "chunked")}; - - entry = HttpTestUtils.makeCacheEntry(headers); - response.setHeaders(new BasicHeader("Last-Modified", DateUtils.formatStandardDate(responseDate)), - new BasicHeader("Cache-Control", "public"), - new BasicHeader("Content-Length", "0")); - - final HttpCacheEntry updatedEntry = impl.updateCacheEntry(null, entry, - Instant.now(), Instant.now(), response); - - assertThat(updatedEntry, ContainsHeaderMatcher.contains("Transfer-Encoding", "chunked")); - assertThat(updatedEntry, Matchers.not(ContainsHeaderMatcher.contains("Content-Length", "0"))); - } - } diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java index 374640c30..fd1902821 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheValidityPolicy.java @@ -37,7 +37,6 @@ import java.time.Instant; import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.client5.http.utils.DateUtils; import org.apache.hc.core5.http.Header; -import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.message.BasicHeader; import org.apache.hc.core5.util.TimeValue; import org.junit.jupiter.api.BeforeEach; @@ -330,33 +329,6 @@ public class TestCacheValidityPolicy { assertFalse(impl.isRevalidatable(entry)); } - @Test - public void testMissingContentLengthDoesntInvalidateEntry() { - final int contentLength = 128; - final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry( - new Header[] { }, // no Content-Length header - HttpTestUtils.getRandomBytes(contentLength)); - assertTrue(impl.contentLengthHeaderMatchesActualLength(entry)); - } - - @Test - public void testCorrectContentLengthDoesntInvalidateEntry() { - final int contentLength = 128; - final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry( - new Header[] { new BasicHeader(HttpHeaders.CONTENT_LENGTH, Integer.toString(contentLength)) }, - HttpTestUtils.getRandomBytes(contentLength)); - assertTrue(impl.contentLengthHeaderMatchesActualLength(entry)); - } - - @Test - public void testWrongContentLengthInvalidatesEntry() { - final int contentLength = 128; - final HttpCacheEntry entry = HttpTestUtils.makeCacheEntry( - new Header[]{ new BasicHeader(HttpHeaders.CONTENT_LENGTH, Integer.toString(contentLength+1)) }, - HttpTestUtils.getRandomBytes(contentLength)); - assertFalse(impl.contentLengthHeaderMatchesActualLength(entry)); - } - @Test public void testNegativeAgeHeaderValueReturnsMaxAge() { final Header[] headers = new Header[] { new BasicHeader("Age", "-100") }; diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedHttpResponseGenerator.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedHttpResponseGenerator.java index 7f4396f2f..7945c76bb 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedHttpResponseGenerator.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedHttpResponseGenerator.java @@ -39,7 +39,6 @@ import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.Header; -import org.apache.hc.core5.http.message.BasicHeader; import org.apache.hc.core5.util.TimeValue; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; @@ -74,31 +73,6 @@ public class TestCachedHttpResponseGenerator { Assertions.assertEquals(buf.length, Integer.parseInt(length.getValue()), "Content-Length does not match buffer length"); } - @Test - public void testContentLengthIsNotAddedWhenTransferEncodingIsPresent() throws Exception { - - final Header[] hdrs = new Header[] { new BasicHeader("Transfer-Encoding", "chunked") }; - final byte[] buf = new byte[] { 1, 2, 3, 4, 5 }; - final HttpCacheEntry entry1 = HttpTestUtils.makeCacheEntry(hdrs, buf); - - final SimpleHttpResponse response = impl.generateResponse(request, entry1); - - final Header length = response.getFirstHeader("Content-Length"); - - Assertions.assertNull(length); - } - - @Test - public void testResponseMatchesCacheEntry() throws Exception { - final SimpleHttpResponse response = impl.generateResponse(request, entry); - - Assertions.assertTrue(response.containsHeader("Content-Length")); - - Assertions.assertSame("HTTP", response.getVersion().getProtocol()); - Assertions.assertEquals(1, response.getVersion().getMajor()); - Assertions.assertEquals(1, response.getVersion().getMinor()); - } - @Test public void testResponseStatusCodeMatchesCacheEntry() throws Exception { final SimpleHttpResponse response = impl.generateResponse(request, entry); diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedResponseSuitabilityChecker.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedResponseSuitabilityChecker.java index 9ff60df84..939e87fa2 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedResponseSuitabilityChecker.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachedResponseSuitabilityChecker.java @@ -73,25 +73,10 @@ public class TestCachedResponseSuitabilityChecker { return HttpTestUtils.makeCacheEntry(elevenSecondsAgo, nineSecondsAgo, headers); } - @Test - public void testNotSuitableIfContentLengthHeaderIsWrong() { - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","1") - }; - entry = getEntry(headers); - responseCacheControl = ResponseCacheControl.builder() - .setMaxAge(3600) - .build(); - - Assertions.assertFalse(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); - } - @Test public void testSuitableIfCacheEntryIsFresh() { final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","128") + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; entry = getEntry(headers); responseCacheControl = ResponseCacheControl.builder() @@ -103,8 +88,7 @@ public class TestCachedResponseSuitabilityChecker { @Test public void testNotSuitableIfCacheEntryIsNotFresh() { final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","128") + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; entry = getEntry(headers); responseCacheControl = ResponseCacheControl.builder() @@ -119,8 +103,7 @@ public class TestCachedResponseSuitabilityChecker { .setNoCache(true) .build(); final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","128") + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; entry = getEntry(headers); responseCacheControl = ResponseCacheControl.builder() @@ -135,8 +118,7 @@ public class TestCachedResponseSuitabilityChecker { .setMaxAge(10) .build(); final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","128") + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; responseCacheControl = ResponseCacheControl.builder() .setMaxAge(3600) @@ -151,8 +133,7 @@ public class TestCachedResponseSuitabilityChecker { .setMaxAge(15) .build(); final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","128") + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; entry = getEntry(headers); responseCacheControl = ResponseCacheControl.builder() @@ -167,8 +148,7 @@ public class TestCachedResponseSuitabilityChecker { .setMinFresh(10) .build(); final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","128") + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; entry = getEntry(headers); responseCacheControl = ResponseCacheControl.builder() @@ -183,8 +163,7 @@ public class TestCachedResponseSuitabilityChecker { .setMinFresh(10) .build(); final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","128") + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; entry = getEntry(headers); responseCacheControl = ResponseCacheControl.builder() @@ -199,8 +178,7 @@ public class TestCachedResponseSuitabilityChecker { .setMaxStale(10) .build(); final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","128") + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; entry = getEntry(headers); responseCacheControl = ResponseCacheControl.builder() @@ -215,8 +193,7 @@ public class TestCachedResponseSuitabilityChecker { .setMaxStale(2) .build(); final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","128") + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; entry = getEntry(headers); responseCacheControl = ResponseCacheControl.builder() @@ -232,8 +209,7 @@ public class TestCachedResponseSuitabilityChecker { final Header[] headers = { new BasicHeader("Date", DateUtils.formatStandardDate(oneSecondAgo)), - new BasicHeader("Last-Modified", DateUtils.formatStandardDate(twentyOneSecondsAgo)), - new BasicHeader("Content-Length", "128") + new BasicHeader("Last-Modified", DateUtils.formatStandardDate(twentyOneSecondsAgo)) }; entry = HttpTestUtils.makeCacheEntry(oneSecondAgo, oneSecondAgo, headers); @@ -249,8 +225,7 @@ public class TestCachedResponseSuitabilityChecker { @Test public void testSuitableIfCacheEntryIsHeuristicallyFreshEnoughByDefault() { final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length", "128") + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; entry = getEntry(headers); @@ -268,8 +243,7 @@ public class TestCachedResponseSuitabilityChecker { public void testSuitableIfRequestMethodisHEAD() { final HttpRequest headRequest = new BasicHttpRequest("HEAD", "/foo"); final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","128") + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; entry = getEntry(headers); responseCacheControl = ResponseCacheControl.builder() @@ -282,8 +256,7 @@ public class TestCachedResponseSuitabilityChecker { @Test public void testNotSuitableIfRequestMethodIsGETAndEntryResourceIsNull() { final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","128") + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; entry = HttpTestUtils.makeCacheEntry(elevenSecondsAgo, nineSecondsAgo, Method.HEAD, HttpStatus.SC_OK, headers, null, null); responseCacheControl = ResponseCacheControl.builder() @@ -293,27 +266,11 @@ public class TestCachedResponseSuitabilityChecker { Assertions.assertFalse(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); } - @Test - public void testNotSuitableForGETIfEntryDoesNotSpecifyARequestMethodOrEntity() { - impl = new CachedResponseSuitabilityChecker(CacheConfig.custom().build()); - final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","128") - }; - entry = HttpTestUtils.makeCacheEntry(elevenSecondsAgo, nineSecondsAgo, Method.GET, HttpStatus.SC_OK, headers, null, null); - responseCacheControl = ResponseCacheControl.builder() - .setMaxAge(3600) - .build(); - - Assertions.assertFalse(impl.canCachedResponseBeUsed(requestCacheControl, responseCacheControl, request, entry, now)); - } - @Test public void testSuitableForGETIfEntryDoesNotSpecifyARequestMethodButContainsEntity() { impl = new CachedResponseSuitabilityChecker(CacheConfig.custom().build()); final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","128") + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; entry = HttpTestUtils.makeCacheEntry(elevenSecondsAgo, nineSecondsAgo, Method.GET, HttpStatus.SC_OK, headers, HttpTestUtils.getRandomBytes(128), null); responseCacheControl = ResponseCacheControl.builder() @@ -342,8 +299,7 @@ public class TestCachedResponseSuitabilityChecker { final HttpRequest headRequest = new BasicHttpRequest("HEAD", "/foo"); impl = new CachedResponseSuitabilityChecker(CacheConfig.custom().build()); final Header[] headers = { - new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)), - new BasicHeader("Content-Length","128") + new BasicHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)) }; entry = HttpTestUtils.makeCacheEntry(elevenSecondsAgo, nineSecondsAgo, Method.GET, HttpStatus.SC_OK, headers, null, null); responseCacheControl = ResponseCacheControl.builder() diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java index 3d1237589..6eb298c40 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestProtocolRequirements.java @@ -4760,64 +4760,6 @@ public class TestProtocolRequirements { Assertions.assertEquals(server, result.getFirstHeader("Server").getValue()); } - /* "If multiple encodings have been applied to an entity, the transfer- - * codings MUST be listed in the order in which they were applied." - * - * http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.41 - */ - @Test - public void testOrderOfMultipleTransferEncodingHeadersIsPreserved() throws Exception { - originResponse.addHeader("Transfer-Encoding","chunked"); - originResponse.addHeader("Transfer-Encoding","x-transfer"); - - Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(originResponse); - - final ClassicHttpResponse result = execute(request); - int transfer_encodings = 0; - final Iterator it = MessageSupport.iterate(result, HttpHeaders.TRANSFER_ENCODING); - while (it.hasNext()) { - final HeaderElement elt = it.next(); - switch(transfer_encodings) { - case 0: - Assertions.assertEquals("chunked",elt.getName()); - break; - case 1: - Assertions.assertEquals("x-transfer",elt.getName()); - break; - default: - Assertions.fail("too many transfer encodings"); - } - transfer_encodings++; - } - Assertions.assertEquals(2, transfer_encodings); - } - - @Test - public void testOrderOfMultipleTransferEncodingsInSingleHeadersIsPreserved() throws Exception { - originResponse.addHeader("Transfer-Encoding","chunked, x-transfer"); - - Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(originResponse); - - final ClassicHttpResponse result = execute(request); - int transfer_encodings = 0; - final Iterator it = MessageSupport.iterate(result, HttpHeaders.TRANSFER_ENCODING); - while (it.hasNext()) { - final HeaderElement elt = it.next(); - switch(transfer_encodings) { - case 0: - Assertions.assertEquals("chunked",elt.getName()); - break; - case 1: - Assertions.assertEquals("x-transfer",elt.getName()); - break; - default: - Assertions.fail("too many transfer encodings"); - } - transfer_encodings++; - } - Assertions.assertEquals(2, transfer_encodings); - } - /* "A Vary field value of '*' signals that unspecified parameters * not limited to the request-headers (e.g., the network address * of the client), play a role in the selection of the response