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

[release/2.1] Fix HttpWebRequest when using system proxy settings #31123

Merged
merged 1 commit into from
Aug 9, 2018
Merged

[release/2.1] Fix HttpWebRequest when using system proxy settings #31123

merged 1 commit into from
Aug 9, 2018

Conversation

davidsh
Copy link
Contributor

@davidsh davidsh commented Jul 17, 2018

While investigating other HttpClient/HttpWebRequest proxy-related bugs, I discovered that
HttpWebRequest was not honoring system proxy settings as defined on Windows with IE
settings or on Linux using environment variables.

The problem is due to how HttpClient and HttpWebRequest differ in how they represent
the default behavior of using system proxy settings with the various properties. Fixed
HttpWebRequest so that it will translate the system proxy settings to the internal
HttpClient/HttpClientHandler objects.

I also removed an invalid Assert in HttpConnection. This assert was firing when using a proxy
that was defined on the loopback adapter using IPv6 literal "[::1]". Due to issue #28863 with Uri,
the Uri.IdnHost property doesn't have the brackets for IPv6 literals. So, the Assert was
occuring.

I did not add any new CI tests because it is currently not possible to test system proxy settings
in CI since it involves changing machine configuration. But I ran manual tests.

Fixes #31122

While investigating other HttpClient/HttpWebRequest proxy-related bugs, I discovered that
HttpWebRequest was not honoring system proxy settings as defined on Windows with IE
settings or on Linux using environment variables.

The problem is due to how HttpClient and HttpWebRequest differ in how they represent
the default behavior of using system proxy settings with the various properties. Fixed
HttpWebRequest so that it will translate the system proxy settings to the internal
HttpClient/HttpClientHandler objects.

I also removed an invalid Assert in HttpConnection. This assert was firing when using a proxy
that was defined on the loopback adapter using IPv6 literal "[::1]".  Due to issue #28863 with Uri,
the Uri.IdnHost property doesn't have the brackets for IPv6 literals. So, the Assert was
occuring.

I did not add any new CI tests because it is currently not possible to test system proxy settings
in CI since it involves changing machine configuration. But I ran manual tests.
@davidsh davidsh added this to the 2.1.x milestone Jul 17, 2018
@davidsh davidsh self-assigned this Jul 17, 2018
@davidsh davidsh changed the title [release/2.1] Fix HttpWebRequest when using system proxy settings (#31100) [release/2.1] Fix HttpWebRequest when using system proxy settings Jul 17, 2018
@davidsh
Copy link
Contributor Author

davidsh commented Jul 17, 2018

@dotnet/dnceng

The branch seems broken for building. Example: https://ci3.dot.net/job/dotnet_corefx/job/release_2.1/job/windows-TGroup_netfx+CGroup_Release+AGroup_x86+TestOuter_false_prtest/279/console

09:02:17 D:\j\workspace\windows-TGrou---bba2808d\Tools\Packaging.targets(1242,5): error : Error when creating nuget packed package from D:\j\workspace\windows-TGrou---bba2808d\bin\packages\Debug\specs\runtime.win-x86.Microsoft.Private.CoreFx.NETCoreApp.nuspec. System.IO.DirectoryNotFoundException: Could not find a part of the path 'D:\j\workspace\windows-TGrou---bba2808d\bin\Windows_NT.x86.Debug\native'. [D:\j\workspace\windows-TGrou---bba2808d\pkg\Microsoft.Private.CoreFx.NETCoreApp\Microsoft.Private.CoreFx.NETCoreApp.pkgproj]
09:02:17 D:\j\workspace\windows-TGrou---bba2808d\Tools\Packaging.targets(1242,5): error : at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath) [D:\j\workspace\windows-TGrou---bba2808d\pkg\Microsoft.Private.CoreFx.NETCoreApp\Microsoft.Private.CoreFx.NETCoreApp.pkgproj]
09:02:17 D:\j\workspace\windows-TGrou---bba2808d\Tools\Packaging.targets(1242,5): error : at System.IO.FileSystemEnumerableIterator1.CommonInit() [D:\j\workspace\windows-TGrou---bba2808d\pkg\Microsoft.Private.CoreFx.NETCoreApp\Microsoft.Private.CoreFx.NETCoreApp.pkgproj] 09:02:17 D:\j\workspace\windows-TGrou---bba2808d\Tools\Packaging.targets(1242,5): error : at System.IO.FileSystemEnumerableIterator1..ctor(String path, String originalUserPath, String searchPattern, SearchOption searchOption, SearchResultHandler`1 resultHandler, Boolean checkHost) [D:\j\workspace\windows-TGrou---bba2808d\pkg\Microsoft.Private.CoreFx.NETCoreApp\Microsoft.Private.CoreFx.NETCoreApp.pkgproj]
09:02:17 D:\j\workspace\windows-TGrou---bba2808d\Tools\Packaging.targets(1242,5): error : at System.IO.Directory.GetFiles(String path, String searchPattern, SearchOption searchOption) [D:\j\workspace\windows-TGrou---bba2808d\pkg\Microsoft.Private.CoreFx.NETCoreApp\Microsoft.Private.CoreFx.NETCoreApp.pkgproj]

@MattGal
Copy link
Member

MattGal commented Jul 17, 2018

@davidsh generally someone from the CoreFX team will be expected to fix branch-specific build breaks, but on behalf of dnceng I'm taking a look and will see if I can find an owner for this.

@danmoseley
Copy link
Member

#31100

@MattGal
Copy link
Member

MattGal commented Jul 17, 2018

@davidsh perusing the logs I don't think this is your doing but the branch does actually appear to be broken on some kind of problem with repo build. This needs to be handled by the product teams, but if anything infrastructure falls out of it we'll be on point to help.

@davidsh davidsh added Servicing-consider Issue for next servicing release review and removed Servicing-consider Issue for next servicing release review labels Jul 17, 2018
@weshaggard
Copy link
Member

I'm taking a look at the CI failure as it seems like something weird is happening with the sync command.

@weshaggard
Copy link
Member

#31139 should fix the CI issue.

@weshaggard
Copy link
Member

@dotnet-bot test this please

@dotnet dotnet deleted a comment from davidsh Jul 19, 2018
@danmoseley
Copy link
Member

@davidsh looks like you have a test failure.

https://mc.dot.net/#/user/davidsh/pr~2Fjenkins~2Fdotnet~2Fcorefx~2Frelease~2F2.1~2F/test~2Ffunctional~2Fcli~2F/9a7f86190608e03d2bf2395782245a603068dd3b/workItem/System.Net.Http.Functional.Tests/analysis/xunit/System.Net.Http.Functional.Tests.PlatformHandler_HttpClientHandler_Proxy_Test~2FProxyAuth_Digest_Succeeds

System.Net.Http.HttpRequestException : An error occurred while sending the request.
---- System.Net.Http.WinHttpException : Error 12152 calling WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, 'The server returned an invalid or unrecognized response'.
Stack Trace :
   at System.Net.Http.HttpClient.FinishSendAsyncBuffered(Task`1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts) in D:\j\workspace\windows-TGrou---2c40d8a1\src\System.Net.Http\src\System\Net\Http\HttpClient.cs:line 479
   at System.Net.Http.Functional.Tests.HttpClientHandler_Proxy_Test.<>c__DisplayClass8_0.<<ProxyAuth_Digest_Succeeds>b__0>d.MoveNext() in D:\j\workspace\windows-TGrou---2c40d8a1\src\System.Net.Http\tests\FunctionalTests\HttpClientHandlerTest.Proxy.cs:line 272
