Skip to content

Commit

Permalink
check for http_proxy & https_proxy env-vars
Browse files Browse the repository at this point in the history
this brings us in line with many other AWS SDKs which check for
this environment variable, and properly set it. a couple things
to note about this:

1. I chose to not add in explicit unit tests/integration tests as
the webproxy already has tests. and testing an environment
variable would bleed into other tests unless we mocked it everywhere,
or we serially ran those tests that depended on the state of the env
var. I think the overhead of either of these is not worth what we
get out of it (which is effectively two calls to BCL, one for the
environment variable, one for constructing a web proxy). if you want
me to go through either of these paths, i can.

2. HTTPS proxies were only recently supported in dotnet:
<dotnet/runtime#31113>. I chose to
'try to construct, error if it fails'. This techincally introduces
a case where you get an exception being thrown you wouldn't before.
This _feels_ like the right path "hey your https proxy isn't being
used even though you told us to use it". I can wrap this in a try
catch block though if we decide we just want to silently ignore it
like we have been so far (this may lead to more bugs being filed
about "why doesn't https_proxy work!").
  • Loading branch information
Mythra committed Jul 1, 2023
1 parent 2f0f8cd commit bbfa559
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 1 deletion.
16 changes: 16 additions & 0 deletions sdk/src/Core/Amazon.Runtime/_bcl/ClientConfig.cs
Expand Up @@ -129,6 +129,22 @@ public WebProxy GetWebProxy()
}
proxy.BypassProxyOnLocal = ProxyBypassOnLocal;
}
if (proxy == null)
{
var httpsProxy = Environment.GetEnvironmentVariable("https_proxy");
var httpProxy = Environment.GetEnvironmentVariable("http_proxy");
if (!string.IsNullOrEmpty(httpsProxy))
{
// WebProxy only supports HTTPS as of .NET 8.0 preview 7,
// or greater. The best we can do is just try to create it,
// and let it fail if .NET doesn't support it.
proxy = new WebProxy(httpsProxy);
}
else if (!string.IsNullOrEmpty(httpProxy))
{
proxy = new WebProxy(httpProxy);
}
}

return proxy;
}
Expand Down
21 changes: 20 additions & 1 deletion sdk/src/Core/Amazon.Runtime/_netstandard/ClientConfig.cs
Expand Up @@ -43,7 +43,26 @@ private static RegionEndpoint GetDefaultRegionEndpoint()
/// </summary>
public IWebProxy GetWebProxy()
{
return proxy;
if (this.proxy != null)
{
return this.proxy;
}
var httpsProxy = Environment.GetEnvironmentVariable("https_proxy");
if (!string.IsNullOrEmpty(httpsProxy))
{
// WebProxy only supports HTTPS as of .NET 8.0 preview 7,
// or greater. The best we can do is just try to create it,
// and let it fail if .NET doesn't support it.
this.proxy = new Amazon.Runtime.Internal.Util.WebProxy(httpsProxy);
return this.proxy;
}
var httpProxy = Environment.GetEnvironmentVariable("http_proxy");
if (!string.IsNullOrEmpty(httpProxy))
{
this.proxy = new Amazon.Runtime.Internal.Util.WebProxy(httpProxy);
return this.proxy;
}
return null;
}

/// <summary>
Expand Down

0 comments on commit bbfa559

Please sign in to comment.