Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Read system proxy information on macOS #36177

Merged
merged 5 commits into from
Apr 13, 2019
Merged

Conversation

filipnavara
Copy link
Member

Partially fixes #26593

Tested with HTTP, HTTPS and Autoconfiguration settings. Doesn't pick-up the stored credentials yet.

@davidsh davidsh requested a review from wfurt March 20, 2019 14:16
@davidsh davidsh added this to the 3.0 milestone Mar 20, 2019
@davidsh davidsh requested review from a team and stephentoub March 20, 2019 14:22
@filipnavara filipnavara changed the title WIP: Read system proxy information on macOS Read system proxy information on macOS Mar 21, 2019
@davidsh
Copy link
Contributor

davidsh commented Mar 22, 2019

Will the current environment variables (http_proxy, etc.) still work? Will they override the "system" Mac OS settings?

@filipnavara
Copy link
Member Author

Will the current environment variables (http_proxy, etc.) still work? Will they override the "system" Mac OS settings?

No. Is that something that is desirable?

@davidsh
Copy link
Contributor

davidsh commented Mar 22, 2019

No. Is that something that is desirable?

As part of my work on Enterprise scenarios I am designing the global proxy solution for HttpClient et. al. We also have requirements for environment variables to work on Windows similar to Linux for various docker scenarios.

So, the idea was that we get the standard set of Linux compatible environment variables for proxy settings to work on Windows. Thus, it would be consistent to keep them working on the Mac as well.

The idea is that the environment variables, if set, would override system configured settings such as IE settings on Windows and Mac OS settings as well.

We already have a class HttpEnvironmentProxy.cs to handle that. So, I would prefer we keep that working as well. But in the absence of those variables defined, then the new Mac OS system proxy config support would be used.

@karelz @geoffkizer

@filipnavara
Copy link
Member Author

It's not hard to make it working, but since the environment variables were not used on Windows as fallback I simply followed that. I could easily instantiate HttpEnvironmentProxy instead of MacProxy if the environment variables are present.

@davidsh
Copy link
Contributor

davidsh commented Apr 3, 2019

@filipnavara Is this PR ready for a final review? Have all comments been addressed? Not sure since some of them don't have responses from you.

@filipnavara
Copy link
Member Author

@davidsh It is ready for review. The implementation of reading proxy credentials is still missing, but I don't think it should block the PR. My main use case is debugging proxies (Charles, Fiddler) where this is not necessary.

@davidsh
Copy link
Contributor

davidsh commented Apr 3, 2019

@wfurt @stephentoub Can you do another review pass on this?

Copy link
Contributor

@davidsh davidsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the PR!

@davidsh davidsh merged commit 2aa2518 into dotnet:master Apr 13, 2019
@jonassteinberg1
Copy link

did the implementation for reading linux-convention proxy environment variables ever get completed? I am facing this issue running core3.1 on CentOS 7 and it works for every other language I test except for core3.1. I'm kind of stumped.

@stephentoub
Copy link
Member

did the implementation for reading linux-convention proxy environment variables ever get completed?

Yup. If you're having issues with it, can you please open a new issue with details and a repro? Thanks!

@jonassteinberg1
Copy link

@stephentoub absolutely.

@filipnavara filipnavara deleted the macproxy branch May 12, 2020 17:22
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
* Read system proxy information on macOS

* Replace CFRunLoopRemoveSource with CFRunLoopSourceInvalidate to mimic recommended flow from Chromium / WebKit


Commit migrated from dotnet/corefx@2aa2518
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use system proxy configuration on macOS
6 participants