--- End of stack trace from previous location where exception was thrown ---
   at System.Net.Test.Common.LoopbackServer.CreateServerAsync(Func`2 funcAsync, Options options) in D:\j\workspace\windows-TGrou---2c40d8a1\src\Common\tests\System\Net\Http\LoopbackServer.cs:line 67
   at System.Net.Http.Functional.Tests.HttpClientHandler_Proxy_Test.ProxyAuth_Digest_Succeeds() in D:\j\workspace\windows-TGrou---2c40d8a1\src\System.Net.Http\tests\FunctionalTests\HttpClientHandlerTest.Proxy.cs:line 256
--- End of stack trace from previous location where exception was thrown ---
----- Inner Stack Trace -----
   at System.Threading.Tasks.RendezvousAwaitable`1.GetResult() in D:\j\workspace\windows-TGrou---2c40d8a1\src\Common\src\System\Threading\Tasks\RendezvousAwaitable.cs:line 62
   at System.Net.Http.WinHttpHandler.StartRequest(WinHttpRequestState state) in D:\j\workspace\windows-TGrou---2c40d8a1\src\System.Net.Http.WinHttpHandler\src\System\Net\Http\WinHttpHandler.cs:line 861

@davidsh
Copy link
Contributor Author

davidsh commented Jul 19, 2018

@davidsh looks like you have a test failure.

We've seen this fail randomly before. It is unlikely caused by this PR since this test is not affected by the PR. We suspect this is a test bug.

I'll re-run the tests.

@davidsh
Copy link
Contributor Author

davidsh commented Jul 19, 2018

@dotnet-bot Test Windows x86 Release Build

@davidsh
Copy link
Contributor Author

davidsh commented Jul 19, 2018

@dotnet-bot test Outerloop Windows x64 Debug Build
@dotnet-bot test Outerloop Linux x64 Debug Build
@dotnet-bot test Outerloop NETFX x86 Debug Build
@dotnet-bot test Outerloop UWP CoreCLR x64 Debug Build
@dotnet-bot test Outerloop OSX x64 Debug Build

@davidsh
Copy link
Contributor Author

davidsh commented Jul 19, 2018

Current failures in CI are known test bugs.

@danmoseley danmoseley added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Jul 25, 2018
@davidsh davidsh removed the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Aug 9, 2018
@davidsh
Copy link
Contributor Author

davidsh commented Aug 9, 2018

@davidsh can you please merge the 2.1 PR and then resolve this issue? The checkin needs to happen before Tuesday to make 2.1.4. Thanks

@davidsh davidsh merged commit f602b21 into dotnet:release/2.1 Aug 9, 2018
@davidsh davidsh deleted the port_31100 branch August 9, 2018 18:44
@gronlund013
Copy link

@davidsh
Is it known that the proxy issues are still an issue?

I am running
.NET Core Runtime 2.1.3
ASP.NET Core 2.1.3
.NET Core SDK 2.1.401

@davidsh
Copy link
Contributor Author

davidsh commented Aug 30, 2018

@davidsh
Is it known that the proxy issues are still an issue?
I am running
.NET Core Runtime 2.1.3
ASP.NET Core 2.1.3
.NET Core SDK 2.1.401

This particular bug will be fixed in .NET Core Runtime 2.1.4 which hasn't been released yet.

See: https://www.microsoft.com/net/download/dotnet-core/2.1

There are other proxy related bugs that will be fixed in 2.1.5 as well.

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.

5 participants