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

Implement global proxy configuration for HttpClient #29147

Closed
davidsh opened this issue Apr 3, 2019 · 32 comments · Fixed by dotnet/corefx#37333
Closed

Implement global proxy configuration for HttpClient #29147

davidsh opened this issue Apr 3, 2019 · 32 comments · Fixed by dotnet/corefx#37333
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Http
Milestone

Comments

@davidsh
Copy link
Contributor

davidsh commented Apr 3, 2019

[edit: this top post has been edited to show the final draft proposal for a new API]

API Proposal

namespace System.Net.Http
{
    public class HttpClient
    {
        ...
        public static IWebProxy DefaultProxy { get; set; } 
        ...
    }
}

Behaviors

The property will default to a non-null value representing the platform/system proxy. With PR 37328, all platforms can use environment variables as the first possible choice of proxy settings. If those variables are set, then an IWebProxy interface of an instance of the internal HttpEnvironmentProxy class is returned.

If the environment variables are not set, then the following happens for the default value of this property:

  • On Windows, the IWebProxy interface of the internal HttpSystemProxy class is returned. HttpSystemProxy wraps the IE/Wininet settings.
  • On OSX, the IWebProxy interface of the internal MacProxy class is returned. MacProxy wraps the OSX system proxy configuration.
  • On Linux, if environment variables are not set, then a non-null instance of an IWebProxy object is returned. It will return 'true' for IsBypassed() for any Uri passed in.

The property will never return null.

The property can be set to any object implementing the IWebProxy interface. This property cannot be set to 'null'. Doing so will throw an exception, ArgumentNullException.

Sample API usage

// I want to pass in default credentials to the system proxy so my any HttpClient request
// will work on my corporate network.
// This includes any HttpClient objects that I might not create as long as they use the
// default settings of HttpClientHandler.
HttpClient.DefaultProxy.Credentials = CredentialCache.DefaultCredentials;

// I want to use a special proxy everywhere I use HttpClient
HttpClient.DefaultProxy = new MyCustomProxy();

History of proxy support in .NET

https://gist.github.com/davidsh/f8768c02faf714de9a3029cbf0166f18

Background

HttpClient has support for reading system proxy configuration (IE settings on Windows, Environment variables on Linux, etc). However, there is no mechanism for setting credentials (i.e. CredentialCache.DefaultCredentials) for a system configured proxy. Currently, this requires creating an HttpClientHandler instance and setting its DefaultProxyCredentials property.

For cases where libraries are creating the HttpClient object, there is no way for consumers of the library to set these credentials easily.

The .NET Framework supports using app.config/web.config settings with a system.net section:

<?xml version="1.0" encoding="utf-8" ?>
<configuration>
  <system.net>
    <defaultProxy useDefaultCredentials="true" />
  </system.net>
</configuration>

This allows applications to inject a global proxy configuration for any HttpClient that is created. However, .NET Core doesn't natively support configuration files.

.NET Framework currently has the WebRequest.DefaultWebProxy property to control this for all HttpWebRequest objects (and affects HttpClient) as well. On .NET Core, the WebRequest class is considered legacy. But we need a similar way to set global proxy config on .NET Core for HttpClient usage.

A static API on HttpClient (or other object) created for global proxy configuration for the application could be used by upstack components to create a similar configuration file (i.e. app.config) behavior.

@davidsh davidsh self-assigned this Apr 3, 2019
@davidsh
Copy link
Contributor Author

davidsh commented Apr 3, 2019

The next step for implementing this will be a new API proposal issue to be opened soon.

@lobster2012-user
Copy link

Don't forget about UseDefaultCredentials .
This parameter is important for the intranet and by default it has value false.

@tebeco
Copy link

tebeco commented Apr 3, 2019

@davish thx a lot for this

