Skip to content

Commit

Permalink
remove allowEmptyProxyConfig parameter
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-knozderko committed Aug 1, 2024
1 parent dd4d836 commit 737d695
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 75 deletions.
30 changes: 13 additions & 17 deletions Snowflake.Data.Tests/UnitTests/HttpUtilTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,11 @@ public void TestGetJitter(int seconds)
}

[Test]
public void TestCreateHttpClientHandlerWithExplicitProxy([Values] bool allowEmptyProxy)
public void TestCreateHttpClientHandlerWithExplicitProxy()
{
// given
var config = new HttpClientConfig(
true,
allowEmptyProxy,
true,
"snowflake.com",
"123",
Expand All @@ -127,11 +126,10 @@ public void TestCreateHttpClientHandlerWithExplicitProxy([Values] bool allowEmpt
}

[Test]
public void TestCreateHttpClientHandlerWithImplicitProxy()
public void TestCreateHttpClientHandlerWithDefaultProxy()
{
// given
var config = new HttpClientConfig(
true,
true,
true,
null,
Expand All @@ -153,21 +151,20 @@ public void TestCreateHttpClientHandlerWithImplicitProxy()
}

[Test]
public void TestDoNotCreateHttpClientHandlerWithImplicitProxyWhenEmptyProxyNotAllowed()
public void TestCreateHttpClientHandlerWithoutProxy()
{
// given
var config = new HttpClientConfig(
true,
false,
true,
false,
null,
null,
null,
null,
null,
false,
false,
7
0
);

// when
Expand All @@ -179,21 +176,20 @@ public void TestDoNotCreateHttpClientHandlerWithImplicitProxyWhenEmptyProxyNotAl
}

[Test]
public void ShouldCreateHttpClientHandlerWithoutProxy([Values] bool allowEmptyProxy)
public void TestIgnoreProxyDetailsIfProxyDisabled()
{
// given
var config = new HttpClientConfig(
true,
false,
allowEmptyProxy,
false,
null,
null,
null,
null,
null,
"snowflake.com",
"123",
"testUser",
"proxyPassword",
"localhost",
false,
false,
0
7
);

// when
Expand Down
49 changes: 12 additions & 37 deletions Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -167,19 +167,6 @@ public void TestResolveConnectionArea(string host, string expectedMessage)
Assert.AreEqual(expectedMessage, message);
}

[Test]
[TestCase("ACCOUNT=account;USER=test;PASSWORD=test;ALLOWEMPTYPROXY=false", "false")]
[TestCase("ACCOUNT=account;USER=test;PASSWORD=test;ALLOWEMPTYPROXY=true", "true")]
[TestCase("ACCOUNT=account;USER=test;PASSWORD=test", "false")]
public void TestParseAllowEmptyProxy(string connectionString, string expectedAllowEmptyProxy)
{
// act
var properties = SFSessionProperties.ParseConnectionString(connectionString, null);

// assert
Assert.AreEqual(expectedAllowEmptyProxy, properties[SFSessionProperty.ALLOWEMPTYPROXY]);
}

[Test]
public void TestFailWhenProxyConfiguredWithoutPort()
{
Expand Down Expand Up @@ -288,8 +275,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
{ SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT, DefaultValue(SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT) },
{ SFSessionProperty.EXPIRATIONTIMEOUT, DefaultValue(SFSessionProperty.EXPIRATIONTIMEOUT) },
{ SFSessionProperty.POOLINGENABLED, DefaultValue(SFSessionProperty.POOLINGENABLED) },
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) },
{ SFSessionProperty.ALLOWEMPTYPROXY, DefaultValue(SFSessionProperty.ALLOWEMPTYPROXY) }
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) }
}
};

