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

Fix Uri.Host for IPv6 Link-local address #29769

Merged
merged 9 commits into from
May 19, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/System.Private.Uri/src/System/IPv6AddressHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal static string ParseCanonicalName(string str, int start, ref bool isLoop
((long*)numbers)[0] = 0L;
((long*)numbers)[1] = 0L;
isLoopback = Parse(str, numbers, start, ref scopeId);
return '[' + CreateCanonicalName(numbers) + ']';
return '[' + CreateCanonicalName(numbers) + ((numbers[0] == 0xfe80) ? scopeId : "") + ']';
}
}

Expand Down
16 changes: 16 additions & 0 deletions src/System.Private.Uri/tests/FunctionalTests/UriIpHostTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,22 @@ private void ParseBadIPv4Address(string badIpv4String)

#region IPv6

[Theory]
[InlineData("fe80::e077:c9a3:eeba:b8e9", "%18", true)]
[InlineData("Fe80::e077:c9a3:eeba:b8e9", "%18", true)]
[InlineData("fE80::e077:c9a3:eeba:b8e9", "%18", true)]
[InlineData("FE80::e077:c9a3:eeba:b8e9", "%18", true)]
[InlineData("FE80::e077:c9a3:eeba:b8e9", "%eth10", true)]
[InlineData("fe81::e077:c9a3:eeba:b8e9", "%18", false)]
[SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
Copy link
Member Author

Choose a reason for hiding this comment

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

@davidsh do you consider porting on netfx?

Copy link
Contributor

Choose a reason for hiding this comment

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

I will mark it as "consider". But those things have longer lead times for review and it might not qualify due to strict app-compat guidelines.

public void UriIPv6Host_LinkLocalAddress(string address, string zoneIndex, bool isValidLinkLocalAddress)
Copy link
Contributor

Choose a reason for hiding this comment

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

The test name should be 3 parts, so it should probably be named like this: "Host_IPv6LinkLocalAddress_HasScopeId"

  • First part describes the method or property under test. In this case, the test is validating the .Host property.
  • Second part describes the inputs to the test.
  • Third part describes the expected result.

Copy link
Contributor

@davidsh davidsh May 17, 2018

Choose a reason for hiding this comment

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

Also, you should separate out the valid and invalid cases into separate tests. The invalid one could be named, "Host_NonIPv6LinkLocalAddress_NoScopeId"

{
string scopedLiteralIpv6 = "[" + address + zoneIndex + "]";
string literalIpV6Uri = "http://" + scopedLiteralIpv6;
var uri = new Uri(literalIpV6Uri);
Assert.Equal(isValidLinkLocalAddress ? scopedLiteralIpv6 : "[" + address + "]", uri.Host, ignoreCase: true);
}

[Fact]
public void UriIPv6Host_CanonicalCollonHex_Success()
{
Expand Down