diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java index 50aac133e..e343597c9 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java @@ -217,7 +217,6 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler final URIAuthority authority = request.getAuthority(); final String scheme = request.getScheme(); final HttpHost target = authority != null ? new HttpHost(scheme, authority) : route.getTargetHost(); - final String via = generateViaHeader(request); // default response context setResponseStatus(context, CacheResponseStatus.CACHE_MISS); @@ -228,8 +227,6 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler return; } - request.addHeader(HttpHeaders.VIA,via); - final RequestCacheControl requestCacheControl = CacheControlHeaderParser.INSTANCE.parse(request); if (cacheableRequestPolicy.isServableFromCache(requestCacheControl, request)) { @@ -296,8 +293,6 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler final HttpResponse backendResponse, final EntityDetails entityDetails) throws HttpException, IOException { final Instant responseDate = getCurrentDate(); - backendResponse.addHeader(HttpHeaders.VIA, generateViaHeader(backendResponse)); - final AsyncExecCallback callback = new BackendResponseHandler(target, request, requestDate, responseDate, scope, asyncExecCallback); callbackRef.set(callback); return callback.handleResponse(backendResponse, entityDetails); @@ -749,8 +744,6 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler } AsyncExecCallback evaluateResponse(final HttpResponse backendResponse, final Instant responseDate) { - backendResponse.addHeader(HttpHeaders.VIA, generateViaHeader(backendResponse)); - final int statusCode = backendResponse.getCode(); if (statusCode == HttpStatus.SC_NOT_MODIFIED || statusCode == HttpStatus.SC_OK) { recordCacheUpdate(scope.clientContext); @@ -980,8 +973,6 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler final HttpResponse backendResponse, final EntityDetails entityDetails) throws HttpException, IOException { final Instant responseDate = getCurrentDate(); - backendResponse.addHeader(HttpHeaders.VIA, generateViaHeader(backendResponse)); - final AsyncExecCallback callback; // Handle 304 Not Modified responses if (backendResponse.getCode() == HttpStatus.SC_NOT_MODIFIED) { diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java index 2cfc51459..2754996a3 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java @@ -154,7 +154,6 @@ class CachingExec extends CachingExecBase implements ExecChainHandler { final URIAuthority authority = request.getAuthority(); final String scheme = request.getScheme(); final HttpHost target = authority != null ? new HttpHost(scheme, authority) : route.getTargetHost(); - final String via = generateViaHeader(request); // default response context setResponseStatus(context, CacheResponseStatus.CACHE_MISS); @@ -167,8 +166,6 @@ class CachingExec extends CachingExecBase implements ExecChainHandler { final CacheHit hit = result != null ? result.hit : null; final CacheHit root = result != null ? result.root : null; - request.addHeader(HttpHeaders.VIA, via); - final RequestCacheControl requestCacheControl = CacheControlHeaderParser.INSTANCE.parse(request); if (!cacheableRequestPolicy.isServableFromCache(requestCacheControl, request)) { LOG.debug("Request is not servable from cache"); @@ -220,7 +217,6 @@ class CachingExec extends CachingExecBase implements ExecChainHandler { LOG.debug("Calling the backend"); final ClassicHttpResponse backendResponse = chain.proceed(request, scope); try { - backendResponse.addHeader(HttpHeaders.VIA, generateViaHeader(backendResponse)); return handleBackendResponse(target, request, scope, requestDate, getCurrentDate(), backendResponse); } catch (final IOException | RuntimeException ex) { backendResponse.close(); @@ -331,8 +327,6 @@ class CachingExec extends CachingExecBase implements ExecChainHandler { responseDate = getCurrentDate(); } - backendResponse.addHeader(HttpHeaders.VIA, generateViaHeader(backendResponse)); - final int statusCode = backendResponse.getCode(); if (statusCode == HttpStatus.SC_NOT_MODIFIED || statusCode == HttpStatus.SC_OK) { recordCacheUpdate(scope.clientContext); @@ -490,8 +484,6 @@ class CachingExec extends CachingExecBase implements ExecChainHandler { try { final Instant responseDate = getCurrentDate(); - backendResponse.addHeader(HttpHeaders.VIA, generateViaHeader(backendResponse)); - if (backendResponse.getCode() != HttpStatus.SC_NOT_MODIFIED) { return handleBackendResponse(target, request, scope, requestDate, responseDate, backendResponse); } else { diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java index 273514aa4..00959860d 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java @@ -28,8 +28,6 @@ package org.apache.hc.client5.http.impl.cache; import java.io.IOException; import java.time.Instant; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; @@ -40,17 +38,12 @@ import org.apache.hc.client5.http.cache.ResourceIOException; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.HttpHost; -import org.apache.hc.core5.http.HttpMessage; import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpResponse; import org.apache.hc.core5.http.HttpStatus; -import org.apache.hc.core5.http.HttpVersion; import org.apache.hc.core5.http.Method; -import org.apache.hc.core5.http.ProtocolVersion; -import org.apache.hc.core5.http.URIScheme; import org.apache.hc.core5.http.protocol.HttpContext; import org.apache.hc.core5.util.TimeValue; -import org.apache.hc.core5.util.VersionInfo; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -60,7 +53,6 @@ public class CachingExecBase { final AtomicLong cacheMisses = new AtomicLong(); final AtomicLong cacheUpdates = new AtomicLong(); - final Map viaHeaders = new ConcurrentHashMap<>(4); final ResponseCachingPolicy responseCachingPolicy; final CacheValidityPolicy validityPolicy; final CachedHttpResponseGenerator responseGenerator; @@ -230,35 +222,6 @@ public class CachingExecBase { return false; } - String generateViaHeader(final HttpMessage msg) { - - if (msg.getVersion() == null) { - msg.setVersion(HttpVersion.DEFAULT); - } - final ProtocolVersion pv = msg.getVersion(); - final String existingEntry = viaHeaders.get(msg.getVersion()); - if (existingEntry != null) { - return existingEntry; - } - - final VersionInfo vi = VersionInfo.loadVersionInfo("org.apache.hc.client5", getClass().getClassLoader()); - final String release = (vi != null) ? vi.getRelease() : VersionInfo.UNAVAILABLE; - - final String value; - final int major = pv.getMajor(); - final int minor = pv.getMinor(); - if (URIScheme.HTTP.same(pv.getProtocol())) { - value = String.format("%d.%d localhost (Apache-HttpClient/%s (cache))", major, minor, - release); - } else { - value = String.format("%s/%d.%d localhost (Apache-HttpClient/%s (cache))", pv.getProtocol(), major, - minor, release); - } - viaHeaders.put(pv, value); - - return value; - } - void setResponseStatus(final HttpContext context, final CacheResponseStatus value) { if (context != null) { context.setAttribute(HttpCacheContext.CACHE_RESPONSE_STATUS, value); diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingH2AsyncClientBuilder.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingH2AsyncClientBuilder.java index 108beb9f3..aff56365f 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingH2AsyncClientBuilder.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingH2AsyncClientBuilder.java @@ -66,6 +66,8 @@ public class CachingH2AsyncClientBuilder extends H2AsyncClientBuilder { protected CachingH2AsyncClientBuilder() { super(); addResponseInterceptorFirst(ResponseProtocolCompliance.INSTANCE); + addResponseInterceptorLast(ResponseViaCache.INSTANCE); + addRequestInterceptorLast(RequestViaCache.INSTANCE); this.deleteCache = true; } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpAsyncClientBuilder.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpAsyncClientBuilder.java index 50656dff3..2f31f030f 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpAsyncClientBuilder.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpAsyncClientBuilder.java @@ -66,6 +66,8 @@ public class CachingHttpAsyncClientBuilder extends HttpAsyncClientBuilder { protected CachingHttpAsyncClientBuilder() { super(); addResponseInterceptorFirst(ResponseProtocolCompliance.INSTANCE); + addResponseInterceptorLast(ResponseViaCache.INSTANCE); + addRequestInterceptorLast(RequestViaCache.INSTANCE); this.deleteCache = true; } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java index ebbd19dd9..56634d4f0 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java @@ -62,6 +62,8 @@ public class CachingHttpClientBuilder extends HttpClientBuilder { protected CachingHttpClientBuilder() { super(); addResponseInterceptorFirst(ResponseProtocolCompliance.INSTANCE); + addResponseInterceptorLast(ResponseViaCache.INSTANCE); + addRequestInterceptorLast(RequestViaCache.INSTANCE); this.deleteCache = true; } diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/RequestViaCache.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/RequestViaCache.java new file mode 100644 index 000000000..3379c0449 --- /dev/null +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/RequestViaCache.java @@ -0,0 +1,58 @@ +/* + * ==================================================================== + * 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.impl.cache; + +import java.io.IOException; + +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.ThreadingBehavior; +import org.apache.hc.core5.http.EntityDetails; +import org.apache.hc.core5.http.HttpException; +import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.HttpRequest; +import org.apache.hc.core5.http.HttpRequestInterceptor; +import org.apache.hc.core5.http.ProtocolVersion; +import org.apache.hc.core5.http.protocol.HttpContext; + +/** + * This response interceptor adds {@literal VIA} header to all outgoing + * request messages dispatched through the caching layer. + */ +@Contract(threading = ThreadingBehavior.IMMUTABLE) +class RequestViaCache implements HttpRequestInterceptor { + + public static final RequestViaCache INSTANCE = new RequestViaCache(); + + @Override + public void process(final HttpRequest request, + final EntityDetails entity, + final HttpContext context) throws HttpException, IOException { + final ProtocolVersion protocolVersion = context.getProtocolVersion(); + request.addHeader(HttpHeaders.VIA, ViaCacheGenerator.INSTANCE.lookup(protocolVersion)); + } + +} diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java index 74239cf00..83f000415 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java @@ -43,6 +43,7 @@ import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.HttpVersion; import org.apache.hc.core5.http.Method; import org.apache.hc.core5.http.ProtocolVersion; +import org.apache.hc.core5.http.message.BasicTokenIterator; import org.apache.hc.core5.http.message.MessageSupport; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -336,15 +337,10 @@ class ResponseCachingPolicy { } private boolean from1_0Origin(final HttpResponse response) { - final Iterator it = MessageSupport.iterate(response, HttpHeaders.VIA); + final Iterator it = new BasicTokenIterator(response.headerIterator(HttpHeaders.VIA)); if (it.hasNext()) { - final HeaderElement elt = it.next(); - final String proto = elt.toString().split("\\s")[0]; - if (proto.contains("/")) { - return proto.equals("HTTP/1.0"); - } else { - return proto.equals("1.0"); - } + final String token = it.next(); + return token.startsWith("1.0 ") || token.startsWith("HTTP/1.0 "); } final ProtocolVersion version = response.getVersion() != null ? response.getVersion() : HttpVersion.DEFAULT; return HttpVersion.HTTP_1_0.equals(version); diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseViaCache.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseViaCache.java new file mode 100644 index 000000000..205933b16 --- /dev/null +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseViaCache.java @@ -0,0 +1,58 @@ +/* + * ==================================================================== + * 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.impl.cache; + +import java.io.IOException; + +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.ThreadingBehavior; +import org.apache.hc.core5.http.EntityDetails; +import org.apache.hc.core5.http.HttpException; +import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.HttpResponseInterceptor; +import org.apache.hc.core5.http.ProtocolVersion; +import org.apache.hc.core5.http.protocol.HttpContext; + +/** + * This response interceptor adds {@literal VIA} header to all incoming + * response messages dispatched through the caching layer. + */ +@Contract(threading = ThreadingBehavior.IMMUTABLE) +class ResponseViaCache implements HttpResponseInterceptor { + + public static final ResponseViaCache INSTANCE = new ResponseViaCache(); + + @Override + public void process(final HttpResponse response, + final EntityDetails entity, + final HttpContext context) throws HttpException, IOException { + final ProtocolVersion protocolVersion = context.getProtocolVersion(); + response.addHeader(HttpHeaders.VIA, ViaCacheGenerator.INSTANCE.lookup(protocolVersion)); + } + +} diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ViaCacheGenerator.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ViaCacheGenerator.java new file mode 100644 index 000000000..22260b86d --- /dev/null +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ViaCacheGenerator.java @@ -0,0 +1,62 @@ +/* + * ==================================================================== + * 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.impl.cache; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +import org.apache.hc.core5.http.ProtocolVersion; +import org.apache.hc.core5.http.URIScheme; +import org.apache.hc.core5.util.VersionInfo; + +final class ViaCacheGenerator { + + final static ViaCacheGenerator INSTANCE = new ViaCacheGenerator(); + + final ConcurrentMap internalCache = new ConcurrentHashMap<>(4); + + String generateViaHeader(final VersionInfo vi, final ProtocolVersion pv) { + final StringBuilder buf = new StringBuilder(); + if (!URIScheme.HTTP.same(pv.getProtocol())) { + buf.append(pv.getProtocol()).append('/'); + } + buf.append(pv.getMajor()).append('.').append(pv.getMinor()); + buf.append(' ').append("localhost").append(' '); + buf.append("(Apache-HttpClient/"); + final String release = (vi != null) ? vi.getRelease() : VersionInfo.UNAVAILABLE; + buf.append(release).append(" (cache))"); + return buf.toString(); + } + + String lookup(final ProtocolVersion pv) { + return internalCache.computeIfAbsent(pv, (v) -> { + final VersionInfo vi = VersionInfo.loadVersionInfo("org.apache.hc.client5", getClass().getClassLoader()); + return generateViaHeader(vi, v); + }); + } + +} 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 c54f34c65..1a31989d4 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 @@ -45,7 +45,6 @@ import org.apache.hc.client5.http.auth.StandardAuthScheme; import org.apache.hc.client5.http.cache.CacheResponseStatus; import org.apache.hc.client5.http.cache.HttpCacheContext; import org.apache.hc.client5.http.cache.HttpCacheEntry; -import org.apache.hc.client5.http.cache.HttpCacheEntryFactory; import org.apache.hc.client5.http.cache.HttpCacheStorage; import org.apache.hc.client5.http.classic.ExecChain; import org.apache.hc.client5.http.classic.ExecRuntime; @@ -58,9 +57,7 @@ import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpException; import org.apache.hc.core5.http.HttpHost; -import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpStatus; -import org.apache.hc.core5.http.HttpVersion; import org.apache.hc.core5.http.io.entity.EntityUtils; import org.apache.hc.core5.http.io.entity.InputStreamEntity; import org.apache.hc.core5.http.io.support.ClassicRequestBuilder; @@ -71,7 +68,6 @@ import org.apache.hc.core5.net.URIAuthority; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; @@ -86,24 +82,6 @@ public class TestCachingExecChain { HttpCacheStorage mockStorage; @Mock DefaultCacheRevalidator cacheRevalidator; - @Mock - CachedHttpResponseGenerator responseGenerator; - @Mock - HttpCacheEntryFactory cacheEntryFactory; - @Mock - CacheValidityPolicy validityPolicy; - @Mock - ResponseCachingPolicy responseCachingPolicy; - @Mock - CacheableRequestPolicy cacheableRequestPolicy; - @Mock - CachedResponseSuitabilityChecker suitabilityChecker; - @Mock - ResponseProtocolCompliance responseCompliance; - @Mock - ConditionalRequestBuilder conditionalRequestBuilder; - @Mock - HttpCache responseCache; HttpRoute route; HttpHost host; @@ -245,27 +223,6 @@ public class TestCachingExecChain { Assertions.assertEquals(CacheResponseStatus.CACHE_MODULE_RESPONSE, context.getCacheResponseStatus()); } - @Test - public void testRecordsClientProtocolInViaHeaderIfRequestNotServableFromCache() throws Exception { - final ClassicHttpRequest originalRequest = new BasicClassicHttpRequest("GET", "/"); - originalRequest.setVersion(HttpVersion.HTTP_1_0); - final ClassicHttpRequest req = originalRequest; - req.setHeader("Cache-Control", "no-cache"); - final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_NO_CONTENT, "No Content"); - - Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp); - - execute(req); - - final ArgumentCaptor reqCapture = ArgumentCaptor.forClass(ClassicHttpRequest.class); - Mockito.verify(mockExecChain).proceed(reqCapture.capture(), Mockito.any()); - - final HttpRequest captured = reqCapture.getValue(); - final String via = captured.getFirstHeader("Via").getValue(); - final String proto = via.split("\\s+")[0]; - Assertions.assertTrue("http/1.0".equalsIgnoreCase(proto) || "1.0".equalsIgnoreCase(proto)); - } - @Test public void testSetsCacheMissContextIfRequestNotServableFromCache() throws Exception { final ClassicHttpRequest req = new HttpGet("http://foo.example.com/"); @@ -278,34 +235,6 @@ public class TestCachingExecChain { Assertions.assertEquals(CacheResponseStatus.CACHE_MISS, context.getCacheResponseStatus()); } - @Test - public void testSetsViaHeaderOnResponseIfRequestNotServableFromCache() throws Exception { - final ClassicHttpRequest req = new HttpGet("http://foo.example.com/"); - req.setHeader("Cache-Control", "no-cache"); - final ClassicHttpResponse resp = new BasicClassicHttpResponse(HttpStatus.SC_NO_CONTENT, "No Content"); - - Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp); - - final ClassicHttpResponse result = execute(req); - Assertions.assertNotNull(result.getFirstHeader("Via")); - } - - @Test - public void testSetsViaHeaderOnResponseForCacheMiss() throws Exception { - final ClassicHttpRequest req1 = new HttpGet("http://foo.example.com/"); - final ClassicHttpResponse resp1 = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - resp1.setEntity(HttpTestUtils.makeBody(128)); - resp1.setHeader("Content-Length", "128"); - resp1.setHeader("ETag", "\"etag\""); - resp1.setHeader("Date", DateUtils.formatStandardDate(Instant.now())); - resp1.setHeader("Cache-Control", "public, max-age=3600"); - - Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1); - - final ClassicHttpResponse result = execute(req1); - Assertions.assertNotNull(result.getFirstHeader("Via")); - } - @Test public void testSetsCacheHitContextIfRequestServedFromCache() throws Exception { final ClassicHttpRequest req1 = new HttpGet("http://foo.example.com/"); @@ -324,24 +253,6 @@ public class TestCachingExecChain { Assertions.assertEquals(CacheResponseStatus.CACHE_HIT, context.getCacheResponseStatus()); } - @Test - public void testSetsViaHeaderOnResponseIfRequestServedFromCache() throws Exception { - final ClassicHttpRequest req1 = new HttpGet("http://foo.example.com/"); - final ClassicHttpRequest req2 = new HttpGet("http://foo.example.com/"); - final ClassicHttpResponse resp1 = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - resp1.setEntity(HttpTestUtils.makeBody(128)); - resp1.setHeader("Content-Length", "128"); - resp1.setHeader("ETag", "\"etag\""); - resp1.setHeader("Date", DateUtils.formatStandardDate(Instant.now())); - resp1.setHeader("Cache-Control", "public, max-age=3600"); - - Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1); - - execute(req1); - final ClassicHttpResponse result = execute(req2); - Assertions.assertNotNull(result.getFirstHeader("Via")); - } - @Test public void testReturns304ForIfModifiedSinceHeaderIfRequestServedFromCache() throws Exception { final Instant now = Instant.now(); @@ -630,38 +541,6 @@ public class TestCachingExecChain { Assertions.assertEquals(CacheResponseStatus.VALIDATED, context.getCacheResponseStatus()); } - @Test - public void testSetsViaHeaderIfRequestWasSuccessfullyValidated() throws Exception { - final Instant now = Instant.now(); - final Instant tenSecondsAgo = now.minusSeconds(10); - - final ClassicHttpRequest req1 = new HttpGet("http://foo.example.com/"); - final ClassicHttpRequest req2 = new HttpGet("http://foo.example.com/"); - - final ClassicHttpResponse resp1 = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - resp1.setEntity(HttpTestUtils.makeBody(128)); - resp1.setHeader("Content-Length", "128"); - resp1.setHeader("ETag", "\"etag\""); - resp1.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)); - resp1.setHeader("Cache-Control", "public, max-age=5"); - - final ClassicHttpResponse resp2 = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - resp2.setEntity(HttpTestUtils.makeBody(128)); - resp2.setHeader("Content-Length", "128"); - resp2.setHeader("ETag", "\"etag\""); - resp2.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)); - resp2.setHeader("Cache-Control", "public, max-age=5"); - - Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1); - - execute(req1); - - Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp2); - - final ClassicHttpResponse result = execute(req2); - Assertions.assertNotNull(result.getFirstHeader("Via")); - } - @Test public void testSetsModuleResponseContextIfValidationRequiredButFailed() throws Exception { final Instant now = Instant.now(); @@ -713,31 +592,6 @@ public class TestCachingExecChain { Assertions.assertEquals(CacheResponseStatus.CACHE_HIT, context.getCacheResponseStatus()); } - @Test - public void testSetViaHeaderIfValidationFailsButNotRequired() throws Exception { - final Instant now = Instant.now(); - final Instant tenSecondsAgo = now.minusSeconds(10); - - final ClassicHttpRequest req1 = new HttpGet("http://foo.example.com/"); - final ClassicHttpRequest req2 = new HttpGet("http://foo.example.com/"); - - final ClassicHttpResponse resp1 = new BasicClassicHttpResponse(HttpStatus.SC_OK, "OK"); - resp1.setEntity(HttpTestUtils.makeBody(128)); - resp1.setHeader("Content-Length", "128"); - resp1.setHeader("ETag", "\"etag\""); - resp1.setHeader("Date", DateUtils.formatStandardDate(tenSecondsAgo)); - resp1.setHeader("Cache-Control", "public, max-age=5"); - - Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1); - - execute(req1); - - Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenThrow(new IOException()); - - final ClassicHttpResponse result = execute(req2); - Assertions.assertNotNull(result.getFirstHeader("Via")); - } - @Test public void testReturns304ForIfNoneMatchPassesIfRequestServedFromOrigin() throws Exception { 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 73c02017d..404a99b72 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 @@ -36,8 +36,6 @@ import java.time.temporal.ChronoUnit; import java.util.Iterator; import java.util.List; import java.util.Random; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import org.apache.hc.client5.http.HttpRoute; import org.apache.hc.client5.http.auth.StandardAuthScheme; @@ -55,7 +53,6 @@ import org.apache.hc.core5.http.HttpException; import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpStatus; -import org.apache.hc.core5.http.HttpVersion; import org.apache.hc.core5.http.io.support.ClassicRequestBuilder; import org.apache.hc.core5.http.message.BasicClassicHttpRequest; import org.apache.hc.core5.http.message.BasicClassicHttpResponse; @@ -2261,129 +2258,4 @@ public class TestProtocolRequirements { Assertions.assertEquals(server, result.getFirstHeader("Server").getValue()); } - @Test - public void testProperlyFormattedViaHeaderIsAddedToRequests() throws Exception { - request.removeHeaders(HttpHeaders.VIA); - Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(originResponse); - - execute(request); - - final ArgumentCaptor reqCapture = ArgumentCaptor.forClass(ClassicHttpRequest.class); - Mockito.verify(mockExecChain).proceed(reqCapture.capture(), Mockito.any()); - - final ClassicHttpRequest captured = reqCapture.getValue(); - final String via = captured.getFirstHeader(HttpHeaders.VIA).getValue(); - assertValidViaHeader(via); - } - - @Test - public void testProperlyFormattedViaHeaderIsAddedToResponses() throws Exception { - originResponse.removeHeaders(HttpHeaders.VIA); - Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(originResponse); - final ClassicHttpResponse result = execute(request); - assertValidViaHeader(result.getFirstHeader(HttpHeaders.VIA).getValue()); - } - - private void assertValidViaHeader(final String via) { - // Via = HttpHeaders.VIA ":" 1#( received-protocol received-by [ comment ] ) - // received-protocol = [ protocol-name "/" ] protocol-version - // protocol-name = token - // protocol-version = token - // received-by = ( host [ ":" port ] ) | pseudonym - // pseudonym = token - - final String[] parts = via.split("\\s+"); - Assertions.assertTrue(parts.length >= 2); - - // received protocol - final String receivedProtocol = parts[0]; - final String[] protocolParts = receivedProtocol.split("/"); - Assertions.assertTrue(protocolParts.length >= 1); - Assertions.assertTrue(protocolParts.length <= 2); - - final String tokenRegexp = "[^\\p{Cntrl}()<>@,;:\\\\\"/\\[\\]?={} \\t]+"; - for(final String protocolPart : protocolParts) { - Assertions.assertTrue(Pattern.matches(tokenRegexp, protocolPart)); - } - - // received-by - if (!Pattern.matches(tokenRegexp, parts[1])) { - // host : port - new HttpHost(parts[1]); // TODO - unused - is this a test bug? else use Assertions.assertNotNull - } - - // comment - if (parts.length > 2) { - final StringBuilder buf = new StringBuilder(parts[2]); - for(int i=3; i reqCapture = ArgumentCaptor.forClass(ClassicHttpRequest.class); - Mockito.verify(mockExecChain).proceed(reqCapture.capture(), Mockito.any()); - - final ClassicHttpRequest captured = reqCapture.getValue(); - final String via = captured.getFirstHeader(HttpHeaders.VIA).getValue(); - final String protocol = via.split("\\s+")[0]; - final String[] protoParts = protocol.split("/"); - if (protoParts.length > 1) { - Assertions.assertTrue("http".equalsIgnoreCase(protoParts[0])); - } - Assertions.assertEquals("1.0",protoParts[protoParts.length-1]); - } - - @Test - public void testViaHeaderOnResponseProperlyRecordsOriginProtocol() throws Exception { - - originResponse = new BasicClassicHttpResponse(HttpStatus.SC_NO_CONTENT, "No Content"); - originResponse.setVersion(HttpVersion.HTTP_1_0); - - Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(originResponse); - - final ClassicHttpResponse result = execute(request); - - final String via = result.getFirstHeader(HttpHeaders.VIA).getValue(); - final String protocol = via.split("\\s+")[0]; - final String[] protoParts = protocol.split("/"); - Assertions.assertTrue(protoParts.length >= 1); - Assertions.assertTrue(protoParts.length <= 2); - if (protoParts.length > 1) { - Assertions.assertTrue("http".equalsIgnoreCase(protoParts[0])); - } - Assertions.assertEquals("1.0", protoParts[protoParts.length - 1]); - } - } diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestViaCacheGenerator.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestViaCacheGenerator.java new file mode 100644 index 000000000..83fff1a65 --- /dev/null +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestViaCacheGenerator.java @@ -0,0 +1,75 @@ +/* + * ==================================================================== + * 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.impl.cache; + + +import org.apache.hc.core5.http.HttpVersion; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class TestViaCacheGenerator { + + private ViaCacheGenerator impl; + + @BeforeEach + public void setUp() { + impl = new ViaCacheGenerator(); + } + + @Test + public void testViaValueGeneration() { + Assertions.assertEquals("1.1 localhost (Apache-HttpClient/UNAVAILABLE (cache))", + impl.generateViaHeader(null, HttpVersion.DEFAULT)); + Assertions.assertEquals("2.0 localhost (Apache-HttpClient/UNAVAILABLE (cache))", + impl.generateViaHeader(null, HttpVersion.HTTP_2)); + } + + @Test + public void testViaValueLookup() { + MatcherAssert.assertThat(impl.lookup(HttpVersion.DEFAULT), + Matchers.startsWith("1.1 localhost (Apache-HttpClient/")); + MatcherAssert.assertThat(impl.lookup(HttpVersion.HTTP_1_0), + Matchers.startsWith("1.0 localhost (Apache-HttpClient/")); + MatcherAssert.assertThat(impl.lookup(HttpVersion.HTTP_1_1), + Matchers.startsWith("1.1 localhost (Apache-HttpClient/")); + MatcherAssert.assertThat(impl.lookup(HttpVersion.HTTP_2), + Matchers.startsWith("2.0 localhost (Apache-HttpClient/")); + MatcherAssert.assertThat(impl.lookup(HttpVersion.HTTP_2_0), + Matchers.startsWith("2.0 localhost (Apache-HttpClient/")); + MatcherAssert.assertThat(impl.lookup(HttpVersion.HTTP_1_0), + Matchers.startsWith("1.0 localhost (Apache-HttpClient/")); + MatcherAssert.assertThat(impl.lookup(HttpVersion.HTTP_1_1), + Matchers.startsWith("1.1 localhost (Apache-HttpClient/")); + MatcherAssert.assertThat(impl.lookup(HttpVersion.HTTP_2_0), + Matchers.startsWith("2.0 localhost (Apache-HttpClient/")); + Assertions.assertEquals(3, impl.internalCache.size()); + } + +}