Skip to content

Commit

Permalink
SNOW-1898533: Do not set proxy in global request config
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-dprzybysz committed Feb 3, 2025
1 parent 4d73661 commit 6c6d174
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 44 deletions.
66 changes: 22 additions & 44 deletions src/main/java/net/snowflake/client/core/HttpUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -299,48 +299,20 @@ public static CloseableHttpClient buildHttpClient(
socketTimeout,
timeToLive);

// Set proxy settings for DefaultRequestConfig. If current proxy settings are the same as for
// the last request, keep the current DefaultRequestConfig. If not, build a new
// DefaultRequestConfig and set the new proxy settings for it
HttpHost proxy =
(key != null && key.usesProxy())
? new HttpHost(
key.getProxyHost(), key.getProxyPort(), key.getProxyHttpProtocol().getScheme())
: null;
// If defaultrequestconfig is not initialized or its proxy settings do not match current proxy
// settings, re-build it (current or old proxy settings could be null, so null check is
// included)
boolean noDefaultRequestConfig =
DefaultRequestConfig == null || DefaultRequestConfig.getProxy() == null;
if (noDefaultRequestConfig || !DefaultRequestConfig.getProxy().equals(proxy)) {
// Create default request config without proxy since different connections could use different
// proxies in multi tenant environments
// Proxy is set later with route planner
if (DefaultRequestConfig == null) {
RequestConfig.Builder builder =
RequestConfig.custom()
.setConnectTimeout((int) connectTimeout)
.setConnectionRequestTimeout((int) connectTimeout)
.setSocketTimeout((int) socketTimeout);
// only set the proxy settings if they are not null
// but no value has been specified for nonProxyHosts
// the route planner will determine whether to use a proxy based on nonProxyHosts value.
String logMessage =
"Rebuilding request config. Connect timeout: "
+ connectTimeout
+ " ms, connection request "
+ "timeout: "
+ connectTimeout
+ " ms, socket timeout: "
+ socketTimeout
+ " ms";
if (proxy != null && Strings.isNullOrEmpty(key.getNonProxyHosts())) {
builder.setProxy(proxy);
logMessage +=
", host: "
+ key.getProxyHost()
+ ", port: "
+ key.getProxyPort()
+ ", scheme: "
+ key.getProxyHttpProtocol().getScheme();
}
logger.debug(logMessage);
logger.debug(
"Rebuilding request config. Connect timeout: {} ms, connection request timeout: {} ms, socket timeout: {} ms",
connectTimeout,
connectTimeout,
socketTimeout);
DefaultRequestConfig = builder.build();
}

Expand Down Expand Up @@ -411,11 +383,18 @@ public static CloseableHttpClient buildHttpClient(
.useSystemProperties()
.setRedirectStrategy(new DefaultRedirectStrategy())
.setUserAgent(buildUserAgent(userAgentSuffix)) // needed for Okta
.disableCookieManagement(); // SNOW-39748

.disableCookieManagement() // SNOW-39748
.setDefaultRequestConfig(DefaultRequestConfig);
if (key != null && key.usesProxy()) {
HttpHost proxy =
new HttpHost(
key.getProxyHost(), key.getProxyPort(), key.getProxyHttpProtocol().getScheme());
logger.debug(
"Instantiating proxy route planner with non-proxy hosts: {}", key.getNonProxyHosts());
"Configuring proxy and route planner - host: {}, port: {}, scheme: {}, nonProxyHosts: {}",
key.getProxyHost(),
key.getProxyPort(),
key.getProxyHttpProtocol().getScheme(),
key.getNonProxyHosts());
// use the custom proxy properties
SnowflakeMutableProxyRoutePlanner sdkProxyRoutePlanner =
httpClientRoutePlanner.computeIfAbsent(
Expand All @@ -426,7 +405,7 @@ public static CloseableHttpClient buildHttpClient(
key.getProxyPort(),
key.getProxyHttpProtocol(),
key.getNonProxyHosts()));
httpClientBuilder = httpClientBuilder.setProxy(proxy).setRoutePlanner(sdkProxyRoutePlanner);
httpClientBuilder.setProxy(proxy).setRoutePlanner(sdkProxyRoutePlanner);
if (!Strings.isNullOrEmpty(key.getProxyUser())
&& !Strings.isNullOrEmpty(key.getProxyPassword())) {
Credentials credentials =
Expand All @@ -440,13 +419,12 @@ public static CloseableHttpClient buildHttpClient(
key.getProxyHost(),
key.getProxyPort());
credentialsProvider.setCredentials(authScope, credentials);
httpClientBuilder = httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider);
httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider);
}
}
httpClientBuilder.setDefaultRequestConfig(DefaultRequestConfig);
if (downloadUnCompressed) {
logger.debug("Disabling content compression for http client");
httpClientBuilder = httpClientBuilder.disableContentCompression();
httpClientBuilder.disableContentCompression();
}
return httpClientBuilder.build();
} catch (NoSuchAlgorithmException | KeyManagementException ex) {
Expand Down
99 changes: 99 additions & 0 deletions src/test/java/net/snowflake/client/core/HttpUtilTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package net.snowflake.client.core;

import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.fail;

import java.lang.reflect.Field;
import java.util.AbstractMap;
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.Configurable;
import org.apache.http.impl.client.CloseableHttpClient;
import org.hamcrest.CoreMatchers;
import org.hamcrest.Matcher;
import org.hamcrest.MatcherAssert;
import org.junit.jupiter.api.Test;

public class HttpUtilTest {

/** SNOW-1898533 */
@Test
public void buildHttpClientRace() throws InterruptedException {
HttpUtil.httpClient.clear();
// start two threads but only need one to fail
CountDownLatch latch = new CountDownLatch(1);
final Queue<AbstractMap.SimpleEntry<Thread, Throwable>> failures =
new ConcurrentLinkedQueue<>();
final HttpClientSettingsKey noProxyKey = new HttpClientSettingsKey(null);
final HttpClientSettingsKey proxyKey =
new HttpClientSettingsKey(
null, "some.proxy.host", 8080, null, null, null, "http", null, false);

Thread noProxyThread =
new Thread(() -> verifyProxyUsage(noProxyKey, failures, latch), "noProxyThread");
noProxyThread.start();

Thread withProxyThread =
new Thread(() -> verifyProxyUsage(proxyKey, failures, latch), "withProxyThread");
withProxyThread.start();

// if latch goes to zero, then one of the threads failed
// if await times out (returns false), then neither thread has failed (both still running)
boolean failed = latch.await(1, TimeUnit.SECONDS);
noProxyThread.interrupt();
withProxyThread.interrupt();
if (failed) {
AbstractMap.SimpleEntry<Thread, Throwable> failure = failures.remove();
fail(failure.getKey().getName() + " failed", failure.getValue());
}
}

private static void verifyProxyUsage(
HttpClientSettingsKey key,
Queue<AbstractMap.SimpleEntry<Thread, Throwable>> failures,
CountDownLatch latch) {
while (!Thread.currentThread().isInterrupted()) {
try (CloseableHttpClient client = HttpUtil.buildHttpClient(key, null, false)) {
assertHttpClientUsesProxy(client, key.usesProxy());
} catch (Throwable e) {
failures.add(new AbstractMap.SimpleEntry<>(Thread.currentThread(), e));
latch.countDown();
break;
}
}
}

private static void assertHttpClientUsesProxy(CloseableHttpClient client, boolean proxyUsed) {
assertRequestConfigWithoutProxyConfig(client);
assertRoutePlannerOverridden(client, proxyUsed);
}

private static void assertRequestConfigWithoutProxyConfig(CloseableHttpClient client) {
MatcherAssert.assertThat(client, CoreMatchers.instanceOf(Configurable.class));
Configurable c = (Configurable) client;
RequestConfig config = c.getConfig();
assertNull(config.getProxy(), "request config has configured proxy");
}

private static void assertRoutePlannerOverridden(CloseableHttpClient client, boolean proxyUsed) {
try {
// HTTP client does not provide information about proxy settings so to detect that we are
// using proxy we have to look inside via reflection and if the route planner is overridden to
// our proxy class
Field routePlannerField = client.getClass().getDeclaredField("routePlanner");
routePlannerField.setAccessible(true);
Matcher<Object> snowflakeProxyPlannerClassMatcher =
CoreMatchers.instanceOf(SnowflakeMutableProxyRoutePlanner.class);
MatcherAssert.assertThat(
routePlannerField.get(client),
proxyUsed
? snowflakeProxyPlannerClassMatcher
: CoreMatchers.not(snowflakeProxyPlannerClassMatcher));
} catch (NoSuchFieldException | IllegalAccessException e) {
throw new RuntimeException(e);
}
}
}

0 comments on commit 6c6d174

Please sign in to comment.