You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2020/03/17 13:51:32 UTC
[httpcomponents-client] branch master updated: HTTPCLIENT-2061:
corrected sequence of request execution interceptors in classic HttpClient
This is an automated email from the ASF dual-hosted git repository.
olegk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git
The following commit(s) were added to refs/heads/master by this push:
new 98daeab HTTPCLIENT-2061: corrected sequence of request execution interceptors in classic HttpClient
98daeab is described below
commit 98daeabc0541939b2118044be5aaaa3e29fe2a38
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Sun Mar 15 10:32:54 2020 +0100
HTTPCLIENT-2061: corrected sequence of request execution interceptors in classic HttpClient
---
.../hc/client5/testing/sync/TestRedirects.java | 48 ++++++++++++++++++++++
.../apache/hc/client5/http/impl/ChainElement.java | 2 +-
.../http/impl/classic/HttpClientBuilder.java | 28 ++++++-------
3 files changed, 62 insertions(+), 16 deletions(-)
diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java
index 78b12b1..8ea330a 100644
--- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java
+++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java
@@ -30,6 +30,8 @@ import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Arrays;
+import java.util.Queue;
+import java.util.concurrent.ConcurrentLinkedQueue;
import org.apache.hc.client5.http.CircularRedirectException;
import org.apache.hc.client5.http.ClientProtocolException;
@@ -63,6 +65,7 @@ import org.apache.hc.core5.http.io.entity.EntityUtils;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.hc.core5.http.message.BasicHeader;
import org.apache.hc.core5.http.protocol.HttpContext;
+import org.hamcrest.CoreMatchers;
import org.junit.Assert;
import org.junit.Test;
@@ -657,4 +660,49 @@ public class TestRedirects extends LocalServerTestBase {
}
}
+ @Test
+ public void testCompressionHeaderRedirect() throws Exception {
+ final Queue<String> values = new ConcurrentLinkedQueue<>();
+ final HttpHost target = start(null, new Decorator<HttpServerRequestHandler>() {
+
+ @Override
+ public HttpServerRequestHandler decorate(final HttpServerRequestHandler requestHandler) {
+ return new RedirectingDecorator(
+ requestHandler,
+ new OldPathRedirectResolver("/oldlocation", "/random", HttpStatus.SC_MOVED_TEMPORARILY)) {
+
+ @Override
+ public void handle(final ClassicHttpRequest request,
+ final ResponseTrigger responseTrigger,
+ final HttpContext context) throws HttpException, IOException {
+ final Header header = request.getHeader(HttpHeaders.ACCEPT_ENCODING);
+ if (header != null) {
+ values.add(header.getValue());
+ }
+ super.handle(request, responseTrigger, context);
+ }
+
+ };
+ }
+
+ });
+
+ final HttpClientContext context = HttpClientContext.create();
+
+ final HttpGet httpget = new HttpGet("/oldlocation/100");
+
+ try (final ClassicHttpResponse response = this.httpclient.execute(target, httpget, context)) {
+ final HttpRequest reqWrapper = context.getRequest();
+
+ Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
+ Assert.assertEquals(URIUtils.create(target, "/random/100"), reqWrapper.getUri());
+
+ EntityUtils.consume(response.getEntity());
+ }
+
+ Assert.assertThat(values.poll(), CoreMatchers.equalTo("gzip, x-gzip, deflate"));
+ Assert.assertThat(values.poll(), CoreMatchers.equalTo("gzip, x-gzip, deflate"));
+ Assert.assertThat(values.poll(), CoreMatchers.nullValue());
+ }
+
}
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/ChainElement.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/ChainElement.java
index 0c34f83..5ebcf4b 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/ChainElement.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/ChainElement.java
@@ -34,6 +34,6 @@ package org.apache.hc.client5.http.impl;
*/
public enum ChainElement {
- REDIRECT, BACK_OFF, RETRY, CACHING, PROTOCOL, CONNECT, MAIN_TRANSPORT
+ REDIRECT, COMPRESS, BACK_OFF, RETRY, CACHING, PROTOCOL, CONNECT, MAIN_TRANSPORT
}
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java
index 573ebb0..49e223e 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java
@@ -871,17 +871,6 @@ public class HttpClientBuilder {
}
}
- // Add redirect executor, if not disabled
- if (!redirectHandlingDisabled) {
- RedirectStrategy redirectStrategyCopy = this.redirectStrategy;
- if (redirectStrategyCopy == null) {
- redirectStrategyCopy = DefaultRedirectStrategy.INSTANCE;
- }
- execChainDefinition.addFirst(
- new RedirectExec(routePlannerCopy, redirectStrategyCopy),
- ChainElement.REDIRECT.name());
- }
-
if (!contentCompressionDisabled) {
if (contentDecoderMap != null) {
final List<String> encodings = new ArrayList<>(contentDecoderMap.keySet());
@@ -892,14 +881,23 @@ public class HttpClientBuilder {
final Registry<InputStreamFactory> decoderRegistry = b2.build();
execChainDefinition.addFirst(
new ContentCompressionExec(encodings, decoderRegistry, true),
- ChainElement.REDIRECT.name());
+ ChainElement.COMPRESS.name());
} else {
- execChainDefinition.addFirst(
- new ContentCompressionExec(true),
- ChainElement.REDIRECT.name());
+ execChainDefinition.addFirst(new ContentCompressionExec(true), ChainElement.COMPRESS.name());
}
}
+ // Add redirect executor, if not disabled
+ if (!redirectHandlingDisabled) {
+ RedirectStrategy redirectStrategyCopy = this.redirectStrategy;
+ if (redirectStrategyCopy == null) {
+ redirectStrategyCopy = DefaultRedirectStrategy.INSTANCE;
+ }
+ execChainDefinition.addFirst(
+ new RedirectExec(routePlannerCopy, redirectStrategyCopy),
+ ChainElement.REDIRECT.name());
+ }
+
// Optionally, add connection back-off executor
if (this.backoffManager != null && this.connectionBackoffStrategy != null) {
execChainDefinition.addFirst(new BackoffStrategyExec(this.connectionBackoffStrategy, this.backoffManager),