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

WebRequest.DefaultWebProxy: Set doesn't work without a previous Get #22720

Closed
wants to merge 1 commit into from

Conversation

arunjvs
Copy link
Contributor

@arunjvs arunjvs commented Jul 28, 2017

Unless we do an unnecessary Get() on WebRequest.DefaultWebProxy, any Set() done will not be honored.

This is especially a problem on Linux, where DefaultWebProxy throws PlatformNotSupportedException(). This in turn breaks libraries like WindowsAzure.Storage which fail unless the Get() is called first and then WebRequest.DefaultWebProxy is set to null.

This fix avoids the unnecessary Get() by setting the already initialized flag sent to the LazyInitializer.

@davidsh

Unless we do an unnecessary Get() on WebRequest.DefaultWebProxy, any Set() done will not be honored.

This is especially a problem on Linux, where DefaultWebProxy throws PlatformNotSupportedException(). This in turn breaks libraries like WindowsAzure.Storage which fail unless the Get() is called first and then WebRequest.DefaultWebProxy is set to null.

This fix avoids the unnecessary Get() by setting the already initialized flag sent to the LazyInitializer.
@stephentoub
Copy link
Member

The fix LGTM. Could you add a test for this as well?

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.

Please add a test for this. We want to see that the test fails before your change and passes after your change.

@karelz
Copy link
Member

karelz commented Jul 29, 2017

Thanks for your contribution @arunjvs! I've sent you collaborator invite. Pro-tip: Disable all CoreFX repo notifications, otherwise you will be DoS'd by all of them ;)

If you need more info about contributing to CoreFX repo, check out our docs (also linked from our main page) https://github.com/dotnet/corefx/wiki/New-contributor-Docs (feel free to update them as well - they are RW for everyone), or just ask ... we will be happy to answer. Thanks!

@arunjvs arunjvs closed this Jul 31, 2017
@arunjvs arunjvs deleted the patch-1 branch July 31, 2017 14:33
@stephentoub
Copy link
Member

@arunjvs, did you mean to close this?

@arunjvs
Copy link
Contributor Author

arunjvs commented Jul 31, 2017

Sorry folks. Lost track of this in the weekend. No intention to close :)
Please look at the attached iteration 2 with test case.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants