Skip to content

Commit

Permalink
Refactor UriEndpoint (#2700)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@yuzawa-san
yuzawa-san committed Mar 19, 2023
1 parent 940d203 commit e241f5f
Show file tree
Hide file tree
Showing 9 changed files with 447 additions and 375 deletions.
4 changes: 2 additions & 2 deletions ...-netty-http/src/main/java/reactor/netty/http/client/AbstractHttpClientMetricsHandler.java
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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();
}

Expand Down
15 changes: 6 additions & 9 deletions reactor-netty-http/src/main/java/reactor/netty/http/client/FailedHttpClientRequest.java
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;

Expand All @@ -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;
}

Expand Down Expand Up @@ -89,7 +86,7 @@ public ContextView currentContextView() {

@Override
public String fullPath() {
return path;
return uriEndpoint.getPath();
}

@Override
Expand Down Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClient.java
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;
}
84 changes: 8 additions & 76 deletions reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConnect.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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);
}
Expand All @@ -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)) {
Expand Down Expand Up @@ -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"})
Expand Down
10 changes: 4 additions & 6 deletions reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientOperations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit e241f5f

Please sign in to comment.