Skip to content
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.

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

SNOW-1042432: Remove dependency on com.amazonaws.Protocol from HttpClientSettingsKey #1627

Merged
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
7ca3a88
remove dependency on com.amazonaws.Protocol from HttpClientSettingsKey
jtjeferreira Feb 5, 2024
a6c26ce
keep backwards compatibility
jtjeferreira Feb 6, 2024
4db26fc
copyright header
jtjeferreira Feb 6, 2024
7cfb93b
fix copyright header
jtjeferreira Feb 8, 2024
f1b57bc
do not use getProxyHttpProtocol().toString(), use key.getProxyHttpPro…
jtjeferreira Feb 8, 2024
6cd37d8
move setProxyForS3 to S3HttpUtil
jtjeferreira Feb 8, 2024
28ea19e
deprecate SnowflakeMutableProxyRoutePlanner constructor
jtjeferreira Feb 8, 2024
7dca1fc
remove last usage of deprecated getProxyProtocol
jtjeferreira Feb 8, 2024
aaa5ca3
make a copy of com.amazonaws.http.apache.SdkProxyRoutePlanner but doe…
jtjeferreira Feb 8, 2024
6e10f57
Revert "make a copy of com.amazonaws.http.apache.SdkProxyRoutePlanner…
jtjeferreira Feb 9, 2024
4c735cc
fix HttpProtocol constructor argument
jtjeferreira Feb 9, 2024
fce6bc5
add Deprecated annotations
jtjeferreira Feb 9, 2024
92a67d6
remove duplicate code by calling the other constructor
jtjeferreira Feb 9, 2024
7e7efab
add copyright header
jtjeferreira Feb 9, 2024
1399811
Merge remote-tracking branch 'upstream/master' into remove_dependecy_…
jtjeferreira Feb 9, 2024
12a8f92
fmt
jtjeferreira Feb 9, 2024
c4e2225
change SnowflakeMutableProxyRoutePlanner constructor
jtjeferreira Feb 14, 2024
d326351
reformat
jtjeferreira Feb 14, 2024
f780f6d
Merge remote-tracking branch 'upstream/master' into remove_dependecy_…
jtjeferreira Feb 14, 2024
ad9f4c1
mark HttpClientSettingsKey.getProxyPassword as internal
jtjeferreira Feb 14, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

package net.snowflake.client.core;

import com.amazonaws.Protocol;
import com.google.common.base.Strings;
import java.io.Serializable;

Expand Down Expand Up @@ -124,16 +123,26 @@ public String getUserAgentSuffix() {
}

