diff --git a/src/main/java/net/snowflake/client/core/HttpUtil.java b/src/main/java/net/snowflake/client/core/HttpUtil.java index 41ea57f32..09e71a993 100644 --- a/src/main/java/net/snowflake/client/core/HttpUtil.java +++ b/src/main/java/net/snowflake/client/core/HttpUtil.java @@ -129,11 +129,13 @@ public static Duration getSocketTimeout() { @SnowflakeJdbcInternalApi public static void setConnectionTimeout(int timeout) { connectionTimeout = Duration.ofMillis(timeout); + initDefaultRequestConfig(connectionTimeout.toMillis(), getSocketTimeout().toMillis()); } @SnowflakeJdbcInternalApi public static void setSocketTimeout(int timeout) { socketTimeout = Duration.ofMillis(timeout); + initDefaultRequestConfig(getConnectionTimeout().toMillis(), socketTimeout.toMillis()); } public static long getDownloadedConditionTimeoutInSeconds() { @@ -299,49 +301,11 @@ 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)) { - 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); - DefaultRequestConfig = builder.build(); + // 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) { + initDefaultRequestConfig(connectTimeout, socketTimeout); } TrustManager[] trustManagers = null; @@ -411,11 +375,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( @@ -426,7 +397,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 = @@ -440,13 +411,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) { @@ -454,6 +424,20 @@ public static CloseableHttpClient buildHttpClient( } } + private static void initDefaultRequestConfig(long connectTimeout, long socketTimeout) { + RequestConfig.Builder builder = + RequestConfig.custom() + .setConnectTimeout((int) connectTimeout) + .setConnectionRequestTimeout((int) connectTimeout) + .setSocketTimeout((int) socketTimeout); + logger.debug( + "Rebuilding request config. Connect timeout: {} ms, connection request timeout: {} ms, socket timeout: {} ms", + connectTimeout, + connectTimeout, + socketTimeout); + DefaultRequestConfig = builder.build(); + } + public static void updateRoutePlanner(HttpClientSettingsKey key) { if (httpClientRoutePlanner.containsKey(key) && !httpClientRoutePlanner diff --git a/src/test/java/net/snowflake/client/core/HttpUtilTest.java b/src/test/java/net/snowflake/client/core/HttpUtilTest.java new file mode 100644 index 000000000..d9b07edd3 --- /dev/null +++ b/src/test/java/net/snowflake/client/core/HttpUtilTest.java @@ -0,0 +1,105 @@ +/* + * Copyright (c) 2025 Snowflake Computing Inc. All right reserved. + */ +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 { + + /** + * Test based on reported + * issue in 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> 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 failure = failures.remove(); + fail(failure.getKey().getName() + " failed", failure.getValue()); + } + } + + private static void verifyProxyUsage( + HttpClientSettingsKey key, + Queue> 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 snowflakeProxyPlannerClassMatcher = + CoreMatchers.instanceOf(SnowflakeMutableProxyRoutePlanner.class); + MatcherAssert.assertThat( + routePlannerField.get(client), + proxyUsed + ? snowflakeProxyPlannerClassMatcher + : CoreMatchers.not(snowflakeProxyPlannerClassMatcher)); + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } +} diff --git a/src/test/java/net/snowflake/client/core/SessionUtilTest.java b/src/test/java/net/snowflake/client/core/SessionUtilTest.java index 86819dc5b..e7bb3e486 100644 --- a/src/test/java/net/snowflake/client/core/SessionUtilTest.java +++ b/src/test/java/net/snowflake/client/core/SessionUtilTest.java @@ -88,25 +88,30 @@ public void testParameterParsing() { @Test public void testConvertSystemPropertyToIntValue() { - // Test that setting real value works - System.setProperty("net.snowflake.jdbc.max_connections", "500"); - assertEquals( - 500, - SystemUtil.convertSystemPropertyToIntValue( - HttpUtil.JDBC_MAX_CONNECTIONS_PROPERTY, HttpUtil.DEFAULT_MAX_CONNECTIONS)); - // Test that entering a non-int sets the value to the default - System.setProperty("net.snowflake.jdbc.max_connections", "notAnInteger"); - assertEquals( - HttpUtil.DEFAULT_MAX_CONNECTIONS, - SystemUtil.convertSystemPropertyToIntValue( - HttpUtil.JDBC_MAX_CONNECTIONS_PROPERTY, HttpUtil.DEFAULT_MAX_CONNECTIONS)); - // Test another system property - System.setProperty("net.snowflake.jdbc.max_connections_per_route", "30"); - assertEquals( - 30, - SystemUtil.convertSystemPropertyToIntValue( - HttpUtil.JDBC_MAX_CONNECTIONS_PER_ROUTE_PROPERTY, - HttpUtil.DEFAULT_MAX_CONNECTIONS_PER_ROUTE)); + try { + // Test that setting real value works + System.setProperty("net.snowflake.jdbc.max_connections", "500"); + assertEquals( + 500, + SystemUtil.convertSystemPropertyToIntValue( + HttpUtil.JDBC_MAX_CONNECTIONS_PROPERTY, HttpUtil.DEFAULT_MAX_CONNECTIONS)); + // Test that entering a non-int sets the value to the default + System.setProperty("net.snowflake.jdbc.max_connections", "notAnInteger"); + assertEquals( + HttpUtil.DEFAULT_MAX_CONNECTIONS, + SystemUtil.convertSystemPropertyToIntValue( + HttpUtil.JDBC_MAX_CONNECTIONS_PROPERTY, HttpUtil.DEFAULT_MAX_CONNECTIONS)); + // Test another system property + System.setProperty("net.snowflake.jdbc.max_connections_per_route", "30"); + assertEquals( + 30, + SystemUtil.convertSystemPropertyToIntValue( + HttpUtil.JDBC_MAX_CONNECTIONS_PER_ROUTE_PROPERTY, + HttpUtil.DEFAULT_MAX_CONNECTIONS_PER_ROUTE)); + } finally { + System.clearProperty("net.snowflake.jdbc.max_connections"); + System.clearProperty("net.snowflake.jdbc.max_connections_per_route"); + } } @Test