carterkozak commented on a change in pull request #274:
URL:
https://github.com/apache/httpcomponents-core/pull/274#discussion_r601541841
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -292,6 +292,8 @@ private String buildString() {
}
if (InetAddressUtils.isIPv6Address(this.host)) {
sb.append("[").append(this.host).append("]");
+ } else if
(InetAddressUtils.isIPv6URLBracketedAddress(this.host)) {
Review comment:
This is an interesting point, and I may not have understood enough of
https://tools.ietf.org/html/rfc3986 to get this right.
Based on a strict interpretation of the RFC, couldn't `new
URIBuilder().setHost("::1").build()` also result in hostname `%3A%3A1`? At some
level we need to distinguish IP address inputs from hostnames, in much the same
way certificates differentiate SAN values between IP addresses and DNS.
I worry about the case in which users use a URIBuilder with data from a java
URI object. When ipv6 is used, the `uri.getHost()` method will return a
bracketed ip address. I don't think it will be obvious that `new
URIBuilder().setScheme("https").setHost(uri.getHost()).build()` will produce a
percent-encoded hostname rather than ipv6. I don't think we need to design the
URIBuilder to reproduce the java URI's quirks, but anything we can do to help
developers avoid unintentional failures will be worthwhile.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
