Fix IPv6Address parsing #37734

wtgodbe · 2019-05-16T23:42:06Z

Ports 4006e52 & 6fdf8be.

@karelz @davidsh @wfurt PTAL, especially at the changes to src/System.Private.Uri/src/System/IPv6AddressHelper.cs

karelz

You didn't port all files. Why?

src/System.Net.Primitives/src/System/Net/IPAddressParser.cs

karelz

Oh, it seems that some of the files changed since the previous release, right?

Co-Authored-By: Karel Zikmund <[email protected]>

wtgodbe · 2019-05-17T16:46:20Z

@karelz yes, System.Net.Primitives\IPV6AddressHelper.cs moved to Common\src\System\Net\IPv6AddressHelper.Common.cs.

…in Fake

stephentoub

Looks fine, but there are a bunch of related test failures.

wtgodbe · 2019-05-20T16:44:14Z

System.Net.Http.Functional.Tests.SocketsHttpHandler_HttpClientHandlerTest.GetAsync_IPBasedUri_Success(address: ::1) [FAIL]

  System.Net.Http.HttpRequestException : IPv4 address 0.0.0.0 and IPv6 address ::0 are unspecified addresses that cannot be used as a target address.
  Parameter name: hostName
  ---- System.ArgumentException : IPv4 address 0.0.0.0 and IPv6 address ::0 are unspecified addresses that cannot be used as a target address.
  Parameter name: hostName

wtgodbe · 2019-05-20T16:44:55Z

System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_HttpClientHandlerTest_Http2.ProxiedIPAddressRequest_NotDefaultPort_CorrectlyFormatted(host: "[::1234]") [FAIL]

  Assert.Contains() Failure
  Not found: GET http://[::1234]/ HTTP/1.1
  In value:  List<String> ["GET http://[::]/ HTTP/1.1", "Host: [::]"]

There's several instances of this one

wtgodbe · 2019-05-20T16:48:13Z

System.Net.Http.Functional.Tests.PlatformHandler_HttpClientHandlerTest.GetAsync_IPv6LinkLocalAddressUri_Success [FAIL]

  System.Net.Http.HttpRequestException : An error occurred while sending the request.
  ---- System.Net.Http.WinHttpException : Error 12029 calling WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, 'A connection with the server could not be established'.

wtgodbe · 2019-05-20T16:49:30Z

System.Net.Http.Tests.HttpEnvironmentProxyTest/HttpProxy_Uri_Parsing(_input: "[::1]", _host: "[::1]", _port: "80", _user: null, _password: null)

Expected: [::1]
Actual: [::]

There are some other tests that fail with this same expectation, like System.PrivateUri.Tests.UriEscapingTest/UriAbsoluteEscaping_FullIPv6Uri_NothingEscaped

wtgodbe · 2019-05-20T17:59:27Z

These same tests are passing in 2.1 with the original change, so I've either ported something wrong here, or missed somewhere where we need an additional change. Many of the failing tests are due to #37734 (comment), I'm still trying to figure out what I've missed.

wtgodbe · 2019-05-20T23:19:41Z

That last commit cleaned up most of the failures, but some remain:

