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/09/19 13:43:40 UTC
[httpcomponents-client] branch master updated: HTTPCLIENT-2116:
Incorrect request message composition when routing requests via a proxy
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 ae67250 HTTPCLIENT-2116: Incorrect request message composition when routing requests via a proxy
ae67250 is described below
commit ae6725056cc4eac6606931471866ce41dbe4ae74
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Wed Sep 16 11:23:09 2020 +0200
HTTPCLIENT-2116: Incorrect request message composition when routing requests via a proxy
---
.../hc/client5/http/impl/ProtocolSupport.java | 71 ++++++++++++++++++++++
.../client5/http/impl/async/AsyncProtocolExec.java | 13 ++--
.../client5/http/impl/async/HttpProxyRequest.java | 58 ++++++++++++++++++
.../http/impl/classic/ClassicHttpProxyRequest.java | 59 ++++++++++++++++++
.../hc/client5/http/impl/classic/ProtocolExec.java | 15 +++--
.../hc/client5/http/impl/TestProtocolSupport.java | 55 +++++++++++++++++
.../http/impl/classic/TestProtocolExec.java | 14 ++---
7 files changed, 267 insertions(+), 18 deletions(-)
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/ProtocolSupport.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/ProtocolSupport.java
new file mode 100644
index 0000000..70f820c
--- /dev/null
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/ProtocolSupport.java
@@ -0,0 +1,71 @@
+/*
+ * ====================================================================
+ * 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
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hc.client5.http.impl;
+
+import org.apache.hc.core5.annotation.Internal;
+import org.apache.hc.core5.http.HttpRequest;
+import org.apache.hc.core5.http.URIScheme;
+import org.apache.hc.core5.net.URIAuthority;
+
+/**
+ * Protocol support methods.
+ *
+ * @since 5.1
+ */
+@Internal
+public final class ProtocolSupport {
+
+ public static String getRequestUri(final HttpRequest request) {
+ final URIAuthority authority = request.getAuthority();
+ if (authority != null) {
+ final StringBuilder buf = new StringBuilder();
+ final String scheme = request.getScheme();
+ buf.append(scheme != null ? scheme : URIScheme.HTTP.id);
+ buf.append("://");
+ if (authority.getUserInfo() != null) {
+ buf.append(authority.getUserInfo());
+ buf.append("@");
+ }
+ buf.append(authority.getHostName());
+ if (authority.getPort() != -1) {
+ buf.append(":");
+ buf.append(authority.getPort());
+ }
+ final String path = request.getPath();
+ if (path == null || !path.startsWith("/")) {
+ buf.append("/");
+ }
+ if (path != null) {
+ buf.append(path);
+ }
+ return buf.toString();
+ } else {
+ return request.getPath();
+ }
+ }
+
+}
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java
index 04fc486..bcf4341 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java
@@ -102,31 +102,34 @@ public final class AsyncProtocolExec implements AsyncExecChainHandler {
@Override
public void execute(
- final HttpRequest request,
+ final HttpRequest userRequest,
final AsyncEntityProducer entityProducer,
final AsyncExecChain.Scope scope,
final AsyncExecChain chain,
final AsyncExecCallback asyncExecCallback) throws HttpException, IOException {
- if (Method.CONNECT.isSame(request.getMethod())) {
+ if (Method.CONNECT.isSame(userRequest.getMethod())) {
throw new ProtocolException("Direct execution of CONNECT is not allowed");
}
final HttpRoute route = scope.route;
final HttpClientContext clientContext = scope.clientContext;
+ final HttpRequest request;
if (route.getProxyHost() != null && !route.isTunnelled()) {
try {
- URI uri = request.getUri();
+ URI uri = userRequest.getUri();
if (!uri.isAbsolute()) {
uri = URIUtils.rewriteURI(uri, route.getTargetHost(), true);
} else {
uri = URIUtils.rewriteURI(uri);
}
- request.setPath(uri.toASCIIString());
+ request = HttpProxyRequest.rewrite(userRequest, uri);
} catch (final URISyntaxException ex) {
- throw new ProtocolException("Invalid request URI: " + request.getRequestUri(), ex);
+ throw new ProtocolException("Invalid request URI: " + userRequest.getRequestUri(), ex);
}
+ } else {
+ request = userRequest;
}
final URIAuthority authority = request.getAuthority();
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpProxyRequest.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpProxyRequest.java
new file mode 100644
index 0000000..e4ed272
--- /dev/null
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpProxyRequest.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
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.hc.client5.http.impl.async;
+
+import java.net.URI;
+import java.util.Iterator;
+
+import org.apache.hc.client5.http.impl.ProtocolSupport;
+import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http.HttpRequest;
+import org.apache.hc.core5.http.message.BasicHttpRequest;
+
+class HttpProxyRequest extends BasicHttpRequest {
+
+ static HttpProxyRequest rewrite(final HttpRequest original, final URI requestUri) {
+ final HttpProxyRequest copy = new HttpProxyRequest(original.getMethod(), requestUri);
+ copy.setVersion(original.getVersion());
+ for (final Iterator<Header> it = original.headerIterator(); it.hasNext(); ) {
+ copy.addHeader(it.next());
+ }
+ return copy;
+ }
+
+ private HttpProxyRequest(final String method, final URI requestUri) {
+ super(method, requestUri);
+ }
+
+ @Override
+ public String getRequestUri() {
+ return ProtocolSupport.getRequestUri(this);
+ }
+
+}
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ClassicHttpProxyRequest.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ClassicHttpProxyRequest.java
new file mode 100644
index 0000000..bfeec4a
--- /dev/null
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ClassicHttpProxyRequest.java
@@ -0,0 +1,59 @@
+/*
+ * ====================================================================
+ * 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
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.hc.client5.http.impl.classic;
+
+import java.net.URI;
+import java.util.Iterator;
+
+import org.apache.hc.client5.http.impl.ProtocolSupport;
+import org.apache.hc.core5.http.ClassicHttpRequest;
+import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http.message.BasicClassicHttpRequest;
+
+class ClassicHttpProxyRequest extends BasicClassicHttpRequest {
+
+ static ClassicHttpProxyRequest rewrite(final ClassicHttpRequest original, final URI requestUri) {
+ final ClassicHttpProxyRequest copy = new ClassicHttpProxyRequest(original.getMethod(), requestUri);
+ copy.setVersion(original.getVersion());
+ for (final Iterator<Header> it = original.headerIterator(); it.hasNext(); ) {
+ copy.addHeader(it.next());
+ }
+ copy.setEntity(original.getEntity());
+ return copy;
+ }
+
+ private ClassicHttpProxyRequest(final String method, final URI requestUri) {
+ super(method, requestUri);
+ }
+
+ @Override
+ public String getRequestUri() {
+ return ProtocolSupport.getRequestUri(this);
+ }
+
+}
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java
index 877ce7b..931202f 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java
@@ -101,13 +101,13 @@ public final class ProtocolExec implements ExecChainHandler {
@Override
public ClassicHttpResponse execute(
- final ClassicHttpRequest request,
+ final ClassicHttpRequest userRequest,
final ExecChain.Scope scope,
final ExecChain chain) throws IOException, HttpException {
- Args.notNull(request, "HTTP request");
+ Args.notNull(userRequest, "HTTP request");
Args.notNull(scope, "Scope");
- if (Method.CONNECT.isSame(request.getMethod())) {
+ if (Method.CONNECT.isSame(userRequest.getMethod())) {
throw new ProtocolException("Direct execution of CONNECT is not allowed");
}
@@ -122,18 +122,21 @@ public final class ProtocolExec implements ExecChainHandler {
final AuthExchange proxyAuthExchange = proxy != null ? context.getAuthExchange(proxy) : new AuthExchange();
try {
+ final ClassicHttpRequest request;
if (proxy != null && !route.isTunnelled()) {
try {
- URI uri = request.getUri();
+ URI uri = userRequest.getUri();
if (!uri.isAbsolute()) {
uri = URIUtils.rewriteURI(uri, target, true);
} else {
uri = URIUtils.rewriteURI(uri);
}
- request.setPath(uri.toASCIIString());
+ request = ClassicHttpProxyRequest.rewrite(userRequest, uri);
} catch (final URISyntaxException ex) {
- throw new ProtocolException("Invalid request URI: " + request.getRequestUri(), ex);
+ throw new ProtocolException("Invalid request URI: " + userRequest.getRequestUri(), ex);
}
+ } else {
+ request = userRequest;
}
final URIAuthority authority = request.getAuthority();
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestProtocolSupport.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestProtocolSupport.java
new file mode 100644
index 0000000..11c8315
--- /dev/null
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestProtocolSupport.java
@@ -0,0 +1,55 @@
+/*
+ * ====================================================================
+ * 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
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hc.client5.http.impl;
+
+import org.apache.hc.core5.http.HttpRequest;
+import org.apache.hc.core5.http.Method;
+import org.apache.hc.core5.http.message.BasicHttpRequest;
+import org.apache.hc.core5.net.URIAuthority;
+import org.hamcrest.CoreMatchers;
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Simple tests for {@link ProtocolSupport}.
+ */
+public class TestProtocolSupport {
+
+ @Test
+ public void testGetRequestUri() {
+ final HttpRequest request = new BasicHttpRequest(Method.GET, "");
+ Assert.assertThat(ProtocolSupport.getRequestUri(request), CoreMatchers.equalTo("/"));
+ request.setAuthority(new URIAuthority("testUser", "localhost", 8080));
+ Assert.assertThat(ProtocolSupport.getRequestUri(request), CoreMatchers.equalTo("http://testUser@localhost:8080/"));
+ request.setScheme("https");
+ Assert.assertThat(ProtocolSupport.getRequestUri(request), CoreMatchers.equalTo("https://testUser@localhost:8080/"));
+ request.setPath("blah");
+ Assert.assertThat(ProtocolSupport.getRequestUri(request), CoreMatchers.equalTo("https://testUser@localhost:8080/blah"));
+ request.setPath("/blah/blah");
+ Assert.assertThat(ProtocolSupport.getRequestUri(request), CoreMatchers.equalTo("https://testUser@localhost:8080/blah/blah"));
+ }
+}
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java
index 38619df..f410d3d 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java
@@ -39,11 +39,11 @@ import org.apache.hc.client5.http.HttpRoute;
import org.apache.hc.client5.http.auth.AuthChallenge;
import org.apache.hc.client5.http.auth.AuthExchange;
import org.apache.hc.client5.http.auth.AuthScheme;
-import org.apache.hc.client5.http.auth.StandardAuthScheme;
import org.apache.hc.client5.http.auth.AuthScope;
import org.apache.hc.client5.http.auth.ChallengeType;
import org.apache.hc.client5.http.auth.Credentials;
import org.apache.hc.client5.http.auth.CredentialsProvider;
+import org.apache.hc.client5.http.auth.StandardAuthScheme;
import org.apache.hc.client5.http.auth.UsernamePasswordCredentials;
import org.apache.hc.client5.http.classic.ExecChain;
import org.apache.hc.client5.http.classic.ExecRuntime;
@@ -250,7 +250,7 @@ public class TestProtocolExec {
@Test
public void testExecEntityEnclosingRequestRetryOnAuthChallenge() throws Exception {
- final HttpRoute route = new HttpRoute(target, proxy);
+ final HttpRoute route = new HttpRoute(target);
final ClassicHttpRequest request = new HttpGet("http://foo/test");
final ClassicHttpResponse response1 = new BasicClassicHttpResponse(401, "Huh?");
response1.setHeader(HttpHeaders.WWW_AUTHENTICATE, StandardAuthScheme.BASIC + " realm=test");
@@ -266,10 +266,10 @@ public class TestProtocolExec {
final HttpClientContext context = new HttpClientContext();
- final AuthExchange proxyAuthExchange = new AuthExchange();
- proxyAuthExchange.setState(AuthExchange.State.SUCCESS);
- proxyAuthExchange.select(new NTLMScheme());
- context.setAuthExchange(proxy, proxyAuthExchange);
+ final AuthExchange authExchange = new AuthExchange();
+ authExchange.setState(AuthExchange.State.SUCCESS);
+ authExchange.select(new NTLMScheme());
+ context.setAuthExchange(target, authExchange);
final BasicCredentialsProvider credentialsProvider = new BasicCredentialsProvider();
credentialsProvider.setCredentials(new AuthScope(target), new UsernamePasswordCredentials("user", "pass".toCharArray()));
@@ -292,7 +292,7 @@ public class TestProtocolExec {
Assert.assertNotNull(finalResponse);
Assert.assertEquals(200, finalResponse.getCode());
- Assert.assertNull(proxyAuthExchange.getAuthScheme());
+ Assert.assertNotNull(authExchange.getAuthScheme());
}
@Test