in a previous issue a proposed solution was something LIKE this (the issue with this one is that it's aspnetcore specific)

https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.features.ifeaturecollection?view=aspnetcore-2.1

Another idea could be something like this
AppContext.SetSwitch(switchName, state)
but it can be hard to have "logic" in it

Another way (that i find more interresting as a consumer) would be an Api that kinda works like powershell :
Get-Proxy destinationUrl
allowing developper to do a "Last Look" on request and return for example :

  • the url of the proxy to use
  • an httphandler to add to that call

it could be on theHttpClientFactory.HERE(Uri destination) itself but i have no idea if the creation of an HttpClient in a third part nuget already "interact" with HttpClientFactory

the goodness of the last one is that we could inject a default callback using WindowsCompatPack, check if windows, hit the Registry, retrieve proxy settings etc ...
that would allow :

  • old behavior for global settings
  • default credential if we get the hand on the handler
  • dynamism (because we have multiple proxy in LAN)
  • no huge impact if the value are "re-used" instead of "per call"

@karelz
Copy link
Member

karelz commented Apr 3, 2019

@tebeco HttpClientFactory is higher-layer library. It does not make sense to make HttpClient complex and make it depend on it - beside complexity, which is hard to explain, it would bring also layering violation which is often sign of bad design.
We should stay in HttpClient and solve the problem here, then let HttpClientFactory / ASP.NET Core use the lower-layer mechanism here in HttpClient.

Rather than shooting for full setting compatibility with .NET Framework, we should IMO focus on extensibility of the mechanisms in HttpClient, so that users and/or libraries can provide additional logic. Then anyone can write smart Registry-reading / config file-reading functionality ...

@epignosisx
Copy link

I would love to see this happening. We have a corporate proxy that causes a lot of pain for us because of the reasons that @davidsh explained.

What if instead of adding a new API to the already complicated HttpClientHandler, we make the windows implementation aware of the HTTP_PROXY environment variable and friends like the Linux version.

Node.js, Rust, and Go all honor these environment variables in Windows, probably most developers running behind a proxy already have these environment variables defined. Another thing that bit us in the past is that our test environments were inside the company network, however the production environment was in a DMZ with no proxy. We learned the difference only after going to prod and finding out that all our calls were failing. In many companies these environment intricacies are only known to the networking or sysadmin teams. Environment variables allow sysadmins to configure the servers making it transparent to the applications running in them.

@karelz
Copy link
Member

karelz commented Apr 6, 2019

@epignosisx enc vars are on the table AFAIK.
We should add new API anyway, to enable config-style settings by host (e.g. ASP.NET). Someone can then even implement a simple library to read .NET Framework-style config for full compat (it should not be in the platform itself as .NET Core does not have config concept at the moment).

@epignosisx
Copy link

@karelz thanks for the feedback, makes sense. Since adding a new API will have to go through the review process, I’m afraid it will miss the 3.0 train. If that’s the case, could the team work first on honoring the env vars? I’m assuming this will be simpler to implement and can make it for 3.0.

@karelz
Copy link
Member

karelz commented Apr 6, 2019

It is not going to miss 3.0, unless something very unexpected happens, don't worry.
3.0 date is not announced yet - it will be announced at Build conference in May. If you look at number of remaining 3.0 bugs (eg via my tweets on progress), you can do some guesswork as well.

davidsh referenced this issue in davidsh/corefx Apr 24, 2019
SocketsHttpHandler was not using the proxy bypass list specified in IE settings in cases where
there is a combination of both 'AutoDetect' and 'Manual' settings in the IE settings dialog.

SocketsHttpHandler uses WinHttpHandler's WinInetProxyHelper class as part of the HttpSystemProxy
class. But it was consuming it in an incorrect way. WinInetProxyHelper was originally written to
be used only with the rest of the WinHTTP stack for WinHttpHandler. When WinHTTP GetProxyForUrl()
was returning a ProxyBypass string, HttpSystemProxy was ignoring it. It was assuming that the
string for Proxy was correct. But in cases where ProxyBypass is returned, the Proxy string is only
used if the destination uri doesn't match any of the strings in the ProxyBypass list. That logic
would normally be handled automatically by WinHttpHandler. But HttpSystemProxy was simply discarding
the ProxyBypass string returned by WinHTTP GetProxyForUrl().

In order to address this fix, I added new tests for HttpSystemProxy. I utilized the existing mock
layers for registry and WinHTTP interop that are contained in the WinHttpHandler Unit Tests. It might
seem weird to have tests for the internal HttpSystemProxy class located in the WinHttpHandler folder.
But, we already pull in the source code for WinInetProxyHelper from WinHttpHandler into the
SocketsHttpHandler compilation. Using these Unit Tests with the mocking layers allows us to test a
broad range of scenarios (registry settings and network failures) that are normally difficult to test.
I have a plan, though, to refactor the system proxy code and tests as part of implementing #36553.
That will result in the system proxy code and tests ending up in a more logical place.

As part of this fix, I also optimized how SocketsHttpHandler is calling IWebProxy. I explained this
in the comments of HttpSystemProxy.IsBypassed(). In summary, IWebProxy.IsBypassed() shouldn't be used.
In most cases it is the same amount of work than calling IWebProxy.GetProxy(). And the latter can
be used to return a valid proxy uri, or null if a proxy shouldn't be used for that particular
destination uri.

Fixes #33866
davidsh referenced this issue in davidsh/corefx Apr 24, 2019
SocketsHttpHandler was not using the proxy bypass list specified in IE settings in cases where
there is a combination of both 'AutoDetect' and 'Manual' settings in the IE settings dialog.

SocketsHttpHandler uses WinHttpHandler's WinInetProxyHelper class as part of the HttpSystemProxy
class. But it was consuming it in an incorrect way. WinInetProxyHelper was originally written to
be used only with the rest of the WinHTTP stack for WinHttpHandler. When WinHTTP GetProxyForUrl()
was returning a ProxyBypass string, HttpSystemProxy was ignoring it. It was assuming that the
string for Proxy was correct. But in cases where ProxyBypass is returned, the Proxy string is only
used if the destination uri doesn't match any of the strings in the ProxyBypass list. That logic
would normally be handled automatically by WinHttpHandler. But HttpSystemProxy was simply
discarding the ProxyBypass string returned by WinHTTP GetProxyForUrl().

In order to address this fix, I added new tests for HttpSystemProxy. I utilized the existing mock
layers for registry and WinHTTP interop that are contained in the WinHttpHandler Unit Tests. It might
seem weird to have tests for the internal HttpSystemProxy class located in the WinHttpHandler folder.
But, we already pull in the source code for WinInetProxyHelper from WinHttpHandler into the
SocketsHttpHandler compilation. Using these Unit Tests with the mocking layers allows us to test a
broad range of scenarios (registry settings and network failures) that are normally difficult to test.
I have a plan, though, to refactor the system proxy code and tests as part of implementing #36553.
That will result in the system proxy code and tests ending up in a more logical place.

As part of this fix, I also optimized how SocketsHttpHandler is calling IWebProxy. I explained this
in the comments of HttpSystemProxy.IsBypassed(). In summary, IWebProxy.IsBypassed() shouldn't be
used. In most cases it is the same amount of work than calling IWebProxy.GetProxy(). And the latter
can be used to return a valid proxy uri, or null if a proxy shouldn't be used for that particular
destination uri.

Fixes #33866
davidsh referenced this issue in davidsh/corefx Apr 24, 2019
SocketsHttpHandler was not using the proxy bypass list specified in IE settings in cases where
there is a combination of both 'AutoDetect' and 'Manual' settings in the IE settings dialog.

SocketsHttpHandler uses WinHttpHandler's WinInetProxyHelper class as part of the HttpSystemProxy
class. But it was consuming it in an incorrect way. WinInetProxyHelper was originally written to
be used only with the rest of the WinHTTP stack for WinHttpHandler. When WinHTTP GetProxyForUrl()
was returning a ProxyBypass string, HttpSystemProxy was ignoring it. It was assuming that the
string for Proxy was correct. But in cases where ProxyBypass is returned, the Proxy string is only
used if the destination uri doesn't match any of the strings in the ProxyBypass list. That logic
would normally be handled automatically by WinHttpHandler. But HttpSystemProxy was simply
discarding the ProxyBypass string returned by WinHTTP GetProxyForUrl().

In order to address this fix, I added new tests for HttpSystemProxy. I utilized the existing mock
layers for registry and WinHTTP interop that are contained in the WinHttpHandler Unit Tests. It might
seem weird to have tests for the internal HttpSystemProxy class located in the WinHttpHandler folder.
But, we already pull in the source code for WinInetProxyHelper from WinHttpHandler into the
SocketsHttpHandler compilation. Using these Unit Tests with the mocking layers allows us to test a
broad range of scenarios (registry settings and network failures) that are normally difficult to test.
I have a plan, though, to refactor the system proxy code and tests as part of implementing #36553.
That will result in the system proxy code and tests ending up in a more logical place.

As part of this fix, I also optimized how SocketsHttpHandler is calling IWebProxy. I explained this
in the comments of HttpSystemProxy.IsBypassed(). In summary, IWebProxy.IsBypassed() shouldn't be
used. In most cases it is the same amount of work than calling IWebProxy.GetProxy(). And the latter
can be used to return a valid proxy uri, or null if a proxy shouldn't be used for that particular
destination uri.

Fixes #33866
davidsh referenced this issue in davidsh/corefx Apr 25, 2019
SocketsHttpHandler was not using the proxy bypass list specified in IE settings in cases where
there is a combination of both 'AutoDetect' and 'Manual' settings in the IE settings dialog.

SocketsHttpHandler uses WinHttpHandler's WinInetProxyHelper class as part of the HttpSystemProxy
class. But it was consuming it in an incorrect way. WinInetProxyHelper was originally written to
be used only with the rest of the WinHTTP stack for WinHttpHandler. When WinHTTP GetProxyForUrl()
was returning a ProxyBypass string, HttpSystemProxy was ignoring it. It was assuming that the
string for Proxy was correct. But in cases where ProxyBypass is returned, the Proxy string is only
used if the destination uri doesn't match any of the strings in the ProxyBypass list. That logic
would normally be handled automatically by WinHttpHandler. But HttpSystemProxy was simply
discarding the ProxyBypass string returned by WinHTTP GetProxyForUrl().

In order to address this fix, I added new tests for HttpSystemProxy. I utilized the existing mock
layers for registry and WinHTTP interop that are contained in the WinHttpHandler Unit Tests. It might
seem weird to have tests for the internal HttpSystemProxy class located in the WinHttpHandler folder.
But, we already pull in the source code for WinInetProxyHelper from WinHttpHandler into the
SocketsHttpHandler compilation. Using these Unit Tests with the mocking layers allows us to test a
broad range of scenarios (registry settings and network failures) that are normally difficult to test.
I have a plan, though, to refactor the system proxy code and tests as part of implementing #36553.
That will result in the system proxy code and tests ending up in a more logical place.

Fixes #33866
davidsh referenced this issue in dotnet/corefx Apr 25, 2019
SocketsHttpHandler was not using the proxy bypass list specified in IE settings in cases where
there is a combination of both 'AutoDetect' and 'Manual' settings in the IE settings dialog.

SocketsHttpHandler uses WinHttpHandler's WinInetProxyHelper class as part of the HttpSystemProxy
class. But it was consuming it in an incorrect way. WinInetProxyHelper was originally written to
be used only with the rest of the WinHTTP stack for WinHttpHandler. When WinHTTP GetProxyForUrl() was returning a ProxyBypass string, HttpSystemProxy was ignoring it. It was
assuming that the string for Proxy was correct. But in cases where ProxyBypass is returned, the
Proxy string is only used if the destination uri doesn't match any of the strings in the ProxyBypass
list. That logic would normally be handled automatically by WinHttpHandler. But HttpSystemProxy
was simply discarding the ProxyBypass string returned by WinHTTP GetProxyForUrl().

In order to address this fix, I added new tests for HttpSystemProxy. I utilized the existing mock
layers for registry and WinHTTP interop that are contained in the WinHttpHandler Unit Tests. It might
seem weird to have tests for the internal HttpSystemProxy class located in the WinHttpHandler folder.
But, we already pull in the source code for WinInetProxyHelper from WinHttpHandler into the
SocketsHttpHandler compilation. Using these Unit Tests with the mocking layers allows us to test a
broad range of scenarios (registry settings and network failures) that are normally difficult to test.
I have a plan, though, to refactor the system proxy code and tests as part of implementing #36553.
That will result in the system proxy code and tests ending up in a more logical place.

Fixes #33866
@davidsh
Copy link
Contributor Author

davidsh commented Apr 28, 2019

@epignosisx

What if instead of adding a new API to the already complicated HttpClientHandler, we make the windows implementation aware of the HTTP_PROXY environment variable and friends like the Linux version.

I just implemented environment variables support for Windows to match Linux/OSX with PR dotnet/corefx#37238.

We still will be adding a new API.

@davidsh
Copy link
Contributor Author

davidsh commented Apr 28, 2019

cc: @karelz @stephentoub @dotnet/ncl

I have updated the top post of this issue and will be using it as the API proposal issue.

@tebeco
Copy link

tebeco commented Apr 28, 2019

if you got a nightly or something you want me to test at work (except between 04/05 and 11/05), i will gladly test it
What is failing so far for us is :

  • Azure SignalR (websocket pool i guess)
  • Any httpclient to public dns
  • CosmosDb (using DocumentDb / because DocumentDb does not expose(s/d) handler
  • Azure KeyVault
  • almost everything in Azure Sdk that does http call, if has not been fixed in fact (as we develop from the LAN we'd like to fetch / connect to our dedicated environments from local code)
  • Vsts on premise when pushing artifact
  • Vsts on premise when downloading artifact

davidsh referenced this issue in davidsh/corefx Apr 30, 2019
In order to provide a consistent experience with the HttpClient IWebProxy objects,
the returned internal proxy object which represent the system/platform proxy settings
should never be null. If the platform's settings indicate that no proxy is being used,
then return an instance of the internal HttpNoProxy object.

Note that even if the platform settings indicate that a proxy could be used, any
particular Http request might still not go thru a proxy. The final determination of
what proxy is being used for a request is still governed by the return of the
IWebProxy.IsBypassed and IWebProxy.GetProxy methods.

Contributes to #36553
davidsh referenced this issue in dotnet/corefx May 1, 2019
In order to provide a consistent experience with the HttpClient IWebProxy objects,
the returned internal proxy object which represent the system/platform proxy settings
should never be null. If the platform's settings indicate that no proxy is being used,
then return an instance of the internal HttpNoProxy object.

Note that even if the platform settings indicate that a proxy could be used, any
particular Http request might still not go thru a proxy. The final determination of
what proxy is being used for a request is still governed by the return of the
IWebProxy.IsBypassed and IWebProxy.GetProxy methods.

Contributes to #36553
davidsh referenced this issue in davidsh/corefx May 1, 2019
This PR implements the new static HttpClient.DefaultProxy property which was approved
during API review.

Modify the SystemProxyInfo.ConstructSystemProxy method to a Singleton.

Modify SocketsHttpHandler to use the HttpClient.DefaultProxy property.

Rename the HttpSystemProxy class to HttpWindowsProxy.

Add some HttpClient tests for the new property.

Closes #36553
davidsh referenced this issue in dotnet/corefx May 1, 2019
This PR implements the new static HttpClient.DefaultProxy property which was approved
during API review.

Modify the SystemProxyInfo.ConstructSystemProxy method to a Singleton.

Modify SocketsHttpHandler to use the HttpClient.DefaultProxy property.

Rename the HttpSystemProxy class to HttpWindowsProxy.

Add some HttpClient tests for the new property.

Closes #36553
@tebeco
Copy link

tebeco commented May 2, 2019

@davidsh
how can we make sure all Azure Sdk will be tested against this for 3.0 ?

SignalR creates websocket in order to connect to Azure SignalR
as this change focus on http_proxy for httpclient handler i'm not sure websocket will benefits from it

also as CosmosDb/KeyVault and other Api manually creates their own httpclient manually with custom handler
It's would be lovely to have a way to make sure before 3.0 land that all scenario from azure sdk are covered ;)

@davidsh
Copy link
Contributor Author

davidsh commented May 2, 2019

SignalR creates websocket in order to connect to Azure SignalR
as this change focus on http_proxy for httpclient handler i'm not sure websocket will benefits from it

If they are creating a .NET Core websocket via System.Net.WebSockets.Client, then they will benefit from it. The implementation of .NET Core websocket uses the common HTTP stack on .NET Core (which uses SocketsHttpHandler) to start the websocket. SocketsHttpHandler will query HttpClient.DefaultProxy.

also as CosmosDb/KeyVault and other Api manually creates their own httpclient manually with custom handler
It's would be lovely to have a way to make sure before 3.0 land that all scenario from azure sdk are covered

If those components are using HttpClient, then they will benefit from the API indirectly. But if they have custom proxy requirements, they should make sure to set that property (or HttpClient.DefaultProxy.Credentials) appropriately.

If you think there are new issues to be investigated here, please open a new issue for that in the appropriate repo.

@davidsh
Copy link
Contributor Author

davidsh commented May 2, 2019

@karelz

@tebeco
Copy link

tebeco commented May 2, 2019

The main issue for us now is to be able to benefits from "outside" resource from :

  • LAN (production)
  • Dmz (production)
  • LAN (on premise / DevInt / Uat...)
  • Developper computer

the nightmare for us today is that, in order to simply press F5, we have to:

  • run local a fake proxy that authenticate request in our name (CNTLM / Px ....)
  • run fiddler
  • start the app (dotnet run)

these "outside" resources are mainly :

  • azure signalr (we plan to push stream millions of msg per day if it sustains the load)
  • KeyVault (tons of apps configuration)
  • CosmosDb (distributed cache and other stuff)

but for now the use of a such "hack" does not go well for developper so it's kinda hard to move on azure if we can't just

git clone
dotnet restore
dotnet run

it's probably time to review Azure Sdk public Api to allow developper to inject httpClient handler / expose it / etc ...
and make sure it benefits from all that goodness here ^^

but again i have some issue with just http_proxy, i did not dive in the code but can someone confirm that no_proxy is already working ?

if this is not the case i would love this issue to be re-open as "on premise" apps need to connect to both LAN (internal resource) or Azure for example

that's why a "callback" that could be registered like

static void Main()
{
    AppContext.SetSwitch("ProxySwitcher.Http", ResolveProxy)

//could add SOCKS / WebSocket ...
}

public IWebProxy ResolveProxy(Uri uri)
{
  //MY CODE / MY CHOICE
}

it could be a "type" with an interface to implements instead of a straight callback

this would be way more flexible from consumer PoV
(this is what was used to OPT_OUT SocketHttpHandler when Challenger was broken in 2.0 i think)

@davidsh
Copy link
Contributor Author

davidsh commented May 2, 2019

but again i have some issue with just http_proxy, i did not dive in the code but can someone confirm that no_proxy is already working ?

All environment variables are now working across all platforms. See: dotnet/corefx#37200, dotnet/corefx#37238

@tebeco
Copy link

tebeco commented May 2, 2019

@davidsh

i edited the previous comment, do you want me to edit and put the rest here for clarity for further readers ?

@tebeco
Copy link

tebeco commented May 2, 2019

All environment variables are now working across all platforms. See: dotnet/corefx#37200, dotnet/corefx#37238

i feel bad i did not even read the title of the PR ;) (jumped too fast to part of the code)
my bad

@davidsh
Copy link
Contributor Author

davidsh commented May 2, 2019

i edited the previous comment, do you want me to edit and put the rest here for clarity for further readers ?

Not sure I understand. But if you feel that there is some new issue not addressed here regarding configuration with either .NET Core or other components, please open up a new issue. For things outside of .NET Core, it would be best to open up the issue in those repo's. Thx.

@tebeco
Copy link

tebeco commented May 3, 2019

Yes you are right

is it technically possible to get the nightly of 3.0 running against current azure sdk ?
or does the sdk needs to be first built against the nightly then used here for test ?

@Suchiman
Copy link
Contributor

Suchiman commented May 4, 2019

@davidsh When you wrote https://github.com/dotnet/corefx/issues/34466#issuecomment-452845198 i kinda expected a completely new API. Given that the new design still sticks with IWebProxy, what gave adding a new property on HttpClient that isn't exclusively used for HttpClient the edge over just fixing WebRequest.DefaultProxy? Given that .NET Core's HttpClient is not respecting WebRequest.DefaultProxy, this is a breaking change from framework. It also invalidates all guidelines so far and is also not available in .NETStandard 2.0. Furthermore, we now have two places where we can set a DefaultProxy, what is the effect of setting WebRequest.DefaultProxy past this change? All in all this just looks like a deviation from the pit of success for no obvious good reason.

@karelz
Copy link
Member

karelz commented May 17, 2019

@Suchiman one of the key reasons is that WebRequest is obsolete API which we do not fully support on .NET Core, just in compat way. Reusing parts of it would be fairly confusing.
There are many new features which are not available in .NET Standard 2.0. The whole SocketsHttpHandler is not available there. The global proxy problem is Core specific.
I believe that WebRequest.DefaultProxy is no-op on .NET Core. Given that it is obsolete API, we recommend to not use it at all, except binary compat reason, and then be aware it does not do what you think it should ...

@Suchiman
Copy link
Contributor

Suchiman commented May 17, 2019

@karelz WebRequest.DefaultWebProxy isn't a no-op, see https://github.com/dotnet/corefx/issues/34466#issue-397415281 (the comment was written after digging through the source to figure a way to make it work, although i have not evaluated if it is still valid after this change, which would be another breaking change if it weren't).

Now you can set two different "global" proxies.

The SocketsHttpHandler argument isn't valid IMO, since it's an implementation detail (you get it automatically by just doing new HttpClient()).

except binary compat reason, and then be aware it does not do what you think it should ...

which is bad and in on itself not a reason to break it because you can.

Maybe it makes sense to forward WebRequest.DefaultWebProxy to HttpClient.DefaultProxy?

@karelz
Copy link
Member

karelz commented May 17, 2019

WebRequest.DefaultWebProxy isn't a no-op

Good point. I didn't know that. Either way, it is scoped to the obsoleted APIs HttpWebRequest/*WebRequest/WebClient.

I don't think we should encourage anyone to use WebRequest.DefaultWebProxy API. It is obsolete and we should leave it as such.

@Suchiman
Copy link
Contributor

While i believe it's redundant to have two properties doing the same thing, just in different circumstances, i can get behind the idea of trying to hide WebRequest and familiar classes to newcomers. But what about replacing WebRequest.DefaultWebProxy by

public static IWebProxy DefaultWebProxy
{
    get
    {
        return HttpClient.DefaultProxy;
    }
    set
    {
        HttpClient.DefaultProxy = value;
    }
}

similiar to how it's done for many things on the obsolete AppDomain API.

@karelz
Copy link
Member

karelz commented May 17, 2019

That would just encourage more people to use it and it would confuse more users. I don't think it is a good idea.

@tebeco
Copy link

tebeco commented May 17, 2019

time to remove WebRequest as part of 3.0 and other Obsolete API ;)

