Skip to content

Commit

Permalink
Initialise lazy the ProxyProvider configuration
Browse files Browse the repository at this point in the history
Configurations like address, headers, password are calculated just before sending the request.
If a configuration is changed a new connection pool will be created.

Fixes #3501
  • Loading branch information
violetagg committed Nov 26, 2024
1 parent d088539 commit 2b46265
Show file tree
Hide file tree
Showing 8 changed files with 217 additions and 53 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020-2023 VMware, Inc. or its affiliates, All Rights Reserved.
* Copyright (c) 2020-2024 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 @@ -52,7 +52,16 @@ public abstract class ClientTransport<T extends ClientTransport<T, CONF>,
* @return a {@link Mono} of {@link Connection}
*/
protected Mono<? extends Connection> connect() {
CONF config = configuration();
CONF originalConfiguration = configuration();
CONF config;
if (originalConfiguration.proxyProvider() == null && originalConfiguration.proxyProviderSupplier() != null) {
T dup = duplicate();
config = dup.configuration();
config.proxyProvider(config.proxyProviderSupplier().get());
}
else {
config = originalConfiguration;
}

ConnectionObserver observer = config.defaultConnectionObserver().then(config.observer);

Expand Down Expand Up @@ -212,6 +221,7 @@ public T noProxy() {
if (configuration().hasProxy()) {
T dup = duplicate();
dup.configuration().proxyProvider = null;
dup.configuration().proxyProviderSupplier = null;
if (dup.configuration().resolver == NoopAddressResolverGroup.INSTANCE) {
dup.configuration().resolver = null;
}
Expand Down Expand Up @@ -242,13 +252,25 @@ public T proxy(Consumer<? super ProxyProvider.TypeSpec> proxyOptions) {
Objects.requireNonNull(proxyOptions, "proxyOptions");
ProxyProvider.Build builder = (ProxyProvider.Build) ProxyProvider.builder();
proxyOptions.accept(builder);
return proxyWithProxyProvider(builder.build());
return proxyWithProxyProviderSupplier(() -> builder.build());
}

final T proxyWithProxyProvider(ProxyProvider proxy) {
T dup = duplicate();
CONF conf = dup.configuration();
conf.proxyProvider = proxy;
conf.proxyProviderSupplier = null;
if (conf.resolver == null) {
conf.resolver = NoopAddressResolverGroup.INSTANCE;
}
return dup;
}

final T proxyWithProxyProviderSupplier(Supplier<ProxyProvider> proxy) {
T dup = duplicate();
CONF conf = dup.configuration();
conf.proxyProvider = null;
conf.proxyProviderSupplier = proxy;
if (conf.resolver == null) {
conf.resolver = NoopAddressResolverGroup.INSTANCE;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020-2023 VMware, Inc. or its affiliates, All Rights Reserved.
* Copyright (c) 2020-2024 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 @@ -104,7 +104,7 @@ public final Consumer<? super Connection> doOnDisconnected() {
* @return true if that {@link ClientTransportConfig} is configured with a proxy
*/
public final boolean hasProxy() {
return proxyProvider != null;
return proxyProvider != null || proxyProviderSupplier != null;
}

/**
Expand All @@ -127,6 +127,16 @@ public final ProxyProvider proxyProvider() {
return proxyProvider;
}

/**
* Return the {@link ProxyProvider} supplier if any or null.
*
* @return the {@link ProxyProvider} supplier if any or null
*/
@Nullable
public final Supplier<ProxyProvider> proxyProviderSupplier() {
return proxyProviderSupplier;
}

/**
* Return the remote configured {@link SocketAddress}.
*
Expand Down Expand Up @@ -155,11 +165,12 @@ public final AddressResolverGroup<?> resolver() {
Consumer<? super CONF> doOnConnect;
Consumer<? super Connection> doOnConnected;
Consumer<? super Connection> doOnDisconnected;
Consumer<? super Connection> doOnResolve;
Consumer<? super Connection> doOnResolve;
BiConsumer<? super Connection, ? super SocketAddress> doAfterResolve;
BiConsumer<? super Connection, ? super Throwable> doOnResolveError;
NameResolverProvider nameResolverProvider;
ProxyProvider proxyProvider;
Supplier<ProxyProvider> proxyProviderSupplier;
Supplier<? extends SocketAddress> remoteAddress;
AddressResolverGroup<?> resolver;

Expand All @@ -181,6 +192,7 @@ protected ClientTransportConfig(ClientTransportConfig<CONF> parent) {
this.doOnResolveError = parent.doOnResolveError;
this.nameResolverProvider = parent.nameResolverProvider;
this.proxyProvider = parent.proxyProvider;
this.proxyProviderSupplier = parent.proxyProviderSupplier;
this.remoteAddress = parent.remoteAddress;
this.resolver = parent.resolver;
}
Expand Down Expand Up @@ -224,6 +236,10 @@ protected void proxyProvider(@Nullable ProxyProvider proxyProvider) {
this.proxyProvider = proxyProvider;
}

protected void proxyProviderSupplier(@Nullable Supplier<ProxyProvider> proxyProviderSupplier) {
this.proxyProviderSupplier = proxyProviderSupplier;
}

protected AddressResolverGroup<?> resolverInternal() {
AddressResolverGroup<?> resolverGroup = resolver != null ? resolver : defaultAddressResolverGroup();
if (metricsRecorder != null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017-2023 VMware, Inc. or its affiliates, All Rights Reserved.
* Copyright (c) 2017-2024 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 @@ -62,29 +62,29 @@ public static ProxyProvider.TypeSpec builder() {
}

final String username;
final Function<? super String, ? extends String> password;
final Supplier<? extends InetSocketAddress> address;
final String password;
final InetSocketAddress address;
final Predicate<SocketAddress> nonProxyHostPredicate;
final Supplier<? extends HttpHeaders> httpHeaders;
final HttpHeaders httpHeaders;
final Proxy type;
final long connectTimeoutMillis;

ProxyProvider(ProxyProvider.Build builder) {
this.username = builder.username;
this.password = builder.password;
this.password = builder.password != null ? builder.password : getPasswordValue(builder.passwordFunction);
this.nonProxyHostPredicate = builder.nonProxyHostPredicate;
if (Objects.isNull(builder.address)) {
if (builder.host != null) {
this.address = () -> AddressUtils.createResolved(builder.host, builder.port);
this.address = AddressUtils.createResolved(builder.host, builder.port);
}
else {
throw new IllegalArgumentException("Neither address nor host is specified");
}
}
else {
this.address = builder.address;
this.address = builder.address.get();
}
this.httpHeaders = builder.httpHeaders;
this.httpHeaders = builder.httpHeaders.get();
this.type = builder.type;
this.connectTimeoutMillis = builder.connectTimeoutMillis;
}
Expand All @@ -104,6 +104,15 @@ public final Proxy getType() {
* @return The supplier for the address to connect to.
*/
public final Supplier<? extends InetSocketAddress> getAddress() {
return () -> this.address;
}

/**
* The address to connect to.
*
* @return The address to connect to.
*/
public final SocketAddress getSocketAddress() {
return this.address;
}

Expand All @@ -125,28 +134,23 @@ public final Predicate<SocketAddress> getNonProxyHostsPredicate() {
* @return a new eventual {@link ProxyHandler}
*/
public final ProxyHandler newProxyHandler() {
InetSocketAddress proxyAddr = this.address.get();

final boolean b = Objects.nonNull(username) && Objects.nonNull(password);

String username = this.username;
String password = b ? this.password.apply(username) : null;

final ProxyHandler proxyHandler;
switch (this.type) {
case HTTP:
proxyHandler = b ?
new HttpProxyHandler(proxyAddr, username, password, this.httpHeaders.get()) :
new HttpProxyHandler(proxyAddr, this.httpHeaders.get());
new HttpProxyHandler(address, username, password, this.httpHeaders) :
new HttpProxyHandler(address, this.httpHeaders);
break;
case SOCKS4:
proxyHandler = Objects.nonNull(username) ? new Socks4ProxyHandler(proxyAddr, username) :
new Socks4ProxyHandler(proxyAddr);
proxyHandler = Objects.nonNull(username) ? new Socks4ProxyHandler(address, username) :
new Socks4ProxyHandler(address);
break;
case SOCKS5:
proxyHandler = b ?
new Socks5ProxyHandler(proxyAddr, username, password) :
new Socks5ProxyHandler(proxyAddr);
new Socks5ProxyHandler(address, username, password) :
new Socks5ProxyHandler(address);
break;
default:
throw new IllegalArgumentException("Proxy type unsupported : " + this.type);
Expand Down Expand Up @@ -195,7 +199,7 @@ public void addProxyHandler(Channel channel) {
@Override
public String toString() {
return "ProxyProvider {" +
"address=" + address.get() +
"address=" + address +
", nonProxyHosts=" + nonProxyHostPredicate +
", type=" + type +
'}';
Expand All @@ -211,37 +215,37 @@ public boolean equals(Object o) {
}
ProxyProvider that = (ProxyProvider) o;
return Objects.equals(username, that.username) &&
Objects.equals(getPasswordValue(), that.getPasswordValue()) &&
Objects.equals(getAddress().get(), that.getAddress().get()) &&
Objects.equals(password, that.password) &&
Objects.equals(address, that.address) &&
getNonProxyHostsValue() == that.getNonProxyHostsValue() &&
Objects.equals(httpHeaders.get(), that.httpHeaders.get()) &&
getType() == that.getType() &&
Objects.equals(httpHeaders, that.httpHeaders) &&
type == that.type &&
connectTimeoutMillis == that.connectTimeoutMillis;
}

@Override
public int hashCode() {
int result = 1;
result = 31 * result + Objects.hashCode(username);
result = 31 * result + Objects.hashCode(getPasswordValue());
result = 31 * result + Objects.hashCode(getAddress().get());
result = 31 * result + Objects.hashCode(password);
result = 31 * result + Objects.hashCode(address);
result = 31 * result + Boolean.hashCode(getNonProxyHostsValue());
result = 31 * result + Objects.hashCode(httpHeaders.get());
result = 31 * result + Objects.hashCode(getType());
result = 31 * result + Objects.hashCode(httpHeaders);
result = 31 * result + Objects.hashCode(type);
result = 31 * result + Long.hashCode(connectTimeoutMillis);
return result;
}

private boolean getNonProxyHostsValue() {
return nonProxyHostPredicate.test(getAddress().get());
return nonProxyHostPredicate.test(address);
}

@Nullable
private String getPasswordValue() {
if (username == null || password == null) {
private String getPasswordValue(@Nullable Function<? super String, ? extends String> passwordFunction) {
if (username == null || passwordFunction == null) {
return null;
}
return password.apply(username);
return passwordFunction.apply(username);
}

static final LoggingHandler LOGGING_HANDLER =
Expand Down Expand Up @@ -311,17 +315,17 @@ static ProxyProvider createHttpProxyFrom(Properties properties) {
String nonProxyHosts = properties.getProperty(HTTP_NON_PROXY_HOSTS, DEFAULT_NON_PROXY_HOSTS);
RegexShouldProxyPredicate transformedNonProxyHosts = RegexShouldProxyPredicate.fromWildcardedPattern(nonProxyHosts);

ProxyProvider.Builder proxy = ProxyProvider.builder()
.type(ProxyProvider.Proxy.HTTP)
ProxyProvider.Build proxy = new ProxyProvider.Build();
proxy.type(ProxyProvider.Proxy.HTTP)
.host(hostname)
.port(port)
.nonProxyHostsPredicate(transformedNonProxyHosts);

if (properties.containsKey(userProperty)) {
proxy = proxy.username(properties.getProperty(userProperty));
proxy.username(properties.getProperty(userProperty));

if (properties.containsKey(passwordProperty)) {
proxy = proxy.password(u -> properties.getProperty(passwordProperty));
proxy.password(properties.getProperty(passwordProperty));
}
else {
throw new NullPointerException("Proxy username is set via '" + userProperty + "', but '" + passwordProperty + "' is not set.");
Expand All @@ -342,16 +346,16 @@ static ProxyProvider createSocksProxyFrom(Properties properties) {
ProxyProvider.Proxy type = SOCKS_VERSION_5.equals(version) ? Proxy.SOCKS5 : Proxy.SOCKS4;
int port = parsePort(properties.getProperty(SOCKS_PROXY_PORT, "1080"), SOCKS_PROXY_PORT);

ProxyProvider.Builder proxy = ProxyProvider.builder()
.type(type)
ProxyProvider.Build proxy = new ProxyProvider.Build();
proxy.type(type)
.host(hostname)
.port(port);

if (properties.containsKey(SOCKS_USERNAME)) {
proxy = proxy.username(properties.getProperty(SOCKS_USERNAME));
proxy.username(properties.getProperty(SOCKS_USERNAME));
}
if (properties.containsKey(SOCKS_PASSWORD)) {
proxy = proxy.password(u -> properties.getProperty(SOCKS_PASSWORD));
proxy.password(properties.getProperty(SOCKS_PASSWORD));
}

return proxy.build();
Expand Down Expand Up @@ -382,7 +386,8 @@ static final class Build implements TypeSpec, AddressSpec, Builder {
static final Predicate<SocketAddress> ALWAYS_PROXY = a -> false;

String username;
Function<? super String, ? extends String> password;
String password;
Function<? super String, ? extends String> passwordFunction;
String host;
int port;
Supplier<? extends InetSocketAddress> address;
Expand All @@ -401,8 +406,15 @@ public final Builder username(String username) {
}

@Override
public final Builder password(Function<? super String, ? extends String> password) {
public final Builder password(Function<? super String, ? extends String> passwordFunction) {
this.password = null;
this.passwordFunction = passwordFunction;
return this;
}

final Builder password(String password) {
this.password = password;
this.passwordFunction = null;
return this;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020-2023 VMware, Inc. or its affiliates, All Rights Reserved.
* Copyright (c) 2020-2024 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 @@ -213,22 +213,26 @@ void testCreateClientWithSystemProxyProvider() {
void proxyOverriddenWithNullIfSystemPropertiesHaveNoProxySet() {
TestClientTransport transport = createTestTransportForProxy();
transport.proxy(spec -> spec.type(ProxyProvider.Proxy.HTTP).host("proxy").port(8080));
assertThat(transport.configuration().proxyProvider).isNotNull();
assertThat(transport.configuration().proxyProvider).isNull();
assertThat(transport.configuration().proxyProviderSupplier).isNotNull();
assertThat(transport.configuration().resolver()).isSameAs(NoopAddressResolverGroup.INSTANCE);

transport.proxyWithSystemProperties(new Properties());
assertThat(transport.configuration().proxyProvider).isNull();
assertThat(transport.configuration().proxyProviderSupplier).isNull();
assertThat(transport.configuration().resolver()).isNull();
}

@Test
void noProxyIfSystemPropertiesHaveNoProxySet() {
TestClientTransport transport = createTestTransportForProxy();
assertThat(transport.configuration().proxyProvider).isNull();
assertThat(transport.configuration().proxyProviderSupplier).isNull();
assertThat(transport.configuration().resolver()).isNull();

transport.proxyWithSystemProperties(new Properties());
assertThat(transport.configuration().proxyProvider).isNull();
assertThat(transport.configuration().proxyProviderSupplier).isNull();
assertThat(transport.configuration().resolver()).isNull();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ protected CoreSubscriber<PooledRef<Connection>> createDisposableAcquire(
boolean acceptGzip = false;
ChannelMetricsRecorder metricsRecorder = config.metricsRecorder() != null ? config.metricsRecorder().get() : null;
SocketAddress proxyAddress = ((ClientTransportConfig<?>) config).proxyProvider() != null ?
((ClientTransportConfig<?>) config).proxyProvider().getAddress().get() : null;
((ClientTransportConfig<?>) config).proxyProvider().getSocketAddress() : null;
Function<String, String> uriTagValue = null;
if (config instanceof HttpClientConfig) {
acceptGzip = ((HttpClientConfig) config).acceptGzip;
Expand Down
Loading

0 comments on commit 2b46265

Please sign in to comment.