Skip to content

http-netty: ON_NEW_CONNECTION discovery ignores proxy config #3682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
idelpivnitskiy merged 1 commit into from
Oct 8, 2026
Merged

http-netty: ON_NEW_CONNECTION discovery ignores proxy config #3682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/HttpClients.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,9 @@ public static <U, R> SingleAddressHttpClientBuilder<U, R> forSingleAddress(
mappingServiceDiscoverer(u -> (R) u,
"from " + address.getClass().getSimpleName() + " to an " +
Object.class.getSimpleName()),
__ -> address);
// Reverse the cast above instead of returning the captured address: the connection
// factory receives the proxy address when a proxy is configured, not the origin address.
r -> (U) r);
default:
throw new IllegalArgumentException("Unsupported strategy: " + discoveryStrategy);
}
Expand Down
18 changes: 17 additions & 1 deletion servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/HttpProxyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2019, 2021-2022 Apple Inc. and the ServiceTalk project authors
* Copyright © 2019-2026 Apple Inc. and the ServiceTalk project authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -21,6 +21,7 @@
import io.servicetalk.http.api.HttpResponse;
import io.servicetalk.http.api.ProxyConfigBuilder;
import io.servicetalk.http.api.SingleAddressHttpClientBuilder;
import io.servicetalk.http.netty.HttpClients.DiscoveryStrategy;
import io.servicetalk.http.netty.HttpsProxyTest.TargetAddressCheckConnectionFactoryFilter;
import io.servicetalk.transport.api.ClientSslConfigBuilder;
import io.servicetalk.transport.api.HostAndPort;
Expand All @@ -38,9 +39,11 @@
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
import java.util.stream.Stream;
import javax.annotation.Nullable;

import static io.servicetalk.concurrent.api.Single.succeeded;
import static io.servicetalk.dns.discovery.netty.DnsServiceDiscoverers.globalARecordsDnsServiceDiscoverer;
import static io.servicetalk.http.api.HttpHeaderNames.HOST;
import static io.servicetalk.http.api.HttpResponseStatus.OK;
import static io.servicetalk.http.api.HttpSerializers.textSerializerUtf8;
Expand Down Expand Up @@ -119,6 +122,19 @@ void testRequestForSingleAddress(HttpProtocol clientProtocol, HttpProtocol serve
testRequest(clientProtocol, serverProtocol, HttpClients::forSingleAddress);
}

private static Stream<Arguments> protocolsAndDiscoveryStrategies() {
return protocols().stream().flatMap(protocols -> Stream.of(DiscoveryStrategy.values())
.map(strategy -> Arguments.of(protocols.get()[0], protocols.get()[1], strategy)));
}

@ParameterizedTest(name = "[{index}] clientProtocol={0} serverProtocol={1} discoveryStrategy={2}")
@MethodSource("protocolsAndDiscoveryStrategies")
void testRequestForSingleAddressWithServiceDiscoverer(HttpProtocol clientProtocol, HttpProtocol serverProtocol,
DiscoveryStrategy discoveryStrategy) throws Exception {
testRequest(clientProtocol, serverProtocol, address ->
HttpClients.forSingleAddress(globalARecordsDnsServiceDiscoverer(), address, discoveryStrategy));
}