Expand Down Expand Up @@ -325,8 +311,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
{ SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT, DefaultValue(SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT) },
{ SFSessionProperty.EXPIRATIONTIMEOUT, DefaultValue(SFSessionProperty.EXPIRATIONTIMEOUT) },
{ SFSessionProperty.POOLINGENABLED, DefaultValue(SFSessionProperty.POOLINGENABLED) },
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) },
{ SFSessionProperty.ALLOWEMPTYPROXY, DefaultValue(SFSessionProperty.ALLOWEMPTYPROXY) }
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) }
}
};
var testCaseWithProxySettings = new TestCase()
Expand Down Expand Up @@ -364,8 +349,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
{ SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT, DefaultValue(SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT) },
{ SFSessionProperty.EXPIRATIONTIMEOUT, DefaultValue(SFSessionProperty.EXPIRATIONTIMEOUT) },
{ SFSessionProperty.POOLINGENABLED, DefaultValue(SFSessionProperty.POOLINGENABLED) },
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) },
{ SFSessionProperty.ALLOWEMPTYPROXY, DefaultValue(SFSessionProperty.ALLOWEMPTYPROXY) }
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) }
},
ConnectionString =
$"ACCOUNT={defAccount};USER={defUser};PASSWORD={defPassword};useProxy=true;proxyHost=proxy.com;proxyPort=1234;nonProxyHosts=localhost"
Expand Down Expand Up @@ -405,8 +389,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
{ SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT, DefaultValue(SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT) },
{ SFSessionProperty.EXPIRATIONTIMEOUT, DefaultValue(SFSessionProperty.EXPIRATIONTIMEOUT) },
{ SFSessionProperty.POOLINGENABLED, DefaultValue(SFSessionProperty.POOLINGENABLED) },
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) },
{ SFSessionProperty.ALLOWEMPTYPROXY, DefaultValue(SFSessionProperty.ALLOWEMPTYPROXY) }
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) }
},
ConnectionString =
$"ACCOUNT={defAccount};USER={defUser};PASSWORD={defPassword};proxyHost=proxy.com;proxyPort=1234;nonProxyHosts=localhost"
Expand Down Expand Up @@ -445,8 +428,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
{ SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT, DefaultValue(SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT) },
{ SFSessionProperty.EXPIRATIONTIMEOUT, DefaultValue(SFSessionProperty.EXPIRATIONTIMEOUT) },
{ SFSessionProperty.POOLINGENABLED, DefaultValue(SFSessionProperty.POOLINGENABLED) },
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) },
{ SFSessionProperty.ALLOWEMPTYPROXY, DefaultValue(SFSessionProperty.ALLOWEMPTYPROXY) }
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) }
}
};
var testCaseWithIncludeRetryReason = new TestCase()
Expand Down Expand Up @@ -482,8 +464,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
{ SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT, DefaultValue(SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT) },
{ SFSessionProperty.EXPIRATIONTIMEOUT, DefaultValue(SFSessionProperty.EXPIRATIONTIMEOUT) },
{ SFSessionProperty.POOLINGENABLED, DefaultValue(SFSessionProperty.POOLINGENABLED) },
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) },
{ SFSessionProperty.ALLOWEMPTYPROXY, DefaultValue(SFSessionProperty.ALLOWEMPTYPROXY) }
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) }
}
};
var testCaseWithDisableQueryContextCache = new TestCase()
Expand Down Expand Up @@ -518,8 +499,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
{ SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT, DefaultValue(SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT) },
{ SFSessionProperty.EXPIRATIONTIMEOUT, DefaultValue(SFSessionProperty.EXPIRATIONTIMEOUT) },
{ SFSessionProperty.POOLINGENABLED, DefaultValue(SFSessionProperty.POOLINGENABLED) },
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) },
{ SFSessionProperty.ALLOWEMPTYPROXY, DefaultValue(SFSessionProperty.ALLOWEMPTYPROXY) }
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) }
},
ConnectionString =
$"ACCOUNT={defAccount};USER={defUser};PASSWORD={defPassword};DISABLEQUERYCONTEXTCACHE=true"
Expand Down Expand Up @@ -556,8 +536,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
{ SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT, DefaultValue(SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT) },
{ SFSessionProperty.EXPIRATIONTIMEOUT, DefaultValue(SFSessionProperty.EXPIRATIONTIMEOUT) },
{ SFSessionProperty.POOLINGENABLED, DefaultValue(SFSessionProperty.POOLINGENABLED) },
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) },
{ SFSessionProperty.ALLOWEMPTYPROXY, DefaultValue(SFSessionProperty.ALLOWEMPTYPROXY) }
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) }
},
ConnectionString =
$"ACCOUNT={defAccount};USER={defUser};PASSWORD={defPassword};DISABLE_CONSOLE_LOGIN=false"
Expand Down Expand Up @@ -596,8 +575,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
{ SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT, DefaultValue(SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT) },
{ SFSessionProperty.EXPIRATIONTIMEOUT, DefaultValue(SFSessionProperty.EXPIRATIONTIMEOUT) },
{ SFSessionProperty.POOLINGENABLED, DefaultValue(SFSessionProperty.POOLINGENABLED) },
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) },
{ SFSessionProperty.ALLOWEMPTYPROXY, DefaultValue(SFSessionProperty.ALLOWEMPTYPROXY) }
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) }
}
};
var testCaseUnderscoredAccountName = new TestCase()
Expand Down Expand Up @@ -633,8 +611,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
{ SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT, DefaultValue(SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT) },
{ SFSessionProperty.EXPIRATIONTIMEOUT, DefaultValue(SFSessionProperty.EXPIRATIONTIMEOUT) },
{ SFSessionProperty.POOLINGENABLED, DefaultValue(SFSessionProperty.POOLINGENABLED) },
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) },
{ SFSessionProperty.ALLOWEMPTYPROXY, DefaultValue(SFSessionProperty.ALLOWEMPTYPROXY) }
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) }
}
};
var testCaseUnderscoredAccountNameWithEnabledAllowUnderscores = new TestCase()
Expand Down Expand Up @@ -670,8 +647,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
{ SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT, DefaultValue(SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT) },
{ SFSessionProperty.EXPIRATIONTIMEOUT, DefaultValue(SFSessionProperty.EXPIRATIONTIMEOUT) },
{ SFSessionProperty.POOLINGENABLED, DefaultValue(SFSessionProperty.POOLINGENABLED) },
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) },
{ SFSessionProperty.ALLOWEMPTYPROXY, DefaultValue(SFSessionProperty.ALLOWEMPTYPROXY) }
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) }
}
};
var testQueryTag = "Test QUERY_TAG 12345";
Expand Down Expand Up @@ -709,8 +685,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
{ SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT, DefaultValue(SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT) },
{ SFSessionProperty.EXPIRATIONTIMEOUT, DefaultValue(SFSessionProperty.EXPIRATIONTIMEOUT) },
{ SFSessionProperty.POOLINGENABLED, DefaultValue(SFSessionProperty.POOLINGENABLED) },
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) },
{ SFSessionProperty.ALLOWEMPTYPROXY, DefaultValue(SFSessionProperty.ALLOWEMPTYPROXY) }
{ SFSessionProperty.DISABLE_SAML_URL_CHECK, DefaultValue(SFSessionProperty.DISABLE_SAML_URL_CHECK) }
}
};

