[Uri] Fix IPv6 address format #25782

caesar-chen · 2018-04-05T18:24:49Z

Here are the issues in Uri with IPv6 address we may want to fix:

[Pending discussion] Uri.IdnHost should include [] around IPv6 address.
- Currently it returns ::1234, it should return [::1234].
[Originally done with PR Fix Uri.Host for IPv6 Link-local address corefx#29829 but reverted with Revert "Fix Uri.Host for IPv6 Link-local address (#29829)" corefx#30062] Uri.Host LLA (Link-local address) IPv6 address doesn't contain %number part.
- Currently it returns [fe80::e077:c9a3:eeba:b8e9], it should return [fe80::e077:c9a3:eeba:b8e9%18].
- Note: Uri.IdnHost correctly contains the %number part.

If we choose to fix these problems, we can undo workarounds in dotnet/corefx#28740, dotnet/corefx#28578, dotnet/corefx#28849 and dotnet/corefx#28971.

/cc: @dotnet/ncl

The text was updated successfully, but these errors were encountered:

stephentoub · 2018-04-05T23:11:39Z

This issue is marked as future, but isn't there a bug here in SocketsHttpHandler we need to fix in 2.1? That we're incorrectly dropping the scope from a link-local address?

caesar-chen · 2018-04-05T23:21:58Z

but isn't there a bug here in SocketsHttpHandler we need to fix in 2.1?

Yes, we have #25726 opened for it, I will be working on it. Opened this issue to track future Uri work.

stephentoub · 2018-04-06T00:20:59Z

Ah, ok.

MarcoRossignoli · 2018-04-18T20:23:36Z

@Caesar1995 one question LLA start with fe: or fe80: ?

caesar-chen · 2018-04-18T22:05:19Z

IPv6 LLA starts with fe80. See RFC 4291:

Link-Local addresses have the following format:

   |   10     |
   |  bits    |         54 bits         |          64 bits           |
   +----------+-------------------------+----------------------------+
   |1111111010|           0             |       interface ID         |
   +----------+-------------------------+----------------------------+

IPv6 notation will be like: FE80::/10

MarcoRossignoli · 2018-04-19T07:01:16Z

Currently it returns [fe::ee:b8e9], it should return [fe::ee:b8e9%18].

@Caesar1995 ok i ask this because in you sample for LLA on issue address start with fe.

After this change i think we need also to update sample on docs https://msdn.microsoft.com/en-us/library/system.uri.dnssafehost(v=vs.110).aspx#Anchor_3, again porting on netfx?

caesar-chen · 2018-04-19T16:35:23Z

ok i ask this because in you sample for LLA on issue address start with fe.

Oops, corrected in sample to avoid confusion. Thanks!

After this change i think...

This is a proposal which need investigation (so don't start working on it yet). If we choose to accept it, then yes we need to do those following steps.

MarcoRossignoli · 2018-04-20T12:05:21Z

@Caesar1995 i did a pass through parsing code, let me know if i can try a PR(if this proposal success).

rmkerr · 2018-05-10T20:34:16Z

Hey @MarcoRossignoli if you'd like to take this issue in two segments I'm confident that we would accept a fix to part 2 here, since that behavior is clearly a bug. Part 1 requires a little more discussion, so I think splitting the fix up would be best.

MarcoRossignoli · 2018-05-10T23:48:12Z

@rmkerr sure! I'll work on it! Thank's for consideration!

MarcoRossignoli · 2018-05-16T20:48:14Z

@rmkerr i'm working on issue and when compile i get compile error

UriRelativeResolutionTest.cs(26,116): error CS0117: 'PlatformDetection' does not contain a definition for 'IsNetfx472OrNewer'

i'm able to go on if remove field, any idea?

caesar-chen · 2018-05-16T20:54:03Z

Did you try to clean && sync && build the entire corefx project? That attribute is introduced recently.

rmkerr · 2018-05-16T20:54:34Z

I would pull the latest version of the repo, then clean and build the whole project again. You can find more info on that here: developer guide. You probably last built the test utilities before this PR was merged, adding the property you mentioned.

MarcoRossignoli · 2018-05-17T08:08:47Z

@Caesar1995 @rmkerr my fault...i work on more host, i was convinced to work on updated repo, it all ok!

ghost · 2018-05-18T18:52:14Z

cc @wfurt, this is also related: https://github.com/dotnet/corefx/issues/27529

Contributes to #28863 This PR address the second part of issue: Uri.Host LLA (Link-local address) IPv6 address doesn't contain %number part. Currently it returns [fe80::e077:c9a3:eeba:b8e9], it should return [fe80::e077:c9a3:eeba:b8e9%18]. Note: Uri.IdnHost correctly contains the %number part.

MarcoRossignoli · 2018-05-19T07:05:50Z

@Caesar1995 we've merged fix to part 2 you could update issue description.

karelz · 2018-05-19T16:55:51Z

Thanks @MarcoRossignoli! Top post updated.

Contributes to #28863 Replace PR #29769 after revert #29818 for Outerloop CI fail This PR address the second part of issue: Uri.Host LLA (Link-local address) IPv6 address doesn't contain %number part. Currently it returns [fe80::e077:c9a3:eeba:b8e9], it should return [fe80::e077:c9a3:eeba:b8e9%18]. Note: Uri.IdnHost correctly contains the %number part.

Tratcher · 2018-06-01T18:35:03Z

This last change is now failing in Kestrel tests and we need to make sure we have a consistent story (aspnet/KestrelHttpServer#2637). Note these tests don't use HttpClient directly, only System.Uri and Sockets.

Do you expect the various http clients to send this scope id in the Host header? Walking the spec has an interesting history here.

https://tools.ietf.org/html/rfc7230#section-5.4
Host = uri-host [ ":" port ] ; Section 2.7.1
uri-host = <host, see [RFC3986], Section 3.2.2>
https://tools.ietf.org/html/rfc3986#section-3.2.2
host = IP-literal / IPv4address / reg-name
IP-literal = "[" ( IPv6address / IPvFuture ) "]"
IPv6address = {... long and messy, but without scope ids ...}
A host identified by an IPv6 literal address is represented inside
the square brackets without a preceding version flag. The ABNF
provided here is a translation of the text definition of an IPv6
literal address provided in [RFC3513]. This syntax does not support
IPv6 scoped addressing zone identifiers.

So no, but...

Updated by https://tools.ietf.org/html/rfc6874#section-2
IP-literal = "[" ( IPv6address / IPv6addrz / IPvFuture ) "]"
ZoneID = 1*( unreserved / pct-encoded )
IPv6addrz = IPv6address "%25" ZoneID

So yes, but then https://tools.ietf.org/html/rfc6874#section-4
An HTTP client, proxy, or other intermediary MUST remove any ZoneID
attached to an outgoing URI, as it has only local significance at the
sending host.

That looks like a pretty definitive No for putting them in Host headers.

Chrome, IE, and Edge do not accept scope Ids in the address bar so they won't send one. Curl.exe (windows) allows the scope but strips it from the Host. HttpClient currently does send a scope id in the Host and the request is rejected by Kestrel with a 400. That looks like a bug you'll need to address either in HttpClient or System.Uri.

davidsh · 2018-06-01T18:45:17Z

Do you expect the various http clients to send this scope id in the Host header?

What does .NET Framework HttpWebRequest / HttpClient do?

Tratcher · 2018-06-01T18:50:12Z

.NET 4.6.1 HttpClient strips the scope id from the Host header.

Tratcher · 2018-06-01T20:02:35Z

With this URI change getting the host without the scopeid is a bit obnoxious.

internal static string GetHost(Uri requestUri)
        {
            var authority = requestUri.Authority;
            if (requestUri.HostNameType == UriHostNameType.IPv6)
            {
                // Make sure there's no % scope id. https://github.com/aspnet/KestrelHttpServer/issues/2637
                var address = IPAddress.Parse(requestUri.Host);
                address = new IPAddress(address.GetAddressBytes()); // Drop scope Id.
                authority = $"[{address}]:{requestUri.Port.ToString(CultureInfo.InvariantCulture)}";
            }
            return authority;
        }

davidsh · 2018-06-01T20:12:07Z

I think we may need to revert dotnet/corefx#29829 in the Master branch right away and reconsider how best to solve the overall problems.

rmkerr · 2018-06-01T22:57:48Z

I don't know if we need to revert this immediately or not, but we should definitely discuss the approach. It seems like a case where what's best from a pure URI standpoint conflicts with what's best for the HTTP stack. I'm inclined to say that the HTTP stack takes precedence, but I'd like to hear what others think.

Tratcher · 2018-06-01T23:02:28Z

What's best for Uri is what's best for it's consumers. What other consumers use Host, Authority, IdnHost, and what are their requirements?
Http can work around it if needed.

davidsh · 2018-06-01T23:22:43Z

I'm inclined to say that the HTTP stack takes precedence

I tend to agree with this. I think perhaps if we really want a Uri property to return the host portion with scope-ids we might need a new property to do it. Otherwise, we risk breaking more things.

MarcoRossignoli · 2018-06-02T20:16:23Z

I think perhaps if we really want a Uri property to return the host portion with scope-ids we might need a new property to do it

This could be the safest choice...even if i think it would be a "strange" property, may lead to confusion.

EDIT:

Otherwise, we risk breaking more things.

I mean you have a "big picture" if it's highly likely issues other than "HttpClients" that's okay IMO.

Tratcher · 2018-06-04T17:15:30Z

Hmm, I just realized this changed the output of ToString as well. .NET ToString does not include the scope, but now core does.

davidsh · 2018-06-04T17:19:38Z

Hmm, I just realized this changed the output of ToString as well. .NET ToString does not include the scope, but now core does.

I don't think that's necessarily a bad thing. The .OriginalString still has the scope-id. In fact, we've seen an interesting interop issue using UWP apps when System.Uri needs to get converted at the lower layers to WinRT Windows.Foundation.Uri. These uri's with scope-id that are valid System.Uri are broken when Windows.Foundation.Uri tries to re-create them (using .OriginalString). That is a WinRT bug tracking also in GitHub corefx.

rmkerr · 2018-06-04T17:57:37Z

Agreed -- ideally I think the scope ID should be included in ToString. It's a meaningful part of the URI, and by removing it we change the meaning. Obviously we do have to consider app compat though, so it is worth considering what use cases we might break.

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.

karelz · 2019-10-02T18:08:52Z

Triage: We may not be able to fix it at all, the goal is to decide in 5.0 timeframe.

scalablecory · 2020-01-27T05:16:22Z

Ran into this one today!

CC @MihaZupan

wfurt · 2020-01-27T17:28:55Z

I was thinking about it more and I'm not sure this is a problem other than surprise. According to documentation:

The RFC 3490 compliant International Domain Name of the host, using Punycode as appropriate. This string, after being unescaped if necessary, is safe to use for DNS resolution.

If we put scopeID or '[]` the value will no longer be safe to use for DNS IMHO and additional work would be needed by the caller to make it passable to NameResolution API.

I'm wondering what was the original intention for this property.

wfurt · 2022-05-11T19:33:24Z

I'm wondering if we should just close this @stephentoub @MihaZupan
While the existing behavior may be suboptimal, changing it is going to be major breaking change IMHO.
I'm not sure we would be ever brave enough to do it and this sits here long time with no use.
If feels like we would be better off to document current behavior and feel with it in SocketsHttpHandler.

MihaZupan · 2024-02-16T17:23:52Z

I doubt we'll be changing the behavior here given how long we've been sitting with this behavior, but we should at least document it better.

karelz changed the title ~~Correctly format IPv6 address in uri layer~~ Uri - fix IPv6 address format Apr 5, 2018

rmkerr changed the title ~~Uri - fix IPv6 address format~~ [Uri] Fix IPv6 address format May 2, 2018

karelz assigned MarcoRossignoli May 11, 2018

karelz unassigned MarcoRossignoli May 19, 2018

caesar-chen self-assigned this Jun 4, 2018

caesar-chen removed their assignment Mar 8, 2019

msftgits transferred this issue from dotnet/corefx Jan 31, 2020

msftgits added this to the 5.0 milestone Jan 31, 2020

karelz modified the milestones: 5.0, Future Mar 31, 2020

wfurt mentioned this issue Sep 20, 2021

HTTP 400 ("Invalid Host header") for link-local IPv6 addresses #59341

Closed

wfurt mentioned this issue Feb 5, 2024

Uri.TryCreate (and ctor) behave weird when using the IPv6 format #97939

Open

MihaZupan added documentation Documentation bug or enhancement, does not impact product or test code and removed bug labels Feb 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Uri] Fix IPv6 address format #25782

[Uri] Fix IPv6 address format #25782

caesar-chen commented Apr 5, 2018

stephentoub commented Apr 5, 2018

caesar-chen commented Apr 5, 2018

stephentoub commented Apr 6, 2018

MarcoRossignoli commented Apr 18, 2018

caesar-chen commented Apr 18, 2018

MarcoRossignoli commented Apr 19, 2018 •

edited

Loading

caesar-chen commented Apr 19, 2018

MarcoRossignoli commented Apr 20, 2018 •

edited

Loading

rmkerr commented May 10, 2018

MarcoRossignoli commented May 10, 2018

MarcoRossignoli commented May 16, 2018

caesar-chen commented May 16, 2018

rmkerr commented May 16, 2018

MarcoRossignoli commented May 17, 2018

ghost commented May 18, 2018

MarcoRossignoli commented May 19, 2018 •

edited

Loading

karelz commented May 19, 2018

Tratcher commented Jun 1, 2018 •

edited

Loading

davidsh commented Jun 1, 2018

Tratcher commented Jun 1, 2018

Tratcher commented Jun 1, 2018 •

edited by karelz

Loading

davidsh commented Jun 1, 2018

rmkerr commented Jun 1, 2018

Tratcher commented Jun 1, 2018 •

edited

Loading

davidsh commented Jun 1, 2018

MarcoRossignoli commented Jun 2, 2018 •

edited

Loading

Tratcher commented Jun 4, 2018

davidsh commented Jun 4, 2018

rmkerr commented Jun 4, 2018

karelz commented Oct 2, 2019

scalablecory commented Jan 27, 2020

wfurt commented Jan 27, 2020

wfurt commented May 11, 2022

MihaZupan commented Feb 16, 2024

[Uri] Fix IPv6 address format #25782

[Uri] Fix IPv6 address format #25782

Comments

caesar-chen commented Apr 5, 2018

stephentoub commented Apr 5, 2018

caesar-chen commented Apr 5, 2018

stephentoub commented Apr 6, 2018

MarcoRossignoli commented Apr 18, 2018

caesar-chen commented Apr 18, 2018

MarcoRossignoli commented Apr 19, 2018 • edited Loading

caesar-chen commented Apr 19, 2018

MarcoRossignoli commented Apr 20, 2018 • edited Loading

rmkerr commented May 10, 2018

MarcoRossignoli commented May 10, 2018

MarcoRossignoli commented May 16, 2018

caesar-chen commented May 16, 2018

rmkerr commented May 16, 2018

MarcoRossignoli commented May 17, 2018

ghost commented May 18, 2018

MarcoRossignoli commented May 19, 2018 • edited Loading

karelz commented May 19, 2018

Tratcher commented Jun 1, 2018 • edited Loading

davidsh commented Jun 1, 2018

Tratcher commented Jun 1, 2018

Tratcher commented Jun 1, 2018 • edited by karelz Loading

davidsh commented Jun 1, 2018

rmkerr commented Jun 1, 2018

Tratcher commented Jun 1, 2018 • edited Loading

davidsh commented Jun 1, 2018

MarcoRossignoli commented Jun 2, 2018 • edited Loading

Tratcher commented Jun 4, 2018

davidsh commented Jun 4, 2018

rmkerr commented Jun 4, 2018

karelz commented Oct 2, 2019

scalablecory commented Jan 27, 2020

wfurt commented Jan 27, 2020

wfurt commented May 11, 2022

MihaZupan commented Feb 16, 2024

MarcoRossignoli commented Apr 19, 2018 •

edited

Loading

MarcoRossignoli commented Apr 20, 2018 •

edited

Loading

MarcoRossignoli commented May 19, 2018 •

edited

Loading

Tratcher commented Jun 1, 2018 •

edited

Loading

Tratcher commented Jun 1, 2018 •

edited by karelz

Loading

Tratcher commented Jun 1, 2018 •

edited

Loading

MarcoRossignoli commented Jun 2, 2018 •

edited

Loading