@ParameterizedTest(name = "[{index}] clientProtocol={0} serverProtocol={1}")
@MethodSource("protocols")
void testRequestForResolvedAddress(HttpProtocol clientProtocol, HttpProtocol serverProtocol) throws Exception {
Expand Down
44 changes: 39 additions & 5 deletions servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/HttpsProxyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2019-2023 Apple Inc. and the ServiceTalk project authors
* Copyright © 2019-2026 Apple Inc. and the ServiceTalk project authors
*
* 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 @@ -32,6 +32,8 @@
import io.servicetalk.http.api.ProxyConnectException;
import io.servicetalk.http.api.ProxyConnectResponseException;
import io.servicetalk.http.api.ReservedBlockingHttpConnection;
import io.servicetalk.http.api.SingleAddressHttpClientBuilder;
import io.servicetalk.http.netty.HttpClients.DiscoveryStrategy;
import io.servicetalk.test.resources.DefaultTestCerts;
import io.servicetalk.transport.api.ClientSslConfigBuilder;
import io.servicetalk.transport.api.ConnectionObserver;
Expand Down Expand Up @@ -66,6 +68,7 @@
import java.util.List;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import javax.net.ssl.KeyManagerFactory;
Expand All @@ -76,6 +79,7 @@

import static io.servicetalk.concurrent.api.Single.succeeded;
import static io.servicetalk.concurrent.internal.DeliberateException.DELIBERATE_EXCEPTION;
import static io.servicetalk.dns.discovery.netty.DnsServiceDiscoverers.globalARecordsDnsServiceDiscoverer;
import static io.servicetalk.http.api.HttpContextKeys.HTTP_TARGET_ADDRESS_BEHIND_PROXY;
import static io.servicetalk.http.api.HttpHeaderNames.CONNECTION;
import static io.servicetalk.http.api.HttpHeaderNames.CONTENT_LENGTH;
Expand Down Expand Up @@ -164,13 +168,22 @@ void setUp(List<HttpProtocol> protocols, boolean failHandshake,

void setUp(List<HttpProtocol> protocols, boolean failHandshake, boolean failProxyHandshake,
Consumer<HttpHeaders> connectRequestHeadersInitializer, boolean proxyTls) throws Exception {
setUp(protocols, failHandshake, failProxyHandshake, connectRequestHeadersInitializer, proxyTls,
address -> BuilderUtils.newClientBuilder(address, CLIENT_CTX));
}

void setUp(List<HttpProtocol> protocols, boolean failHandshake, boolean failProxyHandshake,
Consumer<HttpHeaders> connectRequestHeadersInitializer, boolean proxyTls,
Function<HostAndPort, SingleAddressHttpClientBuilder<HostAndPort, InetSocketAddress>>
clientBuilderFactory) throws Exception {
initMocks();
if (proxyTls) {
proxyTunnel.sslContext(buildProxySslContext());
}
proxyAddress = proxyTunnel.startProxy();
startServer(protocols);
createClient(protocols, failHandshake, failProxyHandshake, connectRequestHeadersInitializer, proxyTls);
createClient(protocols, failHandshake, failProxyHandshake, connectRequestHeadersInitializer, proxyTls,
clientBuilderFactory);
}

/**
Expand Down Expand Up @@ -254,8 +267,10 @@ private void startServer(List<HttpProtocol> protocols) throws Exception {
}

private void createClient(List<HttpProtocol> protocols, boolean failHandshake, boolean failProxyHandshake,
Consumer<HttpHeaders> connectRequestHeadersInitializer, boolean proxyTls) {
assert serverContext != null && proxyAddress != null;
Consumer<HttpHeaders> connectRequestHeadersInitializer, boolean proxyTls,
Function<HostAndPort, SingleAddressHttpClientBuilder<HostAndPort, InetSocketAddress>>
clientBuilderFactory) {
assert serverAddress != null && proxyAddress != null;
final ProxyConfigBuilder<HostAndPort> proxyBuilder = new ProxyConfigBuilder<>(proxyAddress)
.connectRequestHeadersInitializer(connectRequestHeadersInitializer);
if (proxyTls) {
Expand All @@ -267,7 +282,7 @@ private void createClient(List<HttpProtocol> protocols, boolean failHandshake, b
new ClientSslConfigBuilder(DefaultTestCerts::loadServerCAPem);
proxyBuilder.sslConfig(proxySslConfig.build());
}
client = BuilderUtils.newClientBuilder(serverContext, CLIENT_CTX)
client = clientBuilderFactory.apply(serverAddress)
.proxyConfig(proxyBuilder.build())
.sslConfig(new ClientSslConfigBuilder(DefaultTestCerts::loadServerCAPem)
.peerHost(failHandshake ? "unknown" : serverPemHostname()).build())
Expand All @@ -285,6 +300,25 @@ void testClientRequest(List<HttpProtocol> protocols, boolean proxyTls) throws Ex
assertResponse(client.request(client.get("/path")), protocols.get(0).version, proxyTls);
}

static Stream<Arguments> discoveryStrategiesAndProxyTls() {
return Stream.of(DiscoveryStrategy.values())
.flatMap(strategy -> Stream.of(Arguments.of(strategy, false), Arguments.of(strategy, true)));
}

@ParameterizedTest(name = "{displayName} [{index}] discoveryStrategy={0} proxyTls={1}")
@MethodSource("discoveryStrategiesAndProxyTls")
void testClientRequestWithServiceDiscoverer(DiscoveryStrategy discoveryStrategy, boolean proxyTls)
throws Exception {
setUp(singletonList(HttpProtocol.HTTP_1), false, false, __ -> { /* noop */ }, proxyTls,
address -> HttpClients.forSingleAddress(globalARecordsDnsServiceDiscoverer(), address,
discoveryStrategy)
.ioExecutor(CLIENT_CTX.ioExecutor())
.executor(CLIENT_CTX.executor())
.bufferAllocator(CLIENT_CTX.bufferAllocator()));
assertThat(client, is(notNullValue()));
assertResponse(client.request(client.get("/path")), HttpProtocol.HTTP_1.version, proxyTls);
}

@ParameterizedTest(name = "{displayName} [{index}] protocols={0} proxyTls={1}")
@MethodSource("protocolsAndProxyTls")
void testConnectionRequest(List<HttpProtocol> protocols, boolean proxyTls) throws Exception {
Expand Down
Loading