Expand Down
14 changes: 5 additions & 9 deletions Snowflake.Data/Core/HttpUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ public class HttpClientConfig
{
public HttpClientConfig(
bool crlCheckEnabled,
bool allowEmptyProxy,
bool useProxy,
string proxyHost,
string proxyPort,
Expand All @@ -34,7 +33,6 @@ public HttpClientConfig(
bool includeRetryReason = true)
{
CrlCheckEnabled = crlCheckEnabled;
AllowEmptyProxy = allowEmptyProxy;
UseProxy = useProxy;
ProxyHost = proxyHost;
ProxyPort = proxyPort;
Expand All @@ -49,7 +47,6 @@ public HttpClientConfig(
ConfKey = string.Join(";",
new string[] {
crlCheckEnabled.ToString(),
allowEmptyProxy.ToString(),
useProxy.ToString(),
proxyHost,
proxyPort,
Expand All @@ -63,7 +60,6 @@ public HttpClientConfig(
}

public readonly bool CrlCheckEnabled;
public readonly bool AllowEmptyProxy;
public readonly bool UseProxy;
public readonly string ProxyHost;
public readonly string ProxyPort;
Expand Down Expand Up @@ -162,17 +158,17 @@ internal HttpMessageHandler SetupCustomHttpHandler(HttpClientConfig config)
};
}

httpHandler.UseProxy = config.UseProxy && (config.AllowEmptyProxy || !string.IsNullOrEmpty(config.ProxyHost));
httpHandler.UseProxy = config.UseProxy;

if (httpHandler.UseProxy && !string.IsNullOrEmpty(config.ProxyHost))
if (config.UseProxy && !string.IsNullOrEmpty(config.ProxyHost))
{
logger.Info("Configuring proxy based on connection string properties");
logger.Info("Configuring proxy based on connection properties");
var proxy = ConfigureWebProxy(config);
httpHandler.Proxy = proxy;
}
else if (httpHandler.UseProxy)
else if (config.UseProxy)
{
logger.Info("Proxy enabled, but not configured due to allowEmptyProxy property set to true");
logger.Info("Using a default proxy");
}

return httpHandler;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ public HttpClientConfig BuildHttpClientConfig()
{
return new HttpClientConfig(
!insecureMode,
proxyProperties.allowEmptyProxy,
proxyProperties.useProxy,
proxyProperties.proxyHost,
proxyProperties.proxyPort,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ namespace Snowflake.Data.Core

internal class SFSessionHttpClientProxyProperties
{
internal bool allowEmptyProxy = false;
internal bool useProxy = false;
internal string proxyHost = null;
internal string proxyPort = null;
Expand All @@ -30,7 +29,6 @@ public SFSessionHttpClientProxyProperties ExtractProperties(SFSessionProperties
if (properties.useProxy)
{
// Let's try to get the associated RestRequester
properties.allowEmptyProxy = Boolean.Parse(propertiesDictionary[SFSessionProperty.ALLOWEMPTYPROXY]);
propertiesDictionary.TryGetValue(SFSessionProperty.PROXYHOST, out properties.proxyHost);
propertiesDictionary.TryGetValue(SFSessionProperty.PROXYPORT, out properties.proxyPort);
propertiesDictionary.TryGetValue(SFSessionProperty.NONPROXYHOSTS, out properties.nonProxyHosts);
Expand Down
4 changes: 1 addition & 3 deletions Snowflake.Data/Core/Session/SFSessionProperty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,7 @@ internal enum SFSessionProperty
[SFSessionPropertyAttr(required = false, defaultValue = "true")]
POOLINGENABLED,
[SFSessionPropertyAttr(required = false, defaultValue = "false")]
DISABLE_SAML_URL_CHECK,
[SFSessionPropertyAttr(required = false, defaultValue = "false")]
ALLOWEMPTYPROXY
DISABLE_SAML_URL_CHECK
}

class SFSessionPropertyAttr : Attribute
Expand Down
Loading

0 comments on commit 737d695

Please sign in to comment.