Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-1428.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: Implement configuration-based SOCKS proxy support
links:
- https://github.com/palantir/dialogue/pull/1428
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.net.MalformedURLException;
import java.net.Proxy;
import java.net.Socket;
import java.net.URI;
import java.net.URL;
import java.time.Duration;
import java.util.ArrayList;
Expand Down Expand Up @@ -477,7 +478,7 @@ public CloseableClient build() {

Timeout handshakeTimeout = getHandshakeTimeout(connectTimeout, socketTimeout, name);

InetSocketAddress socksProxyAddress = getSocksProxyAddress();
InetSocketAddress socksProxyAddress = getSocksProxyAddress(conf);
SSLSocketFactory rawSocketFactory = conf.sslSocketFactory();
SSLConnectionSocketFactory sslSocketFactory =
new SSLConnectionSocketFactory(
Expand Down Expand Up @@ -592,6 +593,34 @@ private static InetSocketAddress getSocksProxyAddress() {
return InetSocketAddress.createUnresolved(hostAndPort.getHost(), hostAndPort.getPort());
}

@Nullable
private static InetSocketAddress getSocksProxyAddress(ClientConfiguration clientConfiguration) {
// This is a roundabout way to extract proxy information. SOCKS proxies are configured at
// a different stage from HTTP proxies, so we must search for any socks proxy that the
// ProxySelector can provide. 'selector.select(null)' could work to validate no real work
// is being done, but I think the 127.0.0.1 lookup should be safer.
// Ideally we could pass along the original ServiceConfigurationBlock (if it exists) which
// provides a more precise description of this data.
try {
InetSocketAddress address = clientConfiguration.proxy().select(URI.create("127.0.0.1")).stream()
.filter(proxy -> proxy.type() == Proxy.Type.SOCKS)
.map(Proxy::address)
.filter(InetSocketAddress.class::isInstance)
.map(InetSocketAddress.class::cast)
.findFirst()
.orElseGet(ApacheHttpClientChannels::getSocksProxyAddress);
if (address != null) {
log.debug("Found SOCKS proxy address", UnsafeArg.of("address", address));
} else {
log.debug("No SOCKS proxy found");
}
return address;
} catch (RuntimeException e) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a little scary (as in might hide errors). Is this just to avoid breaking existing stuff in prod in case there is a problem with the socks detection?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, this should log at ERROR rather than DEBUG 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct, I added the catch to avoid adding risk to the production case.

log.debug("Failed to find a SOCKS proxy", e);
return null;
}
}

private static Timeout getSocketTimeout(ClientConfiguration conf, String clientName) {
long socketTimeoutMillis = conf.readTimeout().toMillis();
if (conf.readTimeout().toMillis() != conf.writeTimeout().toMillis()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@
import static org.assertj.core.api.Assertions.assertThat;

import com.google.common.util.concurrent.ListenableFuture;
import com.palantir.conjure.java.api.config.service.ProxyConfiguration;
import com.palantir.conjure.java.api.config.service.ServiceConfiguration;
import com.palantir.conjure.java.client.config.ClientConfiguration;
import com.palantir.conjure.java.client.config.ClientConfigurations;
import com.palantir.conjure.java.config.ssl.SslSocketFactories;
import com.palantir.dialogue.Channel;
import com.palantir.dialogue.Request;
Expand All @@ -39,16 +42,16 @@
*/
final class SocksProxyManualTest {

// @org.junit.jupiter.api.Test
void testTls() throws Exception {
// ssh -D 8081 -v -N localhost
System.setProperty("dialogue.experimental.socks5.proxy", "127.0.0.1:8081");
SSLContext context = SslSocketFactories.createSslContext(TestConfigurations.SSL_CONFIG);
Undertow undertow = Undertow.builder()
.addHttpsListener(8080, null, context, new ResponseCodeHandler(204))
.build();
undertow.start();
try {
ClientConfiguration configuration = TestConfigurations.create("https://localhost:" + 8080);
ClientConfiguration configuration = withSocks("https://localhost:" + 8080, "127.0.0.1:8081");
Channel channel = ApacheHttpClientChannels.create(configuration, "test");
ListenableFuture<Response> future =
channel.execute(TestEndpoint.GET, Request.builder().build());
Expand All @@ -60,15 +63,15 @@ void testTls() throws Exception {
}
}

// @org.junit.jupiter.api.Test
void testPlain() throws Exception {
// ssh -D 8081 -v -N localhost
System.setProperty("dialogue.experimental.socks5.proxy", "127.0.0.1:8081");
Undertow undertow = Undertow.builder()
.addHttpListener(8080, null, new ResponseCodeHandler(204))
.build();
undertow.start();
try {
ClientConfiguration configuration = TestConfigurations.create("http://localhost:" + 8080);
ClientConfiguration configuration = withSocks("http://localhost:" + 8080, "127.0.0.1:8081");
Channel channel = ApacheHttpClientChannels.create(configuration, "test");
ListenableFuture<Response> future =
channel.execute(TestEndpoint.GET, Request.builder().build());
Expand All @@ -79,4 +82,16 @@ void testPlain() throws Exception {
undertow.stop();
}
}

private static ClientConfiguration withSocks(String target, String socksAddress) {
return ClientConfiguration.builder()
.from(ClientConfigurations.of(ServiceConfiguration.builder()
.addUris(target)
.security(TestConfigurations.SSL_CONFIG)
.maxNumRetries(0)
.proxy(ProxyConfiguration.socks(socksAddress))
.build()))
.userAgent(TestConfigurations.AGENT)
.build();
}
}