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

Use system proxy configuration on macOS #24799

Open
wfurt opened this issue Jan 25, 2018 · 17 comments · Fixed by dotnet/corefx#36177
Open

Use system proxy configuration on macOS #24799

wfurt opened this issue Jan 25, 2018 · 17 comments · Fixed by dotnet/corefx#36177
Labels
area-System.Net.Http enhancement Product code improvement that does NOT require public API changes/additions help wanted [up-for-grabs] Good issue for external contributors os-mac-os-x macOS aka OSX
Milestone

Comments

@wfurt
Copy link
Member

wfurt commented Jan 25, 2018

MacOS has system proxy configuration similar to Windows. To have better integration with OS it may be handy to use this instead of relying on environment variables.

proxy

One different caveat is that the proxy is specific to interfaces we we would need to find active interface first

#networksetup -listnetworkserviceorder
#networksetup -getwebproxy Wi-FI
Enabled: No
Server:
Port: 0
Authenticated Proxy Enabled: 0 
@avanderhoorn
Copy link
Contributor

We are looking at having to implement this for our app. Would be nice if this was part of corefx.

@Priya91
Copy link
Contributor

Priya91 commented Mar 14, 2018

@wfurt Does curlhandler read the system proxy settings on osx?

@wfurt
Copy link
Member Author

wfurt commented Mar 14, 2018

no, it does not. That is what I marked it as enhancement.

@borrrden
Copy link

I have some of this work done in my own library. The only further steps needed are to react to the auto proxy url. The work will be on this branch until it is merged. I tried to navigate through the proxies once in corefx, but I got lost on the way through the maze of indirection. If someone wants to point me at a file and say "it should go here" then I can see about making a PR. From what I understand, the API for getting the system proxy is not decided yet.

@karelz karelz changed the title [ManagedHandler] use system proxy configuration on macOS Use system proxy configuration on macOS May 16, 2018
@Priya91
Copy link
Contributor

Priya91 commented Dec 3, 2018

Mono already has an implementation, and I've used this implementation to test locally that it works for all proxy scenarios:

  1. Reading form pac file
  2. Reading from custom config file
  3. Reading from OS settings

https://github.com/mono/mono/blob/master/mcs/class/System/System.Net/MacProxy.cs

Porting it is easy as well..

@filipnavara
Copy link
Member

I was looking into it and loosely porting the Mono implementation to fit CoreFX. There are two possible approaches to take:

  1. Create native library System.Net.Http.Native.Apple and export 2-3 methods from it and keep the manged code rather thin wrapper around it.
  2. Implement it all in managed code.

The problem of the managed code is that it gets rather ugly due to all the Interop wrappers. There's no direct way to load fields from native libraries, so it has to do dlopen/dlsym. Additionally, it is prone to break due to macOS changes. That has already happened when some strings were compared by reference equality, but later versions of macOS started to require proper string comparison.

Opinions?

@stephentoub
Copy link
Member

@filipnavara, thanks for looking into it. If it's feasible, my preference would be (2), or if the shim needs to be used, having the shim be 1:1 as best as possible with the underlying macOS APIs.

Additionally, it is prone to break due to macOS changes.

Can you elaborate on this? What changes would cause a correctly implemented managed implementation to break but a corresponding native one to not break?

@filipnavara
Copy link
Member

Can you elaborate on this? What changes would cause a correctly implemented managed implementation to break but a corresponding native one to not break?

My biggest concern are exported CFString string "constants" which are used here a lot (as dictionary keys). One has to dlopen/dlsym to get to them and that only gets you an indirect pointer that has to be read using Marshal/Unsafe API. Once you get this far you get SafeCFStringHandle from the native library. It is tempting to compare the handle values itself instead of the strings represented by them. The Mono code used to do that and it broke with some macOS version at some point because the native code started returning different handle (ie. instead of the predefined constant you got a handle for a string with the same value).

The above problem is not a big one if you are aware of it, but since there's no DllImport for variables it gets the code quite big and error prone. In the native code a missing or changed constant will break a build. In managed code it will result in null value or broken runtime Assert.

I will share prototype with the managed only version once I get to office on Monday. It will be easier to explain on actual code :)

@wfurt
Copy link
Member Author

wfurt commented Mar 17, 2019

Note that CFString and other Apple types are already exposed in Interop layer. One could probably used the Managed surface without creating System.Net.Http.Native.Apple.
But I don't know what exact API do we need.

@filipnavara
Copy link
Member

Note that CFString and other Apple types are already exposed in Interop layer.

I am well aware of that and reused them in my prototype.

@filipnavara
Copy link
Member

filipnavara commented Mar 18, 2019

Here's my branch: dotnet/corefx@master...filipnavara:macproxy

It only handles basic proxy settings, no auto-configuration and no credentials. It shows most of the interop code that has to be added to make it managed-only.

The original Mono implementation uses a system for passing the system proxy credentials that is incompatible with SocketsHttpHandler. I'm not sure how to handle it properly without doing some invasive changes. I only know the credentials after a call to IWebProxy.GetProxy(uri), which the handler calls after checking IWebProxy.Credentials. In fact the handler works on the assumption that there will be only one ICredentials object for the lifetime of IWebProxy. I'd probably have to implement my own ICredentials implementation that would dynamically check the system settings to make it work.

This is just enough to make HTTP debugging tools like Fiddler/Charles working.

@wfurt
Copy link
Member Author

wfurt commented Mar 18, 2019

cc: @davidsh

@davidsh
Copy link
Contributor

davidsh commented Apr 13, 2019

There is still a TODO in the code to handle reading system proxy credentials for the Mac.

@wfurt
Copy link
Member Author

wfurt commented Apr 14, 2019

Maybe @filipnavara would know, but I did not even figure out how to put in password for proxy on my mac.

@filipnavara
Copy link
Member

Screenshot 2019-04-15 at 08 29 31

@karelz
Copy link
Member

karelz commented Oct 10, 2019

Triage: Last left piece is passwords.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
Copy link
Contributor

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

@dotnet-policy-service dotnet-policy-service bot added backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity labels Apr 10, 2024
@filipnavara filipnavara removed no-recent-activity backlog-cleanup-candidate An inactive issue that has been marked for automated closure. labels Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net.Http enhancement Product code improvement that does NOT require public API changes/additions help wanted [up-for-grabs] Good issue for external contributors os-mac-os-x macOS aka OSX
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants