carterkozak commented on a change in pull request #274:
URL:
https://github.com/apache/httpcomponents-core/pull/274#discussion_r603389603
##########
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:
What do we expect in this case?
```java
@Test
public void testBuilderWithBracketedIpv6Host() throws Exception {
URI firstUri = URI.create("https://[2001:db8::1428:57ab]:123/first";);
URI secondUri = URI.create("https://[::1]:456/second";);
final URI uri = new
URIBuilder(firstUri).setHost(secondUri.getHost()).build();
Assert.assertEquals("https", uri.getScheme());
Assert.assertEquals(123, uri.getPort()); // Current implementation
results in -1
Assert.assertEquals("[::1]", uri.getHost()); // Current
implementation results in null
Assert.assertEquals("/first", uri.getPath());
}
```
Currently it's "garbage in, garbage out" based on the `secondUri` which is a
valid URI unfortunately returning `[::1]` from `secondUri.getHost()`. While we
certainly shouldn't require inputs to be encoded already, I think we may wish
to either allow inputs to `setHost(String)` to be bracketed and strip the
brackets, or throw an exception to avoid passing unexpected garbage to the
networking layer. My preference is to normalize `[::1]` to `::1` given that
URIBuilder interacts directly with the incorrect `URI` type.
I've push an implementation without any special handling of
`setHost(bracketedIpv6String)`, what do you think is best in this case?
--
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]
