fix client SNI handling

[tobyxdd]

The current code assumes that a hostname containing a colon is in the host:port format, but it could actually be an IPv6 address. Calling SplitHostPort in this case returns an error, and right now the Client will fail to initialize.

It is possible and not extremely rare to have a TLS cert with IPv6 SAN(s). Real-life examples include Cloudflare's https://[2606:4700:4700::1111]/ and some users of my project using self-signed CAs to issue certs for their IPv6 addresses.

[marten-seemann]

What if the host doesn't contain any port? net.SplitHostPort will error in that case.

[codecov]

Codecov Report

Base: 85.48% // Head: 85.50% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (1f646f8) compared to base (df762b7).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3613      +/-   ##
==========================================
+ Coverage   85.48%   85.50%   +0.02%     
==========================================
  Files         141      141              
  Lines       10296    10293       -3     
==========================================
- Hits         8801     8800       -1     
+ Misses       1109     1108       -1     
+ Partials      386      385       -1     

Impacted Files	Coverage Δ
client.go	80.00% <100.00%> (+1.09%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[tobyxdd]

What if the host doesn't contain any port? net.SplitHostPort will error in that case.

My change is to handle this situation exactly. An IPv6 address (without a port) contains colons but is not in the host:port format. Instead of checking whether the address has a port by the presence of a colon, we should feed it to net.SplitHostPort to see if it returns an error.

[marten-seemann]

My change is to handle this situation exactly. An IPv6 address (without a port) contains colons but is not in the host:port format. Instead of checking whether the address has a port by the presence of a colon, we should feed it to net.SplitHostPort to see if it returns an error.

Got it, that makes sense. I missed that the if err != nil doesn't return. Can you add a comment why we're not returning (i.e. could be a hostname or an IP address without a port number)?