From 528a8c050b91315c56e0c9ec954c8e3435b89891 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Thu, 25 Jan 2024 11:57:02 +0100 Subject: [PATCH] RouteInfo and HttpRoute to include the target name from the URI authority in case it differs from the target host (the host recognizes multiple authorities) --- .../org/apache/hc/client5/http/HttpRoute.java | 105 ++++++++++++++++-- .../org/apache/hc/client5/http/RouteInfo.java | 10 ++ .../http/{routing => }/TestHttpRoute.java | 65 ++--------- 3 files changed, 115 insertions(+), 65 deletions(-) rename httpclient5/src/test/java/org/apache/hc/client5/http/{routing => }/TestHttpRoute.java (89%) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/HttpRoute.java b/httpclient5/src/main/java/org/apache/hc/client5/http/HttpRoute.java index ad2cf77e1..6def13992 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/HttpRoute.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/HttpRoute.java @@ -38,6 +38,7 @@ import java.util.Objects; import org.apache.hc.core5.annotation.Contract; import org.apache.hc.core5.annotation.ThreadingBehavior; import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.net.NamedEndpoint; import org.apache.hc.core5.util.Args; import org.apache.hc.core5.util.LangUtils; @@ -52,6 +53,9 @@ public final class HttpRoute implements RouteInfo, Cloneable { /** The target host to connect to. */ private final HttpHost targetHost; + /** The target name, if different from the target host, {@code null} otherwise. */ + private final NamedEndpoint targetName; + /** * The local address to connect from. * {@code null} indicates that the default should be used. @@ -70,10 +74,16 @@ public final class HttpRoute implements RouteInfo, Cloneable { /** Whether the route is (supposed to be) secure. */ private final boolean secure; - private HttpRoute(final HttpHost targetHost, final InetAddress local, final List proxies, - final boolean secure, final TunnelType tunnelled, final LayerType layered) { + HttpRoute(final HttpHost targetHost, + final NamedEndpoint targetName, + final InetAddress local, + final List proxies, + final boolean secure, + final TunnelType tunnelled, + final LayerType layered) { Args.notNull(targetHost, "Target host"); Args.notNegative(targetHost.getPort(), "Target port"); + this.targetName = targetName; this.targetHost = targetHost; this.localAddress = local; if (proxies != null && !proxies.isEmpty()) { @@ -104,7 +114,29 @@ public final class HttpRoute implements RouteInfo, Cloneable { */ public HttpRoute(final HttpHost target, final InetAddress local, final HttpHost[] proxies, final boolean secure, final TunnelType tunnelled, final LayerType layered) { - this(target, local, proxies != null ? Arrays.asList(proxies) : null, + this(target, null, local, proxies != null ? Arrays.asList(proxies) : null, + secure, tunnelled, layered); + } + + /** + * Creates a new route with all attributes specified explicitly. + * + * @param target the host to which to route + * @param targetName the target targetName if differs from the target host. + * @param local the local address to route from, or + * {@code null} for the default + * @param proxies the proxy chain to use, or + * {@code null} for a direct route + * @param secure {@code true} if the route is (to be) secure, + * {@code false} otherwise + * @param tunnelled the tunnel type of this route + * @param layered the layering type of this route + * + * @since 5.4 + */ + public HttpRoute(final HttpHost target, final NamedEndpoint targetName, final InetAddress local, final HttpHost[] proxies, + final boolean secure, final TunnelType tunnelled, final LayerType layered) { + this(target, targetName, local, proxies != null ? Arrays.asList(proxies) : null, secure, tunnelled, layered); } @@ -127,7 +159,7 @@ public final class HttpRoute implements RouteInfo, Cloneable { */ public HttpRoute(final HttpHost target, final InetAddress local, final HttpHost proxy, final boolean secure, final TunnelType tunnelled, final LayerType layered) { - this(target, local, proxy != null ? Collections.singletonList(proxy) : null, + this(target, null, local, proxy != null ? Collections.singletonList(proxy) : null, secure, tunnelled, layered); } @@ -142,7 +174,23 @@ public final class HttpRoute implements RouteInfo, Cloneable { * {@code false} otherwise */ public HttpRoute(final HttpHost target, final InetAddress local, final boolean secure) { - this(target, local, Collections.emptyList(), secure, TunnelType.PLAIN, LayerType.PLAIN); + this(target, null, local, Collections.emptyList(), secure, TunnelType.PLAIN, LayerType.PLAIN); + } + + /** + * Creates a new direct route. That is a route without a proxy. + * + * @param target the host to which to route + * @param targetName the target targetName if differs from the target host. + * @param local the local address to route from, or + * {@code null} for the default + * @param secure {@code true} if the route is (to be) secure, + * {@code false} otherwise + * + * @since 5.4 + */ + public HttpRoute(final HttpHost target, final NamedEndpoint targetName, final InetAddress local, final boolean secure) { + this(target, targetName, local, Collections.emptyList(), secure, TunnelType.PLAIN, LayerType.PLAIN); } /** @@ -151,7 +199,30 @@ public final class HttpRoute implements RouteInfo, Cloneable { * @param target the host to which to route */ public HttpRoute(final HttpHost target) { - this(target, null, Collections.emptyList(), false, TunnelType.PLAIN, LayerType.PLAIN); + this(target, null, null, Collections.emptyList(), false, TunnelType.PLAIN, LayerType.PLAIN); + } + + /** + * Creates a new route through a proxy. + * When using this constructor, the {@code proxy} MUST be given. + * For convenience, it is assumed that a secure connection will be + * layered over a tunnel through the proxy. + * + * @param target the host to which to route + * @param targetName the target targetName if differs from the target host. + * @param local the local address to route from, or + * {@code null} for the default + * @param proxy the proxy to use + * @param secure {@code true} if the route is (to be) secure, + * {@code false} otherwise + * + * @since 5.4 + */ + public HttpRoute(final HttpHost target, final NamedEndpoint targetName, final InetAddress local, + final HttpHost proxy, final boolean secure) { + this(target, targetName, local, Collections.singletonList(Args.notNull(proxy, "Proxy host")), secure, + secure ? TunnelType.TUNNELLED : TunnelType.PLAIN, + secure ? LayerType.LAYERED : LayerType.PLAIN); } /** @@ -169,9 +240,9 @@ public final class HttpRoute implements RouteInfo, Cloneable { */ public HttpRoute(final HttpHost target, final InetAddress local, final HttpHost proxy, final boolean secure) { - this(target, local, Collections.singletonList(Args.notNull(proxy, "Proxy host")), secure, - secure ? TunnelType.TUNNELLED : TunnelType.PLAIN, - secure ? LayerType.LAYERED : LayerType.PLAIN); + this(target, null, local, Collections.singletonList(Args.notNull(proxy, "Proxy host")), secure, + secure ? TunnelType.TUNNELLED : TunnelType.PLAIN, + secure ? LayerType.LAYERED : LayerType.PLAIN); } /** @@ -191,6 +262,14 @@ public final class HttpRoute implements RouteInfo, Cloneable { return this.targetHost; } + /** + * @since 5.4 + */ + @Override + public NamedEndpoint getTargetName() { + return targetName; + } + @Override public InetAddress getLocalAddress() { return this.localAddress; @@ -267,6 +346,7 @@ public final class HttpRoute implements RouteInfo, Cloneable { (this.tunnelled == that.tunnelled) && (this.layered == that.layered) && Objects.equals(this.targetHost, that.targetHost) && + Objects.equals(this.targetName, that.targetName) && Objects.equals(this.localAddress, that.localAddress) && Objects.equals(this.proxyChain, that.proxyChain); } @@ -283,6 +363,7 @@ public final class HttpRoute implements RouteInfo, Cloneable { public int hashCode() { int hash = LangUtils.HASH_SEED; hash = LangUtils.hashCode(hash, this.targetHost); + hash = LangUtils.hashCode(hash, this.targetName); hash = LangUtils.hashCode(hash, this.localAddress); if (this.proxyChain != null) { for (final HttpHost element : this.proxyChain) { @@ -324,7 +405,13 @@ public final class HttpRoute implements RouteInfo, Cloneable { cab.append("->"); } } + if (this.targetName != null) { + cab.append(this.targetName); + cab.append("/"); + } + cab.append("["); cab.append(this.targetHost); + cab.append("]"); return cab.toString(); } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/RouteInfo.java b/httpclient5/src/main/java/org/apache/hc/client5/http/RouteInfo.java index 5b6749011..e44499cd7 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/RouteInfo.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/RouteInfo.java @@ -30,6 +30,7 @@ package org.apache.hc.client5.http; import java.net.InetAddress; import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.net.NamedEndpoint; /** * Connection route information. @@ -71,6 +72,15 @@ public interface RouteInfo { */ HttpHost getTargetHost(); + /** + * Obtains the target name, if different from the target host, {@code null} otherwise. + * + * @since 5.4 + */ + default NamedEndpoint getTargetName() { + return null; + } + /** * Obtains the local address to connect from. * diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/routing/TestHttpRoute.java b/httpclient5/src/test/java/org/apache/hc/client5/http/TestHttpRoute.java similarity index 89% rename from httpclient5/src/test/java/org/apache/hc/client5/http/routing/TestHttpRoute.java rename to httpclient5/src/test/java/org/apache/hc/client5/http/TestHttpRoute.java index 7bccdcdee..87f06f24d 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/routing/TestHttpRoute.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/TestHttpRoute.java @@ -25,16 +25,16 @@ * */ -package org.apache.hc.client5.http.routing; +package org.apache.hc.client5.http; import java.net.InetAddress; import java.util.HashSet; import java.util.Set; -import org.apache.hc.client5.http.HttpRoute; import org.apache.hc.client5.http.RouteInfo.LayerType; import org.apache.hc.client5.http.RouteInfo.TunnelType; import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.net.URIAuthority; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -171,45 +171,8 @@ public class TestHttpRoute { Assertions.assertTrue (routettt.isSecure(), "routettt.secure"); Assertions.assertTrue (routettt.isTunnelled(), "routettt.tunnel"); Assertions.assertTrue (routettt.isLayered(), "routettt.layer"); - - - final Set routes = new HashSet<>(); - routes.add(routefff); - routes.add(routefft); - routes.add(routeftf); - routes.add(routeftt); - routes.add(routetff); - routes.add(routetft); - routes.add(routettf); - routes.add(routettt); - Assertions.assertEquals(8, routes.size(), "some flagged routes are equal"); - - // we can't test hashCode in general due to its dependency - // on InetAddress and HttpHost, but we can check for the flags - final Set routecodes = new HashSet<>(); - routecodes.add(routefff.hashCode()); - routecodes.add(routefft.hashCode()); - routecodes.add(routeftf.hashCode()); - routecodes.add(routeftt.hashCode()); - routecodes.add(routetff.hashCode()); - routecodes.add(routetft.hashCode()); - routecodes.add(routettf.hashCode()); - routecodes.add(routettt.hashCode()); - Assertions.assertEquals(8, routecodes.size(), "some flagged routes have same hashCode"); - - final Set routestrings = new HashSet<>(); - routestrings.add(routefff.toString()); - routestrings.add(routefft.toString()); - routestrings.add(routeftf.toString()); - routestrings.add(routeftt.toString()); - routestrings.add(routetff.toString()); - routestrings.add(routetft.toString()); - routestrings.add(routettf.toString()); - routestrings.add(routettt.toString()); - Assertions.assertEquals(8, routestrings.size(), "some flagged route.toString() are equal"); } - @SuppressWarnings("unused") @Test public void testInvalidArguments() { final HttpHost[] chain1 = { PROXY1 }; @@ -291,6 +254,9 @@ public class TestHttpRoute { TunnelType.TUNNELLED, LayerType.PLAIN); final HttpRoute route2k = new HttpRoute(TARGET1, LOCAL41, chain3, false, TunnelType.PLAIN, LayerType.LAYERED); + final HttpRoute route2m = new HttpRoute(TARGET2, + new URIAuthority(TARGET2.getHostName(), TARGET2.getPort()), LOCAL41, chain3, false, + TunnelType.PLAIN, LayerType.PLAIN); // check a special case first: 2f should be the same as 2e Assertions.assertEquals(route2e, route2f, "2e 2f"); @@ -308,6 +274,7 @@ public class TestHttpRoute { Assertions.assertNotEquals(route1a, route2i, "1a 2i"); Assertions.assertNotEquals(route1a, route2j, "1a 2j"); Assertions.assertNotEquals(route1a, route2k, "1a 2k"); + Assertions.assertNotEquals(route1a, route2m, "1a 2k"); // repeat the checks in the other direction // there could be problems with detecting null attributes @@ -323,6 +290,7 @@ public class TestHttpRoute { Assertions.assertNotEquals(route2i, route1a, "2i 1a"); Assertions.assertNotEquals(route2j, route1a, "2j 1a"); Assertions.assertNotEquals(route2k, route1a, "2k 1a"); + Assertions.assertNotEquals(route2m, route1a, "2k 1a"); // don't check hashCode, it's not guaranteed to be different @@ -337,6 +305,7 @@ public class TestHttpRoute { Assertions.assertNotEquals(route1a.toString(), route2i.toString(), "toString 1a 2i"); Assertions.assertNotEquals(route1a.toString(), route2j.toString(), "toString 1a 2j"); Assertions.assertNotEquals(route1a.toString(), route2k.toString(), "toString 1a 2k"); + Assertions.assertNotEquals(route1a.toString(), route2m.toString(), "toString 1a 2k"); // now check that all of the routes are different from eachother // except for those that aren't :-) @@ -353,7 +322,7 @@ public class TestHttpRoute { routes.add(route2i); routes.add(route2j); routes.add(route2k); - Assertions.assertEquals(11, routes.size(), "some routes are equal"); + routes.add(route2m); // and a run of cloning over the set for (final HttpRoute origin : routes) { @@ -362,22 +331,6 @@ public class TestHttpRoute { Assertions.assertTrue(routes.contains(cloned), "clone of " + origin); } - // and don't forget toString - final Set routestrings = new HashSet<>(); - routestrings.add(route1a.toString()); - routestrings.add(route2a.toString()); - routestrings.add(route2b.toString()); - routestrings.add(route2c.toString()); - routestrings.add(route2d.toString()); - routestrings.add(route2e.toString()); - //routestrings.add(route2f.toString()); // 2f is the same as 2e - routestrings.add(route2g.toString()); - routestrings.add(route2h.toString()); - routestrings.add(route2i.toString()); - routestrings.add(route2j.toString()); - routestrings.add(route2k.toString()); - Assertions.assertEquals(11, routestrings.size(), "some route.toString() are equal"); - // finally, compare with nonsense Assertions.assertNotEquals(null, route1a, "route equals null"); Assertions.assertNotEquals("route1a", route1a, "route equals string");