SocketsHttpHandler: Proxied IPv6 requests have incorrectly formatted host/port in request line

[caesar-chen]

On WinHttpHandler, we will append [] around the IPv6 address.
"GET http://[::1234]/ HTTP/1.1", "Proxy-Connection: Keep-Alive", "Host: [::1234]"

On SocketsHttpHandler, currently we don't:
"GET http://::1234/ HTTP/1.1", "Host: [::1234]"

Also, if we specify a port number, SocketsHttpHandler will strip that in the uri send to the proxy as well.

WinHttpHandler:
In value: List ["GET http://[::1234]:8080/ HTTP/1.1", "Proxy-Connection: Keep-Alive", "Host: [::1234]:8080"]

SocketsHttpHandler:
In value: List ["GET http://::1234/ HTTP/1.1", "Host: [::1234]:8080"]

[karelz]

@Caesar1995 that seems rather impactful - doesn't it mean that proxies won't work with IPv6 at all? Moving to 2.1 for now ...

Also, I am concerned about missing "Proxy-Connection: Keep-Alive" header - we should file separate bug IMO - @geoffkizer @stephentoub thoughts?

[caesar-chen]

doesn't it mean that proxies won't work with IPv6 at all?

I don't know, it unlikely doesn't work. I can do a testing on this.

[geoffkizer]

It seems really weird that the port handling is screwed up for IPv6 but not IPv4. Do we understand this? Do we have an issue with IPv4 as well?

(The hostname handling doesn't surprise me that much...)

[caesar-chen]

I found out setting up environment and do IPv6 multi machine testing cost me more time, and I'm blocked on this approach.

I think a better next step for me is that: testing the behavior on .NET Framework & WinHttpHandler about the port/host name handling for IPv6 and IPv4, to see if there is a difference with the SocketsHttpHandler. If Framework and WinHttpHandler behave the same, we will need to fix the SocketsHttpHandler.

[geoffkizer]

It seems like you can test this with a fake proxy, same as you're doing here: dotnet/corefx#28578 (review)

[geoffkizer]

BTW it looks to me like we're not sending the port in the IPv4 case either. This is bad.

[geoffkizer]

In the proxy case, the Host header and the authority part of the request Uri should be the same. We should be able to use the same code for both.

[geoffkizer]

We should add a bunch of tests here for various combinations -- DNS hostname, IPv4 hostname, IPv6 hostname, default port/non default port, secure/nonsecure.

Also interesting to check how we handle if you specify the default port explicitly, e.g. RequestUri = "http://foo.com:80/" or "https://foo.com:443/". I believe that SocketHttpHandler will strip off the port if it's the default, which I believe is correct. I'm curious what other handlers do...

[caesar-chen]

Tested on Framework, the behavior is the same as WinHttpHandler:

IPv6 testing:

On Framework & WinHttpHandler:

No port provided:
In value: List<String> ["GET http://[::1234]/ HTTP/1.1", "Host: [::1234]", "Proxy-Connection: Keep-Alive"]

With port:
In value: List<String> ["GET http://[::1234]:8080/ HTTP/1.1", "Host: [::1234]:8080", "Proxy-Connection: Keep-Alive"]

On SocketsHttpHandler:

No port provided:
In value: List<String> ["GET http://::1234/ HTTP/1.1", "Host: [::1234]"]

With port:
In value: List<String> ["GET http://::1234/ HTTP/1.1", "Host: [::1234]:8080"]

Summary:

1. [] is missing in request line.
2. port number is missing.
3. Proxy-Connection: Keep-Alive is missing.

IPv4 testing:

On Framework & WinHttpHandler:

No port provided:
In value: List<String> ["GET http://321.123.456.789/ HTTP/1.1", "Proxy-Connection: Keep-Alive", "Host: 321.123.456.789"]

With port:
List<String> ["GET http://321.123.456.789:8080/ HTTP/1.1", "Proxy-Connection: Keep-Alive", "Host: 321.123.456.789:8080"]

On SocketsHttpHandler:

No port provided:
In value: List<String> ["GET http://321.123.456.789/ HTTP/1.1", "Host: 321.123.456.789"]

With port:
In value: List<String> ["GET http://321.123.456.789/ HTTP/1.1", "Host: 321.123.456.789:8080"]

Summary:

1. port number is missing.
2. Proxy-Connection: Keep-Alive is missing.

[caesar-chen]

BTW it looks to me like we're not sending the port in the IPv4 case either. This is bad.

Yes, the port is not sent in IPv4 case. We should fix it.

[geoffkizer]

Let's fix the IPv6 handling and the port handling. Let's file a separate issue on Proxy-Connection since that's a different issue.

[caesar-chen]

Let's fix the IPv6 handling and the port handling.

Will do.

Let's file a separate issue on Proxy-Connection since that's a different issue.

Opened dotnet/corefx#28708 to track it.