System.Net.Primitives.Functional.Tests: ParseIPv6_InvalidAddress_ThrowsFormatException: (invalidAddress: \"[\"), (invalidAddress: \"[]\"), (invalidAddress: \"\"). These fail on Windows.10.Amd64.ClientRS5.Open-x86-Release - They should throw a System.Net.Sockets.SocketException, but don't.

These test cases were added with this PR: https://github.com/dotnet/corefx/pull/37734/files#diff-f104ad3dd5c6bd9bf51bc59fdc6b1440R349

wtgodbe · 2019-05-20T23:20:46Z

System.Private.Uri.Functional.Tests: System.PrivateUri.Tests.UriIpHostTest/UriIPv6Host_BadAddress(address: \":1:2:3:4:5:6:7:8\") - This fails on Windows.10.Amd64.ClientRS5.Open-x86-Release. It should be classified Not IPv6, but is classified IPv6.

This test was added in this PR: https://github.com/dotnet/corefx/pull/37734/files#diff-a12b4b6e04e89f4ef9e929180d64ee90R324. Strange that this was the only test case that failed, while other similar cases (":1:2:3:4:5:6:7", ":1:2:3:4:5:6:7:8:9") did not fail.

wtgodbe · 2019-05-20T23:26:55Z

Some catastrophic infra failures on Fedora.28.Amd64.Open-x64:

2019-05-20 20:17:01,663: INFO: dockerhelper(214): write_commands_to_file: Generating Docker execution script
2019-05-20 20:17:39,026: INFO: servicebusrepository(85): renew_workitem_lock: Entering renew_workitem_lock for https://nethelix.servicebus.windows.net/ubuntu.1604.amd64.open/messages/86427214/f3bf795b-6a37-4c22-89b1-fd1cea1ccae0
2019-05-20 20:17:39,156: INFO: saferequests(87): request_with_retry: Response complete with status code '200'
2019-05-20 20:17:39,156: INFO: servicebusrepository(95): renew_workitem_lock: Renewed work item lock. Status Code: 200
2019-05-20 20:18:01,724: ERROR: executor(523): _execute_command_in_container: Exception: UnixHTTPConnectionPool(host='localhost', port=None): Read timed out. (read timeout=60)
2019-05-20 20:18:01,724: INFO: executor(525): _execute_command_in_container: Finished _execute_command_in_container, exit code: None

@MattGal is this a known issue? I'm seeing the same on some Ubuntu.1604.Arm64.Open-arm64:Release jobs

MattGal · 2019-05-20T23:29:50Z

@wtgodbe yes this is https://github.com/dotnet/core-eng/issues/6327. Basically the docker daemon can sometimes (all the time?) take a while to start after boot, and while I've done a bunch of useful stuff I also need to update the scripts to wait til the daemon is ready before we start doing work.

If you have Kusto access, you can see this for work items from your run by using a query like this:

WorkItems | where Started > now(-12d) and MachineName == "a0025SF" | join kind= inner (
   Jobs   | where tolower(QueueName) == "ubuntu.1604.amd64.open" | project  QueueName , JobId, Build, Type, Source 
)   on JobId
| order by Started desc

... where we note only a single 999 failure on this run, as the first work item.

wtgodbe · 2019-05-20T23:30:40Z

Also an infra failure on Windows.10.Amd64.ClientRS4.ES.Open-x64:Debug

Searching 'C:\dotnetbuild\work\cf8bde42-1f0a-471b-9db7-11fec0e11a56\Work\e1102e7a-c27f-48ba-9ea7-5ba8bca8dbb5\Exec..' for log files
Found log 'C:\dotnetbuild\work\cf8bde42-1f0a-471b-9db7-11fec0e11a56\Work\e1102e7a-c27f-48ba-9ea7-5ba8bca8dbb5\Exec..\console.4c6dd660.log'
Uri 'https://helixexternalresults.blob.core.windows.net/dotnet-corefx-refs-pull-37734-merge-cf8bde421f0a471b9d/System.Security.Cryptography.X509Certificates.Tests/console.4c6dd660.log?sv=2017-07-29&sr=c&sig=cIXqF70vjtNEk8ev6QuukPYCi41ProNm68hzO7Hjwzg%3D&se=2019-05-30T20%3A05%3A40Z&sp=rl'
Generated log list:
console.4c6dd660.log

Searching 'C:\dotnetbuild\work\cf8bde42-1f0a-471b-9db7-11fec0e11a56\Work\e1102e7a-c27f-48ba-9ea7-5ba8bca8dbb5\Exec' for test results files
No results file found in any of the following formats: xunit, junit, trx
Traceback (most recent call last):
File "C:\dotnetbuild\work\cf8bde42-1f0a-471b-9db7-11fec0e11a56\Payload\reporter\run.py", line 95, in
main()
File "C:\dotnetbuild\work\cf8bde42-1f0a-471b-9db7-11fec0e11a56\Payload\reporter\run.py", line 84, in main
for b in batches:
File "C:\dotnetbuild\work\cf8bde42-1f0a-471b-9db7-11fec0e11a56\Payload\reporter\helpers.py", line 13, in batch
for item in iterable:
File "C:\dotnetbuild\work\cf8bde42-1f0a-471b-9db7-11fec0e11a56\Payload\reporter\test_results_reader_init_.py", line 102, in read_results
yield add_logs(_no_results_result(), log_list)
File "C:\dotnetbuild\work\cf8bde42-1f0a-471b-9db7-11fec0e11a56\Payload\reporter\test_results_reader_init.py", line 72, in add_logs
tr.attachments.append(TestResultAttachment(
AttributeError: 'NoneType' object has no attribute 'append'
20/05/2019-20:30:06,28
2019-05-20 20:30:06,755: ERROR: xunit-reporter(69): main: Unable to report xunit results: no test results xml file found.

wtgodbe · 2019-05-20T23:33:34Z

I'm seeing a pattern of test failures only occuring on Windows.10.Amd64.ClientRS5.Open-x86:Release - In this PR, System.Net.Primitives.Functional.Tests: ParseIPv6_InvalidAddress_ThrowsFormatException and System.Private.Uri.Functional.Tests: System.PrivateUri.Tests.UriIpHostTest/UriIPv6Host_BadAddress(address: \":1:2:3:4:5:6:7:8\") both fail only on that queue, and in my other PR (#37733), the only test failure is on that queue as well. @wfurt @karelz is there some known networking weirdness on that platform/configuration? I'm still poking around in the meantime.

MattGal · 2019-05-20T23:36:32Z

@wtgodbe scroll up, you're right that post-Python3 transition there are weird bugs we need to iron out in the error code paths, but the reason you're hitting these bugs is you're in an error code path. This is an xunit test failing from crashing with STATUS_STACK_BUFFER_OVERRUN and should be investigated:

C:\dotnetbuild\work\cf8bde42-1f0a-471b-9db7-11fec0e11a56\Work\e1102e7a-c27f-48ba-9ea7-5ba8bca8dbb5\Exec>"C:\dotnetbuild\work\cf8bde42-1f0a-471b-9db7-11fec0e11a56\Payload\dotnet.exe" xunit.console.dll System.Security.Cryptography.X509Certificates.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=nonnetcoreapptests -notrait category=nonwindowstests -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing  
  Discovering: System.Security.Cryptography.X509Certificates.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Security.Cryptography.X509Certificates.Tests (found 446 of 465 test cases)
  Starting:    System.Security.Cryptography.X509Certificates.Tests (parallel test collections = on, max threads = 4)
----- end 20:30:04,70 ----- exit code -1073740791 ----------------------------------------------------------

-1073740791 = 0xc0000409 = STATUS_STACK_BUFFER_OVERRUN

wtgodbe · 2019-05-30T19:20:28Z

@davidsh I remember now why I removed ShouldHaveIpv4Embedded() from IPv6AddressHelper.cs - because IPv6AddressHelper.Common.cs already defines a function private static unsafe bool ShouldHaveIpv4Embedded(ReadOnlySpan<ushort> numbers), so changing the parameter type from ushort* numbers to ReadOnlySpan<ushort> numbers in System.Private.Uri's IPv6AddressHelper.cs exposed the duplicate (System.Private.Uri does reference the Common IPv6AddressHelper.Common.cs file:

corefx/src/System.Private.Uri/src/System.Private.Uri.csproj

Lines 24 to 26 in 88f4967

    
           <Compile Include="$(CommonPath)\System\Net\IPv6AddressHelper.Common.cs"> 
        
             <Link>System\IPv6AddressHelper.Common.cs</Link> 
        
           </Compile>

). Does that sound like the right thing to do?

corefx/src/Common/src/System/Net/IPv6AddressHelper.Common.cs

Line 43 in 88f4967

    
           internal unsafe static bool ShouldHaveIpv4Embedded(ReadOnlySpan<ushort> numbers)

davidsh · 2019-05-30T19:29:54Z

Does that sound like the right thing to do?

Yes. This is probably due to the previous refactoring of this class by a preview PR.

wtgodbe · 2019-05-30T21:49:37Z

Thanks to @davidsh for identifying the issue here - the RS5 queue is actually running netfx tests, so many of the failures there are due to that RS5 machine not being patched. The failures from #37734 (comment) are because that test case expects an inner exception to be thrown, which isn't the behavior on Framework. Fixing up the test cases now.

wtgodbe · 2019-05-30T21:58:20Z

/azp run

azure-pipelines · 2019-05-30T21:58:55Z

Azure Pipelines successfully started running 3 pipeline(s), but failed to run 1 pipeline(s).

davidsh · 2019-05-30T22:27:42Z

/azp run

azure-pipelines · 2019-05-30T22:28:14Z

Azure Pipelines successfully started running 2 pipeline(s), but failed to run 2 pipeline(s).

wtgodbe · 2019-05-30T22:28:47Z

/azp run corefx-outerloop

azure-pipelines · 2019-05-30T22:28:52Z

No pipelines are associated with this pull request.

wtgodbe · 2019-05-30T22:28:55Z

/azp run

azure-pipelines · 2019-05-30T22:29:33Z

Azure Pipelines successfully started running 2 pipeline(s), but failed to run 2 pipeline(s).

wtgodbe · 2019-05-30T22:32:21Z

/azp run

azure-pipelines · 2019-05-30T22:32:52Z

Azure Pipelines successfully started running 2 pipeline(s), but failed to run 2 pipeline(s).

wtgodbe · 2019-05-30T23:02:24Z

Serialization\Value.ReadTests.cs(225,58): error CS0117: 'BitConverter' does not contain a definition for 'SingleToInt32Bits' [D:\a\1\s\src\System.Text.Json\tests\System.Text.Json.Tests.csproj]

Weird... can't imagine why this would start failing just now. Looking into it.

wtgodbe · 2019-05-30T23:16:06Z

Failures are unrelated, #38077 should resolve (but we need to wait for that to go in anyways, since we need to see the netfx tests pass)

wtgodbe · 2019-05-31T00:50:49Z

/azp run

azure-pipelines · 2019-05-31T00:51:22Z

Azure Pipelines successfully started running 3 pipeline(s), but failed to run 1 pipeline(s).

wtgodbe · 2019-05-31T16:34:07Z

outerloop test failures are unrelated, merging

* Fix IPv6Address parsing * Update src/System.Net.Primitives/src/System/Net/IPAddressParser.cs Co-Authored-By: Karel Zikmund <[email protected]> * Use System.Diagnostics, Remove duplicate function, fix method header in Fake * Fix deletion of call to Parse() * Fix call to Parse() * Fix tests to account for NetFx runs * Fix Syntax Commit migrated from dotnet/corefx@f8c382f

Fix IPv6Address parsing

648faa1

karelz requested a review from wfurt May 17, 2019 16:14

karelz added the area-System.Net label May 17, 2019

karelz suggested changes May 17, 2019

View reviewed changes

src/System.Net.Primitives/src/System/Net/IPAddressParser.cs Outdated Show resolved Hide resolved

karelz reviewed May 17, 2019

View reviewed changes

Update src/System.Net.Primitives/src/System/Net/IPAddressParser.cs

100275e

Co-Authored-By: Karel Zikmund <[email protected]>

Use System.Diagnostics, Remove duplicate function, fix method header …

40fac58

…in Fake

stephentoub approved these changes May 17, 2019

View reviewed changes

wtgodbe added 2 commits May 20, 2019 12:44

Fix deletion of call to Parse()

63d7149

Fix call to Parse()

1df2a0b

davidsh added this to the 3.0 milestone May 25, 2019

davidsh mentioned this pull request May 27, 2019

Allow interface names in IPv6 link-local addresses #35278

Merged

wtgodbe force-pushed the IPV6Address branch from ac4161d to 1df2a0b Compare May 30, 2019 19:21

Fix tests to account for NetFx runs

023a82c

Fix Syntax

d270df2

davidsh approved these changes May 30, 2019

View reviewed changes

wtgodbe merged commit f8c382f into dotnet:master May 31, 2019

wtgodbe mentioned this pull request Jun 4, 2019

[release/2.1] Disable tests on netfx for loop in URI & IPv6AddressHelper #38224

Merged

davidsh mentioned this pull request Jun 18, 2019

Fix ipv6 address parsing with leading compressor #38654

Merged

Fix IPv6Address parsing #37734

Fix IPv6Address parsing #37734

Uh oh!

Conversation

wtgodbe commented May 16, 2019

Uh oh!

karelz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

karelz left a comment

Choose a reason for hiding this comment

Uh oh!

wtgodbe commented May 17, 2019

Uh oh!

stephentoub left a comment

Choose a reason for hiding this comment

Uh oh!

wtgodbe commented May 20, 2019

Uh oh!

wtgodbe commented May 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wtgodbe commented May 20, 2019

Uh oh!

wtgodbe commented May 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wtgodbe commented May 20, 2019

Uh oh!

wtgodbe commented May 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wtgodbe commented May 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wtgodbe commented May 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MattGal commented May 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wtgodbe commented May 20, 2019

Uh oh!

wtgodbe commented May 20, 2019

Uh oh!

MattGal commented May 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wtgodbe commented May 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidsh commented May 30, 2019

Uh oh!

wtgodbe commented May 30, 2019

Uh oh!

wtgodbe commented May 30, 2019

Uh oh!

azure-pipelines bot commented May 30, 2019

Uh oh!

davidsh commented May 30, 2019

Uh oh!

azure-pipelines bot commented May 30, 2019

Uh oh!

wtgodbe commented May 30, 2019

Uh oh!

azure-pipelines bot commented May 30, 2019

Uh oh!

wtgodbe commented May 30, 2019

Uh oh!

azure-pipelines bot commented May 30, 2019

Uh oh!

wtgodbe commented May 30, 2019

Uh oh!

azure-pipelines bot commented May 30, 2019

Uh oh!

wtgodbe commented May 30, 2019

Uh oh!

wtgodbe commented May 30, 2019

Uh oh!

wtgodbe commented May 20, 2019 •

edited

Loading

wtgodbe commented May 20, 2019 •

edited

Loading

wtgodbe commented May 20, 2019 •

edited

Loading

wtgodbe commented May 20, 2019 •

edited

Loading

wtgodbe commented May 20, 2019 •

edited

Loading

MattGal commented May 20, 2019 •

edited

Loading

MattGal commented May 20, 2019 •

edited

Loading

wtgodbe commented May 30, 2019 •

edited

Loading