DefaultRoutePlanner to take request authority into account when determining a route for HTTPS requests

This commit is contained in:
Oleg Kalnichevski 2024-01-25 14:08:31 +01:00
parent 528a8c050b
commit 91ab690e50
8 changed files with 97 additions and 14 deletions

View File

@ -184,7 +184,7 @@ abstract class InternalAbstractHttpAsyncClient extends AbstractHttpAsyncClientBa
abstract AsyncExecRuntime createAsyncExecRuntime(HandlerFactory<AsyncPushConsumer> pushHandlerFactory);
abstract HttpRoute determineRoute(HttpHost httpHost, HttpClientContext clientContext) throws HttpException;
abstract HttpRoute determineRoute(HttpHost httpHost, HttpRequest request, HttpClientContext clientContext) throws HttpException;
@Override
protected <T> Future<T> doExecute(
@ -214,6 +214,7 @@ abstract class InternalAbstractHttpAsyncClient extends AbstractHttpAsyncClientBa
final HttpRoute route = determineRoute(
httpHost != null ? httpHost : RoutingSupport.determineHost(request),
request,
clientContext);
final String exchangeId = ExecSupport.getNextExchangeId();
clientContext.setExchangeId(exchangeId);

View File

@ -44,6 +44,7 @@ import org.apache.hc.core5.annotation.Internal;
import org.apache.hc.core5.annotation.ThreadingBehavior;
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.config.Lookup;
import org.apache.hc.core5.http.nio.AsyncPushConsumer;
import org.apache.hc.core5.http.nio.HandlerFactory;
@ -94,8 +95,8 @@ public final class InternalH2AsyncClient extends InternalAbstractHttpAsyncClient
}
@Override
HttpRoute determineRoute(final HttpHost httpHost, final HttpClientContext clientContext) throws HttpException {
final HttpRoute route = routePlanner.determineRoute(httpHost, clientContext);
HttpRoute determineRoute(final HttpHost httpHost, final HttpRequest request, final HttpClientContext clientContext) throws HttpException {
final HttpRoute route = routePlanner.determineRoute(httpHost, request, clientContext);
if (route.isTunnelled()) {
throw new HttpException("HTTP/2 tunneling not supported");
}

View File

@ -46,6 +46,7 @@ import org.apache.hc.core5.annotation.Internal;
import org.apache.hc.core5.annotation.ThreadingBehavior;
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.config.Lookup;
import org.apache.hc.core5.http.nio.AsyncPushConsumer;
import org.apache.hc.core5.http.nio.HandlerFactory;
@ -100,8 +101,8 @@ public final class InternalHttpAsyncClient extends InternalAbstractHttpAsyncClie
}
@Override
HttpRoute determineRoute(final HttpHost httpHost, final HttpClientContext clientContext) throws HttpException {
return routePlanner.determineRoute(httpHost, clientContext);
HttpRoute determineRoute(final HttpHost httpHost, final HttpRequest request, final HttpClientContext clientContext) throws HttpException {
return routePlanner.determineRoute(httpHost, request, clientContext);
}
}

View File

@ -55,6 +55,7 @@ import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
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.config.Lookup;
import org.apache.hc.core5.http.impl.io.HttpRequestExecutor;
import org.apache.hc.core5.http.io.support.ClassicRequestBuilder;
@ -116,8 +117,8 @@ class InternalHttpClient extends CloseableHttpClient implements Configurable {
this.closeables = closeables != null ? new ConcurrentLinkedQueue<>(closeables) : null;
}
private HttpRoute determineRoute(final HttpHost target, final HttpContext context) throws HttpException {
return this.routePlanner.determineRoute(target, context);
private HttpRoute determineRoute(final HttpHost target, final HttpRequest request, final HttpContext context) throws HttpException {
return this.routePlanner.determineRoute(target, request, context);
}
private void setupContext(final HttpClientContext context) {
@ -157,6 +158,7 @@ class InternalHttpClient extends CloseableHttpClient implements Configurable {
setupContext(localcontext);
final HttpRoute route = determineRoute(
target != null ? target : RoutingSupport.determineHost(request),
request,
localcontext);
final String exchangeId = ExecSupport.getNextExchangeId();
localcontext.setExchangeId(exchangeId);

View File

@ -28,6 +28,7 @@
package org.apache.hc.client5.http.impl.routing;
import java.net.InetAddress;
import java.util.Objects;
import org.apache.hc.client5.http.HttpRoute;
import org.apache.hc.client5.http.SchemePortResolver;
@ -40,9 +41,12 @@ import org.apache.hc.core5.annotation.Contract;
import org.apache.hc.core5.annotation.ThreadingBehavior;
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.ProtocolException;
import org.apache.hc.core5.http.URIScheme;
import org.apache.hc.core5.http.protocol.HttpContext;
import org.apache.hc.core5.net.NamedEndpoint;
import org.apache.hc.core5.net.URIAuthority;
/**
* Default implementation of an {@link HttpRoutePlanner}. It will not make use of
@ -60,8 +64,15 @@ public class DefaultRoutePlanner implements HttpRoutePlanner {
this.schemePortResolver = schemePortResolver != null ? schemePortResolver : DefaultSchemePortResolver.INSTANCE;
}
private static boolean sameNamedEndpoint(final NamedEndpoint n1, final NamedEndpoint n2) {
if (n1 == null || n2 == null) {
return false;
}
return Objects.equals(n1.getHostName(), n2.getHostName()) && n1.getPort() == n2.getPort();
}
@Override
public final HttpRoute determineRoute(final HttpHost host, final HttpContext context) throws HttpException {
public final HttpRoute determineRoute(final HttpHost host, final HttpRequest request, final HttpContext context) throws HttpException {
if (host == null) {
throw new ProtocolException("Target host is not specified");
}
@ -76,11 +87,24 @@ public class DefaultRoutePlanner implements HttpRoutePlanner {
if (target.getPort() < 0) {
throw new ProtocolException("Unroutable protocol scheme: " + target);
}
final boolean secure = target.getSchemeName().equalsIgnoreCase(URIScheme.HTTPS.getId());
if (proxy == null) {
return new HttpRoute(target, determineLocalAddress(target, context), secure);
final boolean secure = URIScheme.HTTPS.same(target.getSchemeName());
final URIAuthority authority;
if (secure && request != null && !sameNamedEndpoint(request.getAuthority(), host)) {
authority = request.getAuthority();
} else {
authority = null;
}
return new HttpRoute(target, determineLocalAddress(proxy, context), proxy, secure);
final InetAddress inetAddress = determineLocalAddress(target, context);
if (proxy == null) {
return new HttpRoute(target, authority, inetAddress, secure);
}
return new HttpRoute(target, authority, inetAddress, proxy, secure);
}
@Override
public final HttpRoute determineRoute(final HttpHost host, final HttpContext context) throws HttpException {
return determineRoute(host, null, context);
}
/**

View File

@ -32,6 +32,7 @@ import org.apache.hc.core5.annotation.Contract;
import org.apache.hc.core5.annotation.ThreadingBehavior;
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.protocol.HttpContext;
/**
@ -62,4 +63,20 @@ public interface HttpRoutePlanner {
*/
HttpRoute determineRoute(HttpHost target, HttpContext context) throws HttpException;
/**
* Determines the route for the given host.
*
* @param target the target host for the request.
* @param request the request message. Can be {@code null} if not given / known.
* @param context the context to use for the subsequent execution.
* Implementations may accept {@code null}.
*
* @return the route that the request should take
*
* @since 5.4
*/
default HttpRoute determineRoute(HttpHost target, HttpRequest request, HttpContext context) throws HttpException {
return determineRoute(target, context);
}
}

View File

@ -57,7 +57,6 @@ import org.mockito.MockitoAnnotations;
/**
* Simple tests for {@link InternalHttpClient}.
*/
@SuppressWarnings({"static-access"}) // test code
public class TestInternalHttpClient {
@Mock
@ -101,6 +100,7 @@ public class TestInternalHttpClient {
Mockito.when(routePlanner.determineRoute(
Mockito.eq(new HttpHost("somehost")),
Mockito.any(),
Mockito.<HttpClientContext>any())).thenReturn(route);
Mockito.when(execChain.execute(
Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(
@ -121,6 +121,7 @@ public class TestInternalHttpClient {
Mockito.when(routePlanner.determineRoute(
Mockito.eq(new HttpHost("somehost")),
Mockito.any(),
Mockito.<HttpClientContext>any())).thenReturn(route);
Mockito.when(execChain.execute(
Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(
@ -141,6 +142,7 @@ public class TestInternalHttpClient {
Mockito.when(routePlanner.determineRoute(
Mockito.eq(new HttpHost("somehost")),
Mockito.any(),
Mockito.<HttpClientContext>any())).thenReturn(route);
Mockito.when(execChain.execute(
Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(
@ -163,6 +165,7 @@ public class TestInternalHttpClient {
Mockito.when(routePlanner.determineRoute(
Mockito.eq(new HttpHost("somehost")),
Mockito.any(),
Mockito.<HttpClientContext>any())).thenReturn(route);
Mockito.when(execChain.execute(
Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(
@ -183,6 +186,7 @@ public class TestInternalHttpClient {
Mockito.when(routePlanner.determineRoute(
Mockito.eq(new HttpHost("somehost")),
Mockito.any(),
Mockito.<HttpClientContext>any())).thenReturn(route);
Mockito.when(execChain.execute(
Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(

View File

@ -32,9 +32,12 @@ import org.apache.hc.client5.http.SchemePortResolver;
import org.apache.hc.client5.http.config.RequestConfig;
import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.ProtocolException;
import org.apache.hc.core5.http.message.BasicHttpRequest;
import org.apache.hc.core5.http.protocol.BasicHttpContext;
import org.apache.hc.core5.http.protocol.HttpContext;
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;
@ -43,7 +46,6 @@ import org.mockito.Mockito;
/**
* Tests for {@link DefaultRoutePlanner}.
*/
@SuppressWarnings({"boxing","static-access"}) // test code
public class TestDefaultRoutePlanner {
private SchemePortResolver schemePortResolver;
@ -105,4 +107,35 @@ public class TestDefaultRoutePlanner {
routePlanner.determineRoute(null, context));
}
@Test
public void testVirtualSecureHost() throws Exception {
final HttpHost target = new HttpHost("https", "somehost", 443);
final URIAuthority virtualHost = new URIAuthority("someotherhost", 443);
final HttpRequest request = new BasicHttpRequest("https", "GET", virtualHost, "/");
final HttpContext context = new BasicHttpContext();
final HttpRoute route = routePlanner.determineRoute(target, request, context);
Assertions.assertEquals(target, route.getTargetHost());
Assertions.assertEquals(virtualHost, route.getTargetName());
Assertions.assertEquals(1, route.getHopCount());
Assertions.assertTrue(route.isSecure());
Mockito.verify(schemePortResolver, Mockito.never()).resolve(Mockito.any());
}
@Test
public void testVirtualInsecureHost() throws Exception {
final HttpHost target = new HttpHost("http", "somehost", 80);
final URIAuthority virtualHost = new URIAuthority("someotherhost", 80);
final HttpRequest request = new BasicHttpRequest("http", "GET", virtualHost, "/");
final HttpContext context = new BasicHttpContext();
final HttpRoute route = routePlanner.determineRoute(target, request, context);
Assertions.assertEquals(target, route.getTargetHost());
Assertions.assertNull(route.getTargetName());
Assertions.assertEquals(1, route.getHopCount());
Assertions.assertFalse(route.isSecure());
Mockito.verify(schemePortResolver, Mockito.never()).resolve(Mockito.any());
}
}