/** Be careful of using this! Should only be called when password is later masked. */
String getProxyPassword() {
public String getProxyPassword() {
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
return this.proxyPassword;
}

public String getNonProxyHosts() {
return this.nonProxyHosts;
}

public Protocol getProxyProtocol() {
return this.proxyProtocol.equalsIgnoreCase("https") ? Protocol.HTTPS : Protocol.HTTP;
/**
* @deprecated Use {@link #getProxyHttpProtocol()}
* @return ProxyProtocol
*/
@Deprecated
public com.amazonaws.Protocol getProxyProtocol() {
return this.proxyProtocol.equalsIgnoreCase("https")
? com.amazonaws.Protocol.HTTPS : com.amazonaws.Protocol.HTTP;
}

public HttpProtocol getProxyHttpProtocol() {
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
return this.proxyProtocol.equalsIgnoreCase("https") ? HttpProtocol.HTTPS : HttpProtocol.HTTP;
}

public Boolean getGzipDisabled() {
Expand Down
21 changes: 21 additions & 0 deletions src/main/java/net/snowflake/client/core/HttpProtocol.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright (c) 2024 Snowflake Computing Inc. All rights reserved.
*/

package net.snowflake.client.core;
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved

public enum HttpProtocol {
HTTP("http"),

HTTPS("https");

private final String scheme;

HttpProtocol(String scheme) {
this.scheme = scheme;
}

public String getScheme() {
return scheme;
}
}
65 changes: 9 additions & 56 deletions src/main/java/net/snowflake/client/core/HttpUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import static org.apache.http.client.config.CookieSpecs.IGNORE_COOKIES;

import com.amazonaws.ClientConfiguration;
import com.amazonaws.Protocol;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.microsoft.azure.storage.OperationContext;
Expand All @@ -32,6 +31,7 @@
import net.snowflake.client.jdbc.RestRequest;
import net.snowflake.client.jdbc.SnowflakeSQLException;
import net.snowflake.client.jdbc.SnowflakeUtil;
import net.snowflake.client.jdbc.cloud.storage.S3HttpUtil;
import net.snowflake.client.log.ArgSupplier;
import net.snowflake.client.log.SFLogger;
import net.snowflake.client.log.SFLoggerFactory;
Expand Down Expand Up @@ -139,19 +139,11 @@ public static void closeExpiredAndIdleConnections() {
*
* @param key key to HttpClient map containing OCSP and proxy info
* @param clientConfig the configuration needed by S3 to set the proxy
* @deprecated use S3HttpUtil.setProxyForS3(HttpClientSettingsKey, ClientConfiguration) instead
*/
@Deprecated
public static void setProxyForS3(HttpClientSettingsKey key, ClientConfiguration clientConfig) {
if (key != null && key.usesProxy()) {
clientConfig.setProxyProtocol(key.getProxyProtocol());
clientConfig.setProxyHost(key.getProxyHost());
clientConfig.setProxyPort(key.getProxyPort());
clientConfig.setNonProxyHosts(key.getNonProxyHosts());
if (!Strings.isNullOrEmpty(key.getProxyUser())
&& !Strings.isNullOrEmpty(key.getProxyPassword())) {
clientConfig.setProxyUsername(key.getProxyUser());
clientConfig.setProxyPassword(key.getProxyPassword());
}
}
S3HttpUtil.setProxyForS3(key, clientConfig);
}

/**
Expand All @@ -161,51 +153,12 @@ public static void setProxyForS3(HttpClientSettingsKey key, ClientConfiguration
* @param proxyProperties proxy properties
* @param clientConfig the configuration needed by S3 to set the proxy
* @throws SnowflakeSQLException
* @deprecated use S3HttpUtil.setSessionlessProxyForS3(Properties, ClientConfiguration) instead
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
*/
@Deprecated
public static void setSessionlessProxyForS3(
Properties proxyProperties, ClientConfiguration clientConfig) throws SnowflakeSQLException {
// do nothing yet
if (proxyProperties != null
&& proxyProperties.size() > 0
&& proxyProperties.getProperty(SFSessionProperty.USE_PROXY.getPropertyKey()) != null) {
Boolean useProxy =
Boolean.valueOf(
proxyProperties.getProperty(SFSessionProperty.USE_PROXY.getPropertyKey()));
if (useProxy) {
// set up other proxy related values.
String proxyHost =
proxyProperties.getProperty(SFSessionProperty.PROXY_HOST.getPropertyKey());
int proxyPort;
try {
proxyPort =
Integer.parseInt(
proxyProperties.getProperty(SFSessionProperty.PROXY_PORT.getPropertyKey()));
} catch (NumberFormatException | NullPointerException e) {
throw new SnowflakeSQLException(
ErrorCode.INVALID_PROXY_PROPERTIES, "Could not parse port number");
}
String proxyUser =
proxyProperties.getProperty(SFSessionProperty.PROXY_USER.getPropertyKey());
String proxyPassword =
proxyProperties.getProperty(SFSessionProperty.PROXY_PASSWORD.getPropertyKey());
String nonProxyHosts =
proxyProperties.getProperty(SFSessionProperty.NON_PROXY_HOSTS.getPropertyKey());
String proxyProtocol =
proxyProperties.getProperty(SFSessionProperty.PROXY_PROTOCOL.getPropertyKey());
Protocol protocolEnum =
(!Strings.isNullOrEmpty(proxyProtocol) && proxyProtocol.equalsIgnoreCase("https"))
? Protocol.HTTPS
: Protocol.HTTP;
clientConfig.setProxyHost(proxyHost);
clientConfig.setProxyPort(proxyPort);
clientConfig.setNonProxyHosts(nonProxyHosts);
clientConfig.setProxyProtocol(protocolEnum);
if (!Strings.isNullOrEmpty(proxyUser) && !Strings.isNullOrEmpty(proxyPassword)) {
clientConfig.setProxyUsername(proxyUser);
clientConfig.setProxyPassword(proxyPassword);
}
}
}
S3HttpUtil.setSessionlessProxyForS3(proxyProperties, clientConfig);
}

/**
Expand Down Expand Up @@ -320,7 +273,7 @@ public static CloseableHttpClient buildHttpClient(
HttpHost proxy =
(key != null && key.usesProxy())
? new HttpHost(
key.getProxyHost(), key.getProxyPort(), key.getProxyProtocol().toString())
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
Expand Down Expand Up @@ -403,7 +356,7 @@ public static CloseableHttpClient buildHttpClient(
new SnowflakeMutableProxyRoutePlanner(
key.getProxyHost(),
key.getProxyPort(),
key.getProxyProtocol(),
key.getProxyHttpProtocol(),
key.getNonProxyHosts()));
httpClientBuilder = httpClientBuilder.setProxy(proxy).setRoutePlanner(sdkProxyRoutePlanner);
if (!Strings.isNullOrEmpty(key.getProxyUser())
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/net/snowflake/client/core/SFSession.java
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ public synchronized void open() throws SFException, SnowflakeSQLException {
httpClientSettingsKey.getProxyUser(),
!Strings.isNullOrEmpty(httpClientSettingsKey.getProxyPassword()) ? "***" : "(empty)",
httpClientSettingsKey.getNonProxyHosts(),
httpClientSettingsKey.getProxyProtocol());
httpClientSettingsKey.getProxyHttpProtocol());

// TODO: temporarily hardcode sessionParameter debug info. will be changed in the future
SFLoginInput loginInput = new SFLoginInput();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,31 @@ public class SnowflakeMutableProxyRoutePlanner implements HttpRoutePlanner, Seri
private String host;
private int proxyPort;
private String nonProxyHosts;
private Protocol protocol;
private HttpProtocol protocol;

/**
* @deprecated use SnowflakeMutableProxyRoutePlanner(String host, int proxyPort, HttpProtocol
* protocol, String nonProxyHosts)
*/
@Deprecated
public SnowflakeMutableProxyRoutePlanner(
String host, int proxyPort, Protocol proxyProtocol, String nonProxyHosts) {
proxyRoutePlanner = new SdkProxyRoutePlanner(host, proxyPort, proxyProtocol, nonProxyHosts);
this.host = host;
this.proxyPort = proxyPort;
this.nonProxyHosts = nonProxyHosts;
this.protocol = proxyProtocol;
this.protocol = toSnowflakeProtocol(proxyProtocol);
}

public SnowflakeMutableProxyRoutePlanner(
String host, int proxyPort, HttpProtocol proxyProtocol, String nonProxyHosts) {
this(host, proxyPort, toAwsProtocol(proxyProtocol), nonProxyHosts);
}

public void setNonProxyHosts(String nonProxyHosts) {
this.nonProxyHosts = nonProxyHosts;
proxyRoutePlanner = new SdkProxyRoutePlanner(host, proxyPort, protocol, nonProxyHosts);
proxyRoutePlanner =
new SdkProxyRoutePlanner(host, proxyPort, toAwsProtocol(protocol), nonProxyHosts);
}

public String getNonProxyHosts() {
Expand All @@ -49,4 +60,12 @@ public HttpRoute determineRoute(HttpHost target, HttpRequest request, HttpContex
throws HttpException {
return proxyRoutePlanner.determineRoute(target, request, context);
}

private static Protocol toAwsProtocol(HttpProtocol protocol) {
return protocol == HttpProtocol.HTTP ? Protocol.HTTP : Protocol.HTTPS;
}

private static HttpProtocol toSnowflakeProtocol(Protocol protocol) {
return protocol == Protocol.HTTP ? HttpProtocol.HTTP : HttpProtocol.HTTPS;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* Copyright (c) 2024 Snowflake Computing Inc. All rights reserved.
*/

package net.snowflake.client.jdbc.cloud.storage;
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved

import com.amazonaws.ClientConfiguration;
import com.amazonaws.Protocol;
import com.google.common.base.Strings;
import java.util.Properties;
import net.snowflake.client.core.HttpClientSettingsKey;
import net.snowflake.client.core.HttpProtocol;
import net.snowflake.client.core.SFSessionProperty;
import net.snowflake.client.core.SnowflakeJdbcInternalApi;
import net.snowflake.client.jdbc.ErrorCode;
import net.snowflake.client.jdbc.SnowflakeSQLException;

@SnowflakeJdbcInternalApi
public class S3HttpUtil {
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
/**
* A static function to set S3 proxy params when there is a valid session
*
* @param key key to HttpClient map containing OCSP and proxy info
* @param clientConfig the configuration needed by S3 to set the proxy
*/
public static void setProxyForS3(HttpClientSettingsKey key, ClientConfiguration clientConfig) {
if (key != null && key.usesProxy()) {
clientConfig.setProxyProtocol(
key.getProxyHttpProtocol() == HttpProtocol.HTTPS ? Protocol.HTTPS : Protocol.HTTP);
clientConfig.setProxyHost(key.getProxyHost());
clientConfig.setProxyPort(key.getProxyPort());
clientConfig.setNonProxyHosts(key.getNonProxyHosts());
if (!Strings.isNullOrEmpty(key.getProxyUser())
&& !Strings.isNullOrEmpty(key.getProxyPassword())) {
clientConfig.setProxyUsername(key.getProxyUser());
clientConfig.setProxyPassword(key.getProxyPassword());
}
}
}

/**
* A static function to set S3 proxy params for sessionless connections using the proxy params
* from the StageInfo
*
* @param proxyProperties proxy properties
* @param clientConfig the configuration needed by S3 to set the proxy
* @throws SnowflakeSQLException
*/
public static void setSessionlessProxyForS3(
Properties proxyProperties, ClientConfiguration clientConfig) throws SnowflakeSQLException {
// do nothing yet
if (proxyProperties != null
&& proxyProperties.size() > 0
&& proxyProperties.getProperty(SFSessionProperty.USE_PROXY.getPropertyKey()) != null) {
Boolean useProxy =
Boolean.valueOf(
proxyProperties.getProperty(SFSessionProperty.USE_PROXY.getPropertyKey()));
if (useProxy) {
// set up other proxy related values.
String proxyHost =
proxyProperties.getProperty(SFSessionProperty.PROXY_HOST.getPropertyKey());
int proxyPort;
try {
proxyPort =
Integer.parseInt(
proxyProperties.getProperty(SFSessionProperty.PROXY_PORT.getPropertyKey()));
} catch (NumberFormatException | NullPointerException e) {
throw new SnowflakeSQLException(
ErrorCode.INVALID_PROXY_PROPERTIES, "Could not parse port number");
}
String proxyUser =
proxyProperties.getProperty(SFSessionProperty.PROXY_USER.getPropertyKey());
String proxyPassword =
proxyProperties.getProperty(SFSessionProperty.PROXY_PASSWORD.getPropertyKey());
String nonProxyHosts =
proxyProperties.getProperty(SFSessionProperty.NON_PROXY_HOSTS.getPropertyKey());
String proxyProtocol =
proxyProperties.getProperty(SFSessionProperty.PROXY_PROTOCOL.getPropertyKey());
Protocol protocolEnum =
(!Strings.isNullOrEmpty(proxyProtocol) && proxyProtocol.equalsIgnoreCase("https"))
? Protocol.HTTPS
: Protocol.HTTP;
clientConfig.setProxyHost(proxyHost);
clientConfig.setProxyPort(proxyPort);
clientConfig.setNonProxyHosts(nonProxyHosts);
clientConfig.setProxyProtocol(protocolEnum);
if (!Strings.isNullOrEmpty(proxyUser) && !Strings.isNullOrEmpty(proxyPassword)) {
clientConfig.setProxyUsername(proxyUser);
clientConfig.setProxyPassword(proxyPassword);
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,9 @@ private void setupSnowflakeS3Client(
clientConfig.withSignerOverride("AWSS3V4SignerType");
clientConfig.getApacheHttpClientConfig().setSslSocketFactory(getSSLConnectionSocketFactory());
if (session != null) {
HttpUtil.setProxyForS3(session.getHttpClientKey(), clientConfig);
S3HttpUtil.setProxyForS3(session.getHttpClientKey(), clientConfig);
} else {
HttpUtil.setSessionlessProxyForS3(proxyProperties, clientConfig);
S3HttpUtil.setSessionlessProxyForS3(proxyProperties, clientConfig);
}
AmazonS3Builder<?, ?> amazonS3Builder = AmazonS3Client.builder();
if (encMat != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import net.snowflake.client.jdbc.ErrorCode;
import net.snowflake.client.jdbc.SnowflakeSQLException;
import net.snowflake.client.jdbc.SnowflakeUtil;
import net.snowflake.client.jdbc.cloud.storage.S3HttpUtil;
import org.junit.Test;

public class CoreUtilsMiscellaneousTest {
Expand Down Expand Up @@ -106,7 +107,7 @@ public void testSetProxyForS3() {
"jdbc",
false);
ClientConfiguration clientConfig = new ClientConfiguration();
HttpUtil.setProxyForS3(testKey, clientConfig);
S3HttpUtil.setProxyForS3(testKey, clientConfig);
assertEquals(Protocol.HTTPS, clientConfig.getProxyProtocol());
assertEquals("snowflakecomputing.com", clientConfig.getProxyHost());
assertEquals(443, clientConfig.getProxyPort());
Expand All @@ -126,7 +127,7 @@ public void testSetSessionlessProxyForS3() throws SnowflakeSQLException {
props.put("nonProxyHosts", "baz.com | foo.com");
props.put("proxyProtocol", "http");
ClientConfiguration clientConfig = new ClientConfiguration();
HttpUtil.setSessionlessProxyForS3(props, clientConfig);
S3HttpUtil.setSessionlessProxyForS3(props, clientConfig);
assertEquals(Protocol.HTTP, clientConfig.getProxyProtocol());
assertEquals("localhost", clientConfig.getProxyHost());
assertEquals(8084, clientConfig.getProxyPort());
Expand All @@ -136,7 +137,7 @@ public void testSetSessionlessProxyForS3() throws SnowflakeSQLException {
// Test that exception is thrown when port number is invalid
props.put("proxyPort", "invalidnumber");
try {
HttpUtil.setSessionlessProxyForS3(props, clientConfig);
S3HttpUtil.setSessionlessProxyForS3(props, clientConfig);
} catch (SnowflakeSQLException e) {
assertEquals((int) ErrorCode.INVALID_PROXY_PROPERTIES.getMessageCode(), e.getErrorCode());
}
Expand Down Expand Up @@ -346,7 +347,7 @@ public void testNullAndEmptyProxySettingsForS3() {
HttpClientSettingsKey testKey =
new HttpClientSettingsKey(OCSPMode.FAIL_OPEN, null, 443, null, null, null, "", "", false);
ClientConfiguration clientConfig = new ClientConfiguration();
HttpUtil.setProxyForS3(testKey, clientConfig);
S3HttpUtil.setProxyForS3(testKey, clientConfig);
assertEquals(Protocol.HTTP, clientConfig.getProxyProtocol());
assertEquals("", clientConfig.getProxyHost());
assertEquals(443, clientConfig.getProxyPort());
Expand Down
Loading
Loading