That will fix any potential confusion

@karelz
Copy link
Member

karelz commented May 17, 2019

We don't remove APIs. Especially when they are useful for compat and migration from Framework.

@Suchiman
Copy link
Contributor

While i'm not a fan of having users find out the hard way that this API is intentionally broken in certain ways, i'm out of arguments ¯_(ツ)_/¯

@davidsh
Copy link
Contributor Author

davidsh commented May 27, 2019

@tebeco

Yes you are right
is it technically possible to get the nightly of 3.0 running against current azure sdk ?
or does the sdk needs to be first built against the nightly then used here for test ?

I don't think anyone answered your specific question here. I'm not sure what you mean by "current azure sdk". But if you want to try out .NET Core 3.0, you can download the latest preview release.

https://devblogs.microsoft.com/dotnet/announcing-net-core-3-0-preview-5/

@tebeco
Copy link

tebeco commented May 29, 2019

@davidsh @karelz

Hello again,
I just tried to moved a solution from sdk 2.2.204 to sdk 3.0.100-preview5-011568.
I did not changed any source and i got this :

> rm global.json
> dotnet new globaljson
> dotnet restore --force --no-cache
C:\Program Files\dotnet\sdk\3.0.100-preview5-011568\NuGet.targets(121,5): error : Unable to load the service index for source https://dotnetfeed.blob.core.windows.net/aspnet-aspnetcore-tooling/index.json. [D:\sources\path\to\solution.sln]
C:\Program Files\dotnet\sdk\3.0.100-preview5-011568\NuGet.targets(121,5): error :   Response status code does not indicate success: 407 (authenticationrequired). [D:\sources\path\to\solution.sln]

