Skip to content

Commit

Permalink
Introduce defaultHttp(s)Port for MultiAddressHttpClientBuilder (#2703)
Browse files Browse the repository at this point in the history
Motivation
----------
At the moment, if no port number is explicitly provided, the
default HTTP and HTTPS ports are used (80 and 443). In some
environments it can be useful (i.e. if a client is always talking
to a high port number) to not have to specify it on every
request.

Modifications
-------------
This changeset introduces two new builder options to allow customizing
the defaultHttpPort as well as the defaultHttpsPort and applying it
if no explicit port is provided on the request itself.

Result
------
It is now possible to not specify the port number on every request
if they diverge from the default 80 or 443 port numbers.
  • Loading branch information
daschl authored Sep 22, 2023
1 parent 248df1c commit 9bc9746
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,18 @@ public MultiAddressHttpClientBuilder<U, R> followRedirects(final RedirectConfig
return this;
}

@Override
public MultiAddressHttpClientBuilder<U, R> defaultHttpPort(final int port) {
delegate = delegate.defaultHttpPort(port);
return this;
}

@Override
public MultiAddressHttpClientBuilder<U, R> defaultHttpsPort(final int port) {
delegate = delegate.defaultHttpsPort(port);
return this;
}

@Override
public HttpClient build() {
return delegate.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,4 +147,28 @@ default MultiAddressHttpClientBuilder<U, R> headersFactory(HttpHeadersFactory he
* @see RedirectConfigBuilder
*/
MultiAddressHttpClientBuilder<U, R> followRedirects(@Nullable RedirectConfig config);

/**
* Configures the default port for the HTTP scheme if not explicitly provided as part of the
* {@link HttpRequestMetaData#requestTarget()}.
*
* @param port the port that should be used if not explicitly provided for HTTP requests.
* @return {@code this}.
*/
default MultiAddressHttpClientBuilder<U, R> defaultHttpPort(int port) { // FIXME: 0.43 - remove default impl
throw new UnsupportedOperationException("Setting defaultHttpPort is not yet supported by "
+ getClass().getName());
}

/**
* Configures the default port for the HTTPS scheme if not explicitly provided as part of the
* {@link HttpRequestMetaData#requestTarget()}.
*
* @param port the port that should be used if not explicitly provided for HTTPS requests.
* @return {@code this}.
*/
default MultiAddressHttpClientBuilder<U, R> defaultHttpsPort(int port) { // FIXME: 0.43 - remove default impl
throw new UnsupportedOperationException("Setting defaultHttpsPort is not yet supported by "
+ getClass().getName());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ final class DefaultMultiAddressUrlHttpClientBuilder
private HttpHeadersFactory headersFactory;
@Nullable
private RedirectConfig redirectConfig;
private int defaultHttpPort = HTTP.port();
private int defaultHttpsPort = HTTPS.port();
@Nullable
private SingleAddressInitializer<HostAndPort, InetSocketAddress> singleAddressInitializer;

Expand All @@ -118,7 +120,8 @@ public StreamingHttpClient buildStreaming() {
final HttpExecutionContext executionContext = executionContextBuilder.build();
final ClientFactory clientFactory =
new ClientFactory(builderFactory, executionContext, singleAddressInitializer);
final CachingKeyFactory keyFactory = closeables.prepend(new CachingKeyFactory());
final CachingKeyFactory keyFactory = closeables.prepend(
new CachingKeyFactory(defaultHttpPort, defaultHttpsPort));
final HttpHeadersFactory headersFactory = this.headersFactory;
FilterableStreamingHttpClient urlClient = closeables.prepend(
new StreamingUrlHttpClient(executionContext, keyFactory, clientFactory,
Expand All @@ -144,6 +147,13 @@ public StreamingHttpClient buildStreaming() {
private static final class CachingKeyFactory implements AsyncCloseable {

private final ConcurrentMap<String, UrlKey> urlKeyCache = new ConcurrentHashMap<>();
private final int defaultHttpPort;
private final int defaultHttpsPort;

CachingKeyFactory(final int defaultHttpPort, final int defaultHttpsPort) {
this.defaultHttpPort = defaultHttpPort;
this.defaultHttpsPort = defaultHttpsPort;
}

public UrlKey get(final HttpRequestMetaData metaData) throws MalformedURLException {
final String host = metaData.host();
Expand All @@ -161,7 +171,7 @@ public UrlKey get(final HttpRequestMetaData metaData) throws MalformedURLExcepti

final int parsedPort = metaData.port();
final int port = parsedPort >= 0 ? parsedPort :
(HTTPS_SCHEME.equalsIgnoreCase(scheme) ? HTTPS : HTTP).port();
(HTTPS_SCHEME.equalsIgnoreCase(scheme) ? defaultHttpsPort : defaultHttpPort);

metaData.requestTarget(absoluteToRelativeFormRequestTarget(metaData.requestTarget(), scheme, host));

Expand Down Expand Up @@ -453,4 +463,29 @@ public MultiAddressHttpClientBuilder<HostAndPort, InetSocketAddress> followRedir
this.redirectConfig = config;
return this;
}

@Override
public MultiAddressHttpClientBuilder<HostAndPort, InetSocketAddress> defaultHttpPort(final int port) {
this.defaultHttpPort = verifyPortRange(port);
return this;
}

@Override
public MultiAddressHttpClientBuilder<HostAndPort, InetSocketAddress> defaultHttpsPort(final int port) {
this.defaultHttpsPort = verifyPortRange(port);
return this;
}

/**
* Verifies that the given port number is within the allowed range.
*
* @param port the port number to verify.
* @return the port number if in range.
*/
private static int verifyPortRange(final int port) {
if (port < 1 || port > 0xFFFF) {
throw new IllegalArgumentException("Provided port number is out of range (between 1 and 65535): " + port);
}
return port;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.MalformedURLException;
import java.net.ServerSocket;
import java.net.UnknownHostException;
import java.util.Collection;
import java.util.List;
Expand Down Expand Up @@ -80,6 +82,7 @@

class MultiAddressUrlHttpClientTest {

private static final String ID = MultiAddressUrlHttpClientTest.class.getSimpleName();
private static final String INVALID_HOSTNAME = "invalid.";
private static final String X_REQUESTED_LOCATION = "X-Requested-Location";
private static final String X_RECEIVED_REQUEST_TARGET = "X-Received-Request-Target";
Expand All @@ -97,8 +100,7 @@ class MultiAddressUrlHttpClientTest {
static void beforeClass() throws Exception {
afterClassCloseables = newCompositeCloseable();

client = afterClassCloseables.append(HttpClients.forMultiAddressUrl(
MultiAddressUrlHttpClientTest.class.getSimpleName())
client = afterClassCloseables.append(HttpClients.forMultiAddressUrl(ID)
.followRedirects(new RedirectConfigBuilder().allowNonRelativeRedirects(true).build())
.initializer((scheme, address, builder) -> builder.serviceDiscoverer(sdThatSupportsInvalidHostname()))
.buildStreaming());
Expand Down Expand Up @@ -193,25 +195,25 @@ void requestWithAbsoluteFormRequestTargetWithHostHeader() throws Exception {
StreamingHttpRequest request = client.get(format("http://%s/200?param=value#tag", hostHeader));
// value in the HOST header should be ignored:
request.headers().set(HOST, format("%s:%d", INVALID_HOSTNAME, 8080));
requestAndValidate(request, OK, "/200?param=value#tag");
requestAndValidate(client, request, OK, "/200?param=value#tag");
}

@Test
void requestWithAbsoluteFormRequestTargetWithoutHostHeader() throws Exception {
StreamingHttpRequest request = client.get(format("http://%s/200?param=value#tag", hostHeader));
requestAndValidate(request, OK, "/200?param=value#tag");
requestAndValidate(client, request, OK, "/200?param=value#tag");
}

@Test
void requestWithAbsoluteFormRequestTargetWithoutPath() throws Exception {
StreamingHttpRequest request = client.get(format("http://%s", hostHeader));
requestAndValidate(request, BAD_REQUEST, "/");
requestAndValidate(client, request, BAD_REQUEST, "/");
}

@Test
void requestWithAbsoluteFormRequestTargetWithoutPathWithParams() throws Exception {
StreamingHttpRequest request = client.get(format("http://%s?param=value", hostHeader));
requestAndValidate(request, BAD_REQUEST, "/?param=value");
requestAndValidate(client, request, BAD_REQUEST, "/?param=value");
}

@Test
Expand Down Expand Up @@ -255,14 +257,33 @@ void requestWithAsteriskFormRequestTargetWithHostHeader() {
void requestWithRedirect() throws Exception {
StreamingHttpRequest request = client.get(format("http://%s/301", hostHeader))
.setHeader(X_REQUESTED_LOCATION, format("http://%s/200", hostHeader)); // Location for redirect
requestAndValidate(request, OK, "/200");
requestAndValidate(client, request, OK, "/200");
}

@Test
void requestWithRelativeRedirect() throws Exception {
StreamingHttpRequest request = client.get(format("http://%s/301", hostHeader))
.setHeader(X_REQUESTED_LOCATION, "/200"); // Location for redirect
requestAndValidate(request, OK, "/200");
requestAndValidate(client, request, OK, "/200");
}

@Test
void requestWithCustomDefaultHttpPort() throws Exception {
try (StreamingHttpClient client = HttpClients
.forMultiAddressUrl(ID).defaultHttpPort(serverPort).buildStreaming()) {
StreamingHttpRequest request = client.get(format("http://%s/200", serverHost));
requestAndValidate(client, request, OK, "/200");
}
}

@Test
void requestWithExplicitPortTakesPrecedenceOverCustomDefaultHttpPort() throws Exception {
int illegalPort = availablePort();
try (StreamingHttpClient client = HttpClients
.forMultiAddressUrl(ID).defaultHttpPort(illegalPort).buildStreaming()) {
StreamingHttpRequest request = client.get(format("http://%s:%d/200", serverHost, serverPort));
requestAndValidate(client, request, OK, "/200");
}
}

@Test
Expand All @@ -278,22 +299,22 @@ void multipleRequestsToMultipleServers() throws Exception {
BAD_REQUEST, UNAUTHORIZED, FORBIDDEN,
INTERNAL_SERVER_ERROR, NOT_IMPLEMENTED, BAD_GATEWAY);
for (HttpResponseStatus status : statuses) {
makeGetRequestAndValidate(hostHeader, status);
makeGetRequestAndValidate(hostHeader2, status);
makeGetRequestAndValidate(hostHeader3, status);
makeGetRequestAndValidate(client, hostHeader, status);
makeGetRequestAndValidate(client, hostHeader2, status);
makeGetRequestAndValidate(client, hostHeader3, status);
}
}
}

private static void makeGetRequestAndValidate(final String hostHeader, final HttpResponseStatus status)
throws Exception {
private static void makeGetRequestAndValidate(final StreamingHttpClient client, final String hostHeader,
final HttpResponseStatus status) throws Exception {
StreamingHttpRequest request = client.get(format("http://%s/%d?param=value#tag", hostHeader, status.code()));
requestAndValidate(request, status, format("/%d?param=value#tag", status.code()));
requestAndValidate(client, request, status, format("/%d?param=value#tag", status.code()));
}

private static void requestAndValidate(final StreamingHttpRequest request,
final HttpResponseStatus expectedStatus,
final String expectedRequestTarget) throws Exception {
private static void requestAndValidate(final StreamingHttpClient client, final StreamingHttpRequest request,
final HttpResponseStatus expectedStatus, final String expectedRequestTarget)
throws Exception {
StreamingHttpResponse response = awaitIndefinitelyNonNull(client.request(request));
assertThat(response.status(), is(expectedStatus));
final CharSequence receivedRequestTarget = response.headers().get(X_RECEIVED_REQUEST_TARGET);
Expand All @@ -305,4 +326,11 @@ private static ServerContext startNewLocalServer(final StreamingHttpService http
final CompositeCloseable closeables) throws Exception {
return closeables.append(HttpServers.forAddress(localAddress(0)).listenStreamingAndAwait(httpService));
}

private static int availablePort() throws IOException {
ServerSocket s = new ServerSocket(0);
int port = s.getLocalPort();
s.close();
return port;
}
}

0 comments on commit 9bc9746

Please sign in to comment.