michael-o commented on a change in pull request #274:
URL:
https://github.com/apache/httpcomponents-core/pull/274#discussion_r607035068
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -333,7 +333,12 @@ private void digestURI(final URI uri, final Charset
charset) {
this.scheme = uri.getScheme();
this.encodedSchemeSpecificPart = uri.getRawSchemeSpecificPart();
this.encodedAuthority = uri.getRawAuthority();
- this.host = uri.getHost();
+ final String uriHost = uri.getHost();
+ // URI.getHost incorrectly returns bracketed (encoded) IPv6 values.
Brackets are an
+ // encoding detail of the URI and not part of the host string.
+ this.host = uriHost != null &&
InetAddressUtils.isIPv6URLBracketedAddress(uriHost)
Review comment:
Very nice!
##########
File path: httpcore5/src/test/java/org/apache/hc/core5/net/TestURIBuilder.java
##########
@@ -813,4 +813,32 @@ public void testNormalizeSyntax() throws Exception {
new
URIBuilder("http:///../../.././";).normalizeSyntax().build().toASCIIString());
}
+ @Test
+ public void testIpv6Host() throws Exception {
+ final URI uri = new URIBuilder("https://[::1]:432/path";).build();
+ Assert.assertEquals(432, uri.getPort());
+ Assert.assertEquals("https", uri.getScheme());
+ Assert.assertEquals("[::1]", uri.getHost());
+ Assert.assertEquals("/path", uri.getPath());
+ }
+
+ @Test
+ public void testIpv6HostWithPortUpdate() throws Exception {
+ // Updating the port clears URIBuilder.encodedSchemeSpecificPart
+ // and bypasses the fast/simple path which preserves input.
+ final URI uri = new
URIBuilder("https://[::1]:432/path";).setPort(123).build();
+ Assert.assertEquals(123, uri.getPort());
+ Assert.assertEquals("https", uri.getScheme());
+ Assert.assertEquals("[::1]", uri.getHost());
Review comment:
See my comment above.
##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -333,7 +333,12 @@ private void digestURI(final URI uri, final Charset
charset) {
this.scheme = uri.getScheme();
this.encodedSchemeSpecificPart = uri.getRawSchemeSpecificPart();
this.encodedAuthority = uri.getRawAuthority();
- this.host = uri.getHost();
+ final String uriHost = uri.getHost();
+ // URI.getHost incorrectly returns bracketed (encoded) IPv6 values.
Brackets are an
+ // encoding detail of the URI and not part of the host string.
+ this.host = uriHost != null &&
InetAddressUtils.isIPv6URLBracketedAddress(uriHost)
Review comment:
Question: Should `#getHost()` contain information that is unwraps IPv6
addresses?
--
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]
