-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add support for proxies #190 #4203
Conversation
# Conflicts: # src/AppInstallerCLICore/ExecutionContextData.h # src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw
switch (setting) | ||
{ | ||
case AppInstaller::Settings::StringAdminSetting::DefaultProxy: | ||
return GroupPolicies().GetValueRef<ValuePolicy::DefaultProxy>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just double check, if DefaultProxy group policy is not set. The return will be std::nullopt? Why there's a reference_wrapper needed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, GroupPolicies().GetValue*()
returns std::nullopt
if the policy is not set.
Unless I'm misunderstanding what it does, the reference_wrapper
allows us to return a reference of the value instead of copying it. That is the behavior I would expect if we were able to do std::optional<std::string&>
.
Co-authored-by: yao-msft <[email protected]>
Co-authored-by: yao-msft <[email protected]>
I'm very sorry that my issues and PRs are causing problems for you all. Thanks to microsoft and the winget team for their excellent work! |
For #190
See spec on #4152
See #1776 for a related PR for the feature with the core implementation for proxies in wininet
This PR adds basic support for using proxies. Most of the changes are for enabling the configuration and blocking of the feature. This feature will be gated behind an experimental feature setting
AdminSettings
to support settings with string values, instead of only bool flags. The implementation is mostly a copy of the bool case. In the future we should look back at it to reduce duplication of code.set
subcommand tosettings
that can set the admin settingsManually tested on a VM using mitmproxy
Pending: Adding automated tests tests.
Microsoft Reviewers: Open in CodeFlow