Refactor UriEndpoint (#2700)

Clean up UriEndpoint logic to be based around the java.net.URI. Move relevant methods into this class. Construct instances using static method. Only calculate derived values on demand (e.g. toExternalForm) when needed in the various request implementations. Cleaned up redirection logic. Added additional tests. Made conversion from InetSocketAddress more efficient (removed the substrings which trimmed out the ports). Fixed HttpClientTest to support ipv6 (by using NetUtil.toSocketAddressString to get the [] around the address, and also fixed FailedHttpClientRequest.uri() from returning full URI rather than just Netty definition of "uri" which is raw path and query. Fixed ClientTransportTest to support ipv6.

Fixes #829

Author: yuzawa-san

reactor-netty-core/src/test/java/reactor/netty/transport/ClientTransportTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020-2022 VMware, Inc. or its affiliates, All Rights Reserved.
+ * Copyright (c) 2020-2023 VMware, Inc. or its affiliates, All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -36,6 +36,7 @@
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.time.Duration;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -263,7 +264,15 @@ private void doTestHostsFileEntriesResolver(boolean customResolver) throws Excep
 		TestClientTransportConfig config =
 				new TestClientTransportConfig(provider, Collections.emptyMap(), () -> null);
 		HostsFileEntriesProvider hostsFileEntriesProvider = HostsFileEntriesProvider.parser().parseSilently();
-		List<InetAddress> addresses = hostsFileEntriesProvider.ipv4Entries().get("localhost");
+		List<InetAddress> addresses = new ArrayList<>();
+		List<InetAddress> ipv4Addresses = hostsFileEntriesProvider.ipv4Entries().get("localhost");
+		if (ipv4Addresses != null) {
+			addresses.addAll(ipv4Addresses);
+		}
+		List<InetAddress> ipv6Addresses = hostsFileEntriesProvider.ipv6Entries().get("localhost");
+		if (ipv6Addresses != null) {
+			addresses.addAll(ipv6Addresses);
+		}
 		try {
 			config.loopResources = loop1;
 			if (customResolver) {

reactor-netty-http/src/main/java/reactor/netty/http/client/AbstractHttpClientMetricsHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021-2022 VMware, Inc. or its affiliates, All Rights Reserved.
+ * Copyright (c) 2021-2023 VMware, Inc. or its affiliates, All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -163,7 +163,7 @@ private void extractDetailsFromHttpRequest(ChannelHandlerContext ctx, HttpReques
 		ChannelOperations<?, ?> channelOps = ChannelOperations.get(ctx.channel());
 		if (channelOps instanceof HttpClientOperations) {
 			HttpClientOperations ops = (HttpClientOperations) channelOps;
-			path = uriTagValue == null ? ops.path : uriTagValue.apply(ops.path);
+			path = uriTagValue == null ? ops.fullPath() : uriTagValue.apply(ops.fullPath());
 			contextView = ops.currentContextView();
 		}
 

reactor-netty-http/src/main/java/reactor/netty/http/client/FailedHttpClientRequest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020-2021 VMware, Inc. or its affiliates, All Rights Reserved.
+ * Copyright (c) 2020-2023 VMware, Inc. or its affiliates, All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,7 +23,6 @@
 import io.netty.handler.codec.http.cookie.ClientCookieDecoder;
 import io.netty.handler.codec.http.cookie.Cookie;
 import reactor.netty.http.Cookies;
-import reactor.netty.http.HttpOperations;
 import reactor.util.context.Context;
 import reactor.util.context.ContextView;
 
@@ -45,18 +44,16 @@ final class FailedHttpClientRequest implements HttpClientRequest {
 	final HttpHeaders         headers;
 	final boolean             isWebsocket;
 	final HttpMethod          method;
-	final String              path;
 	final Duration            responseTimeout;
-	final String              uri;
+	final UriEndpoint         uriEndpoint;
 
 	FailedHttpClientRequest(ContextView contextView, HttpClientConfig c) {
 		this.contextView = contextView;
 		this.cookieDecoder = c.cookieDecoder;
 		this.headers = c.headers;
 		this.isWebsocket = c.websocketClientSpec != null;
 		this.method = c.method;
-		this.uri = c.uri == null ? c.uriStr : c.uri.toString();
-		this.path = this.uri != null ? HttpOperations.resolvePath(this.uri) : null;
+		this.uriEndpoint = UriEndpoint.create(c.uri, c.baseUrl, c.uriStr, c.remoteAddress(), c.isSecure(), c.websocketClientSpec != null);
 		this.responseTimeout = c.responseTimeout;
 	}
 
@@ -89,7 +86,7 @@ public ContextView currentContextView() {
 
 	@Override
 	public String fullPath() {
-		return path;
+		return uriEndpoint.getPath();
 	}
 
 	@Override
@@ -144,12 +141,12 @@ public HttpClientRequest responseTimeout(Duration maxReadOperationInterval) {
 
 	@Override
 	public String resourceUrl() {
-		return null;
+		return uriEndpoint.toExternalForm();
 	}
 
 	@Override
 	public String uri() {
-		return uri;
+		return uriEndpoint.getRawUri();
 	}
 
 	@Override

reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClient.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011-2022 VMware, Inc. or its affiliates, All Rights Reserved.
+ * Copyright (c) 2011-2023 VMware, Inc. or its affiliates, All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -1608,4 +1608,8 @@ static String reactorNettyVersion() {
 	static final String WS_SCHEME = "ws";
 
 	static final String WSS_SCHEME = "wss";
+
+	static final int DEFAULT_PORT = 80;
+
+	static final int DEFAULT_SECURE_PORT = 443;
 }

reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConnect.java

@@ -17,8 +17,6 @@
 
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
-import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -38,7 +36,6 @@
 import io.netty.handler.codec.http.HttpHeaders;
 import io.netty.handler.codec.http.HttpMethod;
 import io.netty.handler.codec.http.HttpResponseStatus;
-import io.netty.handler.codec.http.HttpUtil;
 import io.netty.handler.codec.http.HttpVersion;
 import io.netty.handler.ssl.SslClosedEngineException;
 import io.netty.resolver.AddressResolverGroup;
@@ -55,7 +52,6 @@
 import reactor.netty.ConnectionObserver;
 import reactor.netty.NettyOutbound;
 import reactor.netty.channel.AbortedException;
-import reactor.netty.http.HttpOperations;
 import reactor.netty.http.HttpProtocol;
 import reactor.netty.tcp.TcpClientConfig;
 import reactor.netty.transport.AddressUtils;
@@ -455,7 +451,6 @@ static final class HttpClientHandler extends SocketAddress
 		final BiFunction<? super HttpClientRequest, ? super NettyOutbound, ? extends Publisher<Void>>
 		                              handler;
 		final boolean                 compress;
-		final UriEndpointFactory      uriEndpointFactory;
 		final WebsocketClientSpec     websocketClientSpec;
 		final BiPredicate<HttpClientRequest, HttpClientResponse>
 		                              followRedirectPredicate;
@@ -468,7 +463,6 @@ static final class HttpClientHandler extends SocketAddress
 		final Duration                responseTimeout;
 
 		volatile UriEndpoint        toURI;
-		volatile String             resourceUrl;
 		volatile UriEndpoint        fromURI;
 		volatile Supplier<String>[] redirectedFrom;
 		volatile boolean            shouldRetry;
@@ -484,34 +478,10 @@ static final class HttpClientHandler extends SocketAddress
 			this.proxyProvider = configuration.proxyProvider();
 			this.responseTimeout = configuration.responseTimeout;
 			this.defaultHeaders = configuration.headers;
-
-			String baseUrl = configuration.baseUrl;
-
-			this.uriEndpointFactory =
-					new UriEndpointFactory(configuration.remoteAddress(), configuration.isSecure(), URI_ADDRESS_MAPPER);
-
 			this.websocketClientSpec = configuration.websocketClientSpec;
 			this.shouldRetry = !configuration.retryDisabled;
 			this.handler = configuration.body;
-
-			if (configuration.uri == null) {
-				String uri = configuration.uriStr;
-
-				uri = uri == null ? "/" : uri;
-
-				if (baseUrl != null && uri.startsWith("/")) {
-					if (baseUrl.endsWith("/")) {
-						baseUrl = baseUrl.substring(0, baseUrl.length() - 1);
-					}
-					uri = baseUrl + uri;
-				}
-
-				this.toURI = uriEndpointFactory.createUriEndpoint(uri, configuration.websocketClientSpec != null);
-			}
-			else {
-				this.toURI = uriEndpointFactory.createUriEndpoint(configuration.uri, configuration.websocketClientSpec != null);
-			}
-			this.resourceUrl = toURI.toExternalForm();
+			this.toURI = UriEndpoint.create(configuration.uri, configuration.baseUrl, configuration.uriStr, configuration.remoteAddress(), configuration.isSecure(), configuration.websocketClientSpec != null);
 		}
 
 		@Override
@@ -526,18 +496,16 @@ public SocketAddress get() {
 
 		Publisher<Void> requestWithBody(HttpClientOperations ch) {
 			try {
-				ch.resourceUrl = this.resourceUrl;
+				UriEndpoint uriEndpoint = toURI;
+				ch.uriEndpoint = uriEndpoint;
 				ch.responseTimeout = responseTimeout;
 
-				UriEndpoint uri = toURI;
 				HttpHeaders headers = ch.getNettyRequest()
-				                        .setUri(uri.getPathAndQuery())
+				                        .setUri(uriEndpoint.getRawUri())
 				                        .setMethod(method)
 				                        .setProtocolVersion(HttpVersion.HTTP_1_1)
 				                        .headers();
 
-				ch.path = HttpOperations.resolvePath(ch.uri());
-
 				if (!defaultHeaders.isEmpty()) {
 					headers.set(defaultHeaders);
 				}
@@ -546,9 +514,8 @@ Publisher<Void> requestWithBody(HttpClientOperations ch) {
 					headers.set(HttpHeaderNames.USER_AGENT, USER_AGENT);
 				}
 
-				SocketAddress remoteAddress = uri.getRemoteAddress();
 				if (!headers.contains(HttpHeaderNames.HOST)) {
-					headers.set(HttpHeaderNames.HOST, resolveHostHeaderValue(remoteAddress));
+					headers.set(HttpHeaderNames.HOST, uriEndpoint.getHostHeader());
 				}
 
 				if (!headers.contains(HttpHeaderNames.ACCEPT)) {
@@ -608,46 +575,11 @@ Publisher<Void> requestWithBody(HttpClientOperations ch) {
 			}
 		}
 
-		static String resolveHostHeaderValue(@Nullable SocketAddress remoteAddress) {
-			if (remoteAddress instanceof InetSocketAddress) {
-				InetSocketAddress address = (InetSocketAddress) remoteAddress;
-				String host = HttpUtil.formatHostnameForHttp(address);
-				int port = address.getPort();
-				if (port != 80 && port != 443) {
-					host = host + ':' + port;
-				}
-				return host;
-			}
-			else {
-				return "localhost";
-			}
-		}
-
 		void redirect(String to) {
 			Supplier<String>[] redirectedFrom = this.redirectedFrom;
-			UriEndpoint toURITemp;
-			UriEndpoint from = toURI;
-			SocketAddress address = from.getRemoteAddress();
-			if (address instanceof InetSocketAddress) {
-				try {
-					URI redirectUri = new URI(to);
-					if (!redirectUri.isAbsolute()) {
-						URI requestUri = new URI(resourceUrl);
-						redirectUri = requestUri.resolve(redirectUri);
-					}
-					toURITemp = uriEndpointFactory.createUriEndpoint(redirectUri, from.isWs());
-				}
-				catch (URISyntaxException e) {
-					throw new IllegalArgumentException("Cannot resolve location header", e);
-				}
-			}
-			else {
-				toURITemp = uriEndpointFactory.createUriEndpoint(from, to, () -> address);
-			}
-			fromURI = from;
-			toURI = toURITemp;
-			resourceUrl = toURITemp.toExternalForm();
-			this.redirectedFrom = addToRedirectedFromArray(redirectedFrom, from);
+			fromURI = toURI;
+			toURI = toURI.redirect(to);
+			this.redirectedFrom = addToRedirectedFromArray(redirectedFrom, fromURI);
 		}
 
 		@SuppressWarnings({"unchecked", "rawtypes"})

reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientOperations.java

@@ -106,8 +106,7 @@ class HttpClientOperations extends HttpOperations<NettyInbound, NettyOutbound>
 	final Sinks.One<HttpHeaders> trailerHeaders;
 
 	Supplier<String>[]          redirectedFrom = EMPTY_REDIRECTIONS;
-	String                      resourceUrl;
-	String                      path;
+	UriEndpoint                 uriEndpoint;
 	Duration                    responseTimeout;
 
 	volatile ResponseState responseState;
@@ -140,8 +139,7 @@ class HttpClientOperations extends HttpOperations<NettyInbound, NettyOutbound>
 		this.requestHeaders = replaced.requestHeaders;
 		this.cookieEncoder = replaced.cookieEncoder;
 		this.cookieDecoder = replaced.cookieDecoder;
-		this.resourceUrl = replaced.resourceUrl;
-		this.path = replaced.path;
+		this.uriEndpoint = replaced.uriEndpoint;
 		this.responseTimeout = replaced.responseTimeout;
 		this.is100Continue = replaced.is100Continue;
 		this.trailerHeaders = replaced.trailerHeaders;
@@ -504,12 +502,12 @@ public final String uri() {
 
 	@Override
 	public final String fullPath() {
-		return this.path;
+		return uriEndpoint == null ? null : uriEndpoint.getPath();
 	}
 
 	@Override
 	public String resourceUrl() {
-		return resourceUrl;
+		return uriEndpoint == null ? null : uriEndpoint.toExternalForm();
 	}
 
 	@Override