so i tried :

> SET HTTP_PROXY=http://user:name@corpProxy:port
> SET HTTPS_PROXY=http://user:name@corpProxy:port
> SET NO_PROXY=*.corp
> dotnet restore --force --no-cache
C:\Program Files\dotnet\sdk\3.0.100-preview5-011568\NuGet.targets(121,5): error : Unable to load the service index for source https://dotnetfeed.blob.core.windows.net/aspnet-aspnetcore-tooling/index.json. [D:\sources\path\to\solution.sln]
C:\Program Files\dotnet\sdk\3.0.100-preview5-011568\NuGet.targets(121,5): error :   Response status code does not indicate success: 407 (authenticationrequired). [D:\sources\path\to\solution.sln]

Is this PR included in this SDK ?
Also, where does that source comming from -_-
i even added a nuget.config with a clear on all sources :

<?xml version="1.0" encoding="utf-8"?>
<configuration>
 <packageSources>
    <!--To inherit the global NuGet package sources remove the <clear/> line below -->
    <clear />
 </packageSources>
</configuration>

Final touch, we use a Directory.Build.props to setup the RestoreSources :
The idea is that we only use URL that are on-premise (no-auth / no proxy), and one of the feed is a proxy to nuget.org itself.
It's still hitting https://dotnetfeed.blob.core.windows.net/ and crashing. So far this "Global proxy" is hurting us so bad that even dotnet it self is broken on restore ;)

<Project>
  <PropertyGroup>
    <RestoreSources>
      https://INTERNAL-URL-FOR-ARTIFACTORY-ON-PREMISE/api/nuget/v3/nuget-proxy/;
      https://INTERNAL-URL-FOR-ARTIFACTORY-ON-PREMISE/api/nuget/v3/OUR-TEAM/;
    </RestoreSources>
  </PropertyGroup>
</Project>

EDIT:
Just installed sdk 3.0.100-preview6-012103 (with the MSI), the dotnet restore works without HTTP_PROXY (with the same sources.props content)
I have no idea of what's in preview5 that cause this issue

@davidsh
Copy link
Contributor Author

davidsh commented May 29, 2019

@tebeco It's not efficient to comment on this closed issue. This issue was closed since it reflected the new HttpClient.DefaultProxy API that was added, dotnet/corefx#37333. But your scenario is trying to use environment variables to set global proxy. It's not using the new HttpClient.DefaultProxy API (which would have to be used with dotnet restore itself).

We did add support for using environment variables to set global proxy information, dotnet/corefx#37200. If there are problems with that working for your scenario, please open a new issue. We can then investigate it. Please also describe how your environment is set up in terms of authenticated proxy and what authentication schemes are being used. Thanks!

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 3.0 milestone Feb 1, 2020
@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Http
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants