From 8777229a008acf3f339623d5b829f8cc2acf193d Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Tue, 1 Nov 2022 14:07:18 -0700 Subject: [PATCH 1/9] HostAndPort to support parsing ip+port string Motivation: Parsing HostAndPort from 'ip+port' string is a common case, but we don't have tooling to support this. --- .../transport/api/DefaultHostAndPort.java | 197 +++++++++++++- .../transport/api/HostAndPort.java | 26 ++ .../transport/api/HostAndPortTest.java | 241 ++++++++++++++++++ 3 files changed, 463 insertions(+), 1 deletion(-) create mode 100644 servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java diff --git a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java index 0fc0a46a60..39c1c5f101 100644 --- a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java +++ b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java @@ -15,13 +15,23 @@ */ package io.servicetalk.transport.api; +import static java.lang.Integer.parseInt; import static java.util.Objects.requireNonNull; /** * A default immutable implementation of {@link HostAndPort}. */ final class DefaultHostAndPort implements HostAndPort { + /** + * {@code xxx.xxx.xxx.xxx:yyyyy} + */ + private static final int MAX_IPV4_LEN = 21; + /** + * {@code [xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx]:yyyyy} = 47 chars w/out zone id + */ + private static final int MAX_IPV6_LEN = 47 + 12 /* some limit for zone id length */; private final String hostName; + private final String toString; private final int port; /** @@ -30,8 +40,71 @@ final class DefaultHostAndPort implements HostAndPort { * @param port the port. */ DefaultHostAndPort(String hostName, int port) { + this(hostName, port, isIPv6(hostName) ? '[' + hostName + "]:" + port : hostName + ':' + port); + } + + private DefaultHostAndPort(String hostName, int port, String toString) { this.hostName = requireNonNull(hostName); this.port = port; + this.toString = requireNonNull(toString); + } + + /** + * Parse IPv4 {@code xxx.xxx.xxx.xxx:yyyyy} and IPv6 {@code [xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx]:yyyyy} style + * addresses. + * @param ipPort An IPv4 {@code xxx.xxx.xxx.xxx:yyyyy} or IPv6 + * {@code [xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx]:yyyyy} addresses. + * @param startIndex The index at which the address parsing starts. + * @return A {@link HostAndPort} where the hostname is the IP address and the port is parsed from the string. + */ + static HostAndPort parseFromIpPort(String ipPort, int startIndex) { + String inetAddress; + final boolean isv6; + int i; + if (ipPort.charAt(startIndex) == '[') { // check if ipv6 + if (ipPort.length() - startIndex > MAX_IPV6_LEN) { + throw new IllegalArgumentException("Invalid IPv6 address: " + ipPort); + } + i = ipPort.indexOf(']'); + if (i <= startIndex) { + throw new IllegalArgumentException("unable to find end ']' of IPv6 address: " + ipPort); + } + inetAddress = ipPort.substring(startIndex + 1, i); + ++i; + isv6 = true; + if (i >= ipPort.length()) { + throw new IllegalArgumentException("no port found after ']' of IPv6 address: " + ipPort); + } else if (ipPort.charAt(i) != ':') { + throw new IllegalArgumentException("':' expected after ']' for IPv6 address: " + ipPort); + } + } else { + if (ipPort.length() - startIndex > MAX_IPV4_LEN) { + throw new IllegalArgumentException("Invalid IPv4 address: " + ipPort); + } + i = ipPort.lastIndexOf(':'); + if (i < 0) { + throw new IllegalArgumentException("no port found: " + ipPort); + } + inetAddress = ipPort.substring(startIndex, i); + isv6 = false; + } + + final int port; + try { + port = parseInt(ipPort.substring(i + 1)); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("invalid port " + ipPort, e); + } + if (!isValidPort(port) || ipPort.charAt(i + 1) == '+') { // parseInt allows '+' but we don't want this + throw new IllegalArgumentException("invalid port " + ipPort); + } + + if (isv6) { + inetAddress = compressIPv6(inetAddress); + return new DefaultHostAndPort(inetAddress, port, '[' + inetAddress + "]:" + port); + } + validateIPv4(inetAddress, 0, inetAddress.length()); + return new DefaultHostAndPort(inetAddress, port); } @Override @@ -46,7 +119,7 @@ public int port() { @Override public String toString() { - return hostName + ':' + port; + return toString; } @Override @@ -62,4 +135,126 @@ public boolean equals(Object o) { public int hashCode() { return 31 * (31 + port) + hostName.hashCode(); } + + private static boolean isValidPort(int port) { + return port >= 0 && port <= 65535; + } + + private static String compressIPv6(String rawIp) { + if (rawIp.isEmpty()) { + throw new IllegalArgumentException("Empty IP"); + } + // https://datatracker.ietf.org/doc/html/rfc5952#section-2 + // JDK doesn't do IPv6 compression, or remove leading 0s. This may lead to inconsistent String representation + // which will yield different hash-codes and equals comparisons to fail when it shouldn't. + int longestZerosCount = 0; + int longestZerosBegin = -1; + int longestZerosEnd = -1; + int zerosCount = 0; + int zerosBegin = rawIp.charAt(0) != '0' ? -1 : 0; + int zerosEnd = -1; + int lastColon = -1; + boolean isCompressed = false; + char prevChar = '\0'; + StringBuilder compressedIPv6Builder = new StringBuilder(rawIp.length()); + for (int i = 0; i < rawIp.length(); ++i) { + final char c = rawIp.charAt(i); + switch (c) { + case '0': + if (zerosBegin < 0 || i == rawIp.length() - 1) { + compressedIPv6Builder.append('0'); + } + break; + case ':': + if (prevChar == ':') { + isCompressed = true; + compressedIPv6Builder.append(':'); + } else if (zerosBegin >= 0) { + ++zerosCount; + compressedIPv6Builder.append("0:"); + zerosEnd = compressedIPv6Builder.length(); + } else { + compressedIPv6Builder.append(':'); + zerosBegin = compressedIPv6Builder.length(); + } + lastColon = i; + break; + default: + if (!isValidateIPv6Digit(c)) { + boolean shouldInsertRemainder = false; + if (c == '.') { // check for IPv4 mapped + final int zoneIdIndex = rawIp.lastIndexOf('%'); + validateIPv4(rawIp, lastColon + 1, zoneIdIndex < 0 ? rawIp.length() : zoneIdIndex); + shouldInsertRemainder = true; + } else if (c == '%') { // check for IPv6 zone id + // no constraints enforced on zone id + shouldInsertRemainder = true; + } + if (shouldInsertRemainder) { + compressedIPv6Builder.append(c); + ++i; + for (; i < rawIp.length(); ++i) { + compressedIPv6Builder.append(rawIp.charAt(i)); + } + break; + } else { + throw new IllegalArgumentException("Invalid IPv6 address[" + i + "]=" + c); + } + } + // https://datatracker.ietf.org/doc/html/rfc5952#section-4.2.3 + // if there is a tie in the longest length, we must choose the first to compress. + if (zerosEnd > 0 && zerosCount > longestZerosCount) { + longestZerosCount = zerosCount; + longestZerosBegin = zerosBegin; + longestZerosEnd = zerosEnd; + } + zerosBegin = zerosEnd = -1; + zerosCount = 0; + compressedIPv6Builder.append(c); + break; + } + prevChar = c; + } + // https://datatracker.ietf.org/doc/html/rfc5952#section-4.2.2 + // The symbol "::" MUST NOT be used to shorten just one 16-bit 0 field. + if (!isCompressed && longestZerosBegin >= 0 && longestZerosCount > 1) { + compressedIPv6Builder.replace(longestZerosBegin, longestZerosEnd, longestZerosBegin == 0 ? "::" : ":"); + } + return compressedIPv6Builder.toString(); + } + + private static boolean isValidateIPv6Digit(char c) { + return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F'); + } + + private static void validateIPv4(String inetAddress, int startIndex, int endIndex) { + int nibbles = 0; + int digits = 0; + for (int i = startIndex; i < endIndex; ++i) { + final char currChar = inetAddress.charAt(i); + if (currChar >= '0' && currChar <= '9') { + if (++digits > 3) { + throw new IllegalArgumentException("No more than 3 digits per section expected"); + } + } else if (currChar == '.') { + digits = 0; + if (++nibbles > 3) { + throw new IllegalArgumentException("No more than 3 IP section separators expected"); + } + } else { + throw new IllegalArgumentException("Unexpected character in IPv4 address[" + i + "]=" + currChar ); + } + } + if (nibbles != 3) { + throw new IllegalArgumentException("3 IP section separators expected, found " + nibbles); + } + } + + private static boolean isIPv6(String address) { + if (address.isEmpty()) { + return false; + } + char firstChar = address.charAt(0); + return firstChar == '[' || firstChar == ':' || address.indexOf(':', 1) >= 0; + } } diff --git a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/HostAndPort.java b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/HostAndPort.java index 95a0b96ec3..92be0b8f52 100644 --- a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/HostAndPort.java +++ b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/HostAndPort.java @@ -15,6 +15,8 @@ */ package io.servicetalk.transport.api; +import io.servicetalk.buffer.api.CharSequences; + import java.net.InetSocketAddress; /** @@ -57,4 +59,28 @@ static HostAndPort of(String host, int port) { static HostAndPort of(InetSocketAddress address) { return new DefaultHostAndPort(address.getHostString(), address.getPort()); } + + /** + * Parse IPv4 {@code xxx.xxx.xxx.xxx:yyyyy} and IPv6 {@code [xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx]:yyyyy} style + * addresses. + * @param ipPort An IPv4 {@code xxx.xxx.xxx.xxx:yyyyy} or IPv6 + * {@code [xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx]:yyyyy} addresses. + * @return A {@link HostAndPort} where the hostname is the IP address and the port is parsed from the string. + * @see #ofIpPort(String, int) + */ + static HostAndPort ofIpPort(String ipPort) { + return DefaultHostAndPort.parseFromIpPort(ipPort, 0); + } + + /** + * Parse IPv4 {@code xxx.xxx.xxx.xxx:yyyyy} and IPv6 {@code [xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx]:yyyyy} style + * addresses. + * @param ipPort An IPv4 {@code xxx.xxx.xxx.xxx:yyyyy} or IPv6 + * {@code [xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx]:yyyyy} addresses. + * @param startIndex The index at which the address parsing starts. + * @return A {@link HostAndPort} where the hostname is the IP address and the port is parsed from the string. + */ + static HostAndPort ofIpPort(String ipPort, int startIndex) { + return DefaultHostAndPort.parseFromIpPort(ipPort, startIndex); + } } diff --git a/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java b/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java new file mode 100644 index 0000000000..0dcba00071 --- /dev/null +++ b/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java @@ -0,0 +1,241 @@ +/* + * Copyright © 2022 Apple Inc. and the ServiceTalk project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.servicetalk.transport.api; + +import org.junit.jupiter.api.Test; + +import java.net.Inet6Address; +import java.net.InetSocketAddress; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.jupiter.api.Assertions.assertThrows; + +final class HostAndPortTest { + @Test + public void IPv6LoopBack() { + assertIP("[::1]:9999", "::1", 9999); + } + + @Test + public void IPv6Compressed() { + assertIP("[2001:1234::1b12:0:0:1a13]:0", "2001:1234::1b12:0:0:1a13", 0); + } + + @Test + public void IPv6MappedIPv4() { + assertIP("[::FFFF:129.144.52.38]:443", "::FFFF:129.144.52.38", 443); + } + + @Test + public void IPv6WithScope() { + assertIP("[::FFFF:129.144.52.38%2]:65535", "::FFFF:129.144.52.38%2", 65535); + } + + @Test + public void IPv4() { + assertIP("1.2.3.4:8080", "1.2.3.4", 8080); + } + + @Test + public void IPv4MissingComponents() { + assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3:80", "1.2.3", 80)); + } + + @Test + public void IPv4NoAddress() { + assertThrows(IllegalArgumentException.class, () -> assertIP(":80", "", 80)); + } + + @Test + public void IPv4NoPort() { + assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3.4", "1.2.3.4", 0)); + } + + @Test + public void IPv6NoPort() { + assertThrows(IllegalArgumentException.class, () -> assertIP("[::1]", "[::1]", 0)); + } + + @Test + public void IPv6NoAddress() { + assertThrows(IllegalArgumentException.class, () -> assertIP("[]:80", "[]", 80)); + } + + @Test + public void IPv4NegativePort() { + assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3.4:-22", "1.2.3.4", 0)); + } + + @Test + public void IPv6NegativePort() { + assertThrows(IllegalArgumentException.class, () -> assertIP("[::1]-1", "[::1]", 0)); + } + + @Test + public void IPv4PlusPort() { + assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3.4:+22", "1.2.3.4", 0)); + } + + @Test + public void IPv6PlusPort() { + assertThrows(IllegalArgumentException.class, () -> assertIP("[::1]:+22", "[::1]", 0)); + } + + @Test + public void IPv4PortTooHigh() { + assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3.4:65536", "1.2.3.4", 0)); + } + + @Test + public void IPv6PortTooHigh() { + assertThrows(IllegalArgumentException.class, () -> assertIP("[::1]:65536", "[::1]", 0)); + } + + @Test + public void IPv4PortTooLow() { + assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3.4:-1", "1.2.3.4", 0)); + } + + @Test + public void IPv6PortTooLow() { + assertThrows(IllegalArgumentException.class, () -> assertIP("[::1]:-3", "[::1]", 0)); + } + + @Test + public void IPv4PortInvalidChar() { + assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3.4:12x", "1.2.3.4", 0)); + } + + @Test + public void IPv6PortInvalidChar() { + assertThrows(IllegalArgumentException.class, () -> assertIP("[::1]:x", "[::1]", 0)); + } + + @Test + public void IPv6HalfBracketFirst() { + assertThrows(IllegalArgumentException.class, () -> assertIP("::1]:80", "[::1]", 80)); + } + + @Test + public void IPv6HalfBracketLast() { + assertThrows(IllegalArgumentException.class, () -> assertIP("[::1:80", "[::1]", 80)); + } + + @Test + public void IPv6DoubleBracketFirst() { + assertThrows(IllegalArgumentException.class, () -> assertIP("[[::1]:80", "[::1]", 80)); + } + + @Test + public void IPv6DoubleBracketLast() { + assertThrows(IllegalArgumentException.class, () -> assertIP("[::1]]:80", "[::1]", 80)); + } + + @Test + public void IPv6ChineseChar() { + assertThrows(IllegalArgumentException.class, () -> assertIP("[\u4F60\u597D]:8080", "", 8080)); + } + + @Test + public void IPv4ChineseChar() { + assertThrows(IllegalArgumentException.class, () -> assertIP("\u4F60.2.3.4:8080", "", 8080)); + } + + @Test + public void IPv6UTF8() { + assertThrows(IllegalArgumentException.class, () -> assertIP("[::❤]:8080", "", 8080)); + } + + @Test + public void IPv4UTF8() { + assertThrows(IllegalArgumentException.class, () -> assertIP("1.❤.3.4:8080", "", 8080)); + } + + @Test + public void IPv6Latin1AsUTF8() { + assertThrows(IllegalArgumentException.class, () -> assertIP("[::ö]:8080", "", 8080)); + } + + @Test + public void IPv4Latin1AsUTF8() { + assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.ö.4:8080", "", 8080)); + } + + @Test + public void IPv6CompressAndRemoveLeadingZeros() { + assertIP("[0000:0000:0000:0000:0000:0000:0000:0001]:234", "::1", 234); + } + + @Test + public void IPv6CompressAndRemoveLeadingZerosPreserveLastZero() { + assertIP("[1000:0200:0030:0004:0000:0000:0050:0000]:234", "1000:200:30:4::50:0", 234); + } + + @Test + public void IPv6CompressAndRemoveLeadingZeroTwo() { + assertIP("[00:000:0030:0004:0001:2000:0050:0000]:234", "::30:4:1:2000:50:0", 234); + } + + @Test + public void IPv6RemoveLeadingZerosEvenIfAlreadyCompressed() { + // https://datatracker.ietf.org/doc/html/rfc5952#section-4.2.1 + assertIP("[2001:0db8::0001]:1234", "2001:db8::1", 1234); + } + + @Test + public void IPv6NoCompressOneBitField() { + // https://datatracker.ietf.org/doc/html/rfc5952#section-4.2.2 + assertIP("[2001:db8:0:1:1:1:1:1]:1234", "2001:db8:0:1:1:1:1:1", 1234); + assertIP("[2001:db8:0000:1:1:1:1:1]:1234", "2001:db8:0:1:1:1:1:1", 1234); + assertIP("[0000:0db8:0000:01:01:01:01:01]:1234", "0:db8:0:1:1:1:1:1", 1234); + } + + @Test + public void IPv6CompressLongestStreak() { + // https://datatracker.ietf.org/doc/html/rfc5952#section-4.2.3 + assertIP("[2001:0:0:1:0:0:0:1]:1234", "2001:0:0:1::1", 1234); + } + + @Test + public void IPv6CompressFirstIfTie() { + // https://datatracker.ietf.org/doc/html/rfc5952#section-4.2.3 + assertIP("[2001:db8:0:0:1:0:0:1]:1234", "2001:db8::1:0:0:1", 1234); + } + + private static void assertIP(String ipPort, String expectedAddress, int expectedPort) { + assertIP(HostAndPort.ofIpPort(ipPort), expectedAddress, expectedPort); + + for (int i = 1; i <= 3; ++i) { + StringBuilder prefix = new StringBuilder(i); + for (int x = 0; x < i; ++x) { + prefix.append('a'); + } + assertIP(HostAndPort.ofIpPort(prefix + ipPort, prefix.length()), expectedAddress, expectedPort); + } + } + + private static void assertIP(HostAndPort result, String expectedAddress, int expectedPort) { + assertThat(result.hostName(), equalTo(expectedAddress)); + assertThat(result.port(), equalTo(expectedPort)); + InetSocketAddress socketAddress = new InetSocketAddress(expectedAddress, expectedPort); + if (socketAddress.getAddress() instanceof Inet6Address || expectedAddress.startsWith("::FFFF:")) { + assertThat(result.toString(), equalTo('[' + expectedAddress + "]:" + expectedPort)); + } else { + assertThat(result.toString(), equalTo(expectedAddress + ':' + expectedPort)); + } + } +} From 30f89e7f5de942e0e231f9fa7ae4f4528e36eb16 Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Tue, 1 Nov 2022 16:48:52 -0700 Subject: [PATCH 2/9] checkstyle --- .../gradle/checkstyle/suppressions.xml | 25 +++++++++++++++++++ .../transport/api/DefaultHostAndPort.java | 2 +- .../transport/api/HostAndPort.java | 2 -- 3 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 servicetalk-transport-api/gradle/checkstyle/suppressions.xml diff --git a/servicetalk-transport-api/gradle/checkstyle/suppressions.xml b/servicetalk-transport-api/gradle/checkstyle/suppressions.xml new file mode 100644 index 0000000000..a23d9450c7 --- /dev/null +++ b/servicetalk-transport-api/gradle/checkstyle/suppressions.xml @@ -0,0 +1,25 @@ + + + + + + + + diff --git a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java index 39c1c5f101..2b049d21c0 100644 --- a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java +++ b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java @@ -242,7 +242,7 @@ private static void validateIPv4(String inetAddress, int startIndex, int endInde throw new IllegalArgumentException("No more than 3 IP section separators expected"); } } else { - throw new IllegalArgumentException("Unexpected character in IPv4 address[" + i + "]=" + currChar ); + throw new IllegalArgumentException("Unexpected character in IPv4 address[" + i + "]=" + currChar); } } if (nibbles != 3) { diff --git a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/HostAndPort.java b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/HostAndPort.java index 92be0b8f52..443f7c6d27 100644 --- a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/HostAndPort.java +++ b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/HostAndPort.java @@ -15,8 +15,6 @@ */ package io.servicetalk.transport.api; -import io.servicetalk.buffer.api.CharSequences; - import java.net.InetSocketAddress; /** From 700942546ff2d9384ea071de05e3f167e85f84e0 Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Tue, 1 Nov 2022 18:32:47 -0700 Subject: [PATCH 3/9] review comments --- .../gradle/checkstyle/suppressions.xml | 25 ----- .../transport/api/DefaultHostAndPort.java | 96 +++++-------------- .../transport/api/HostAndPort.java | 4 +- .../transport/api/HostAndPortTest.java | 5 + .../utils/internal/NetworkUtils.java | 8 ++ 5 files changed, 42 insertions(+), 96 deletions(-) delete mode 100644 servicetalk-transport-api/gradle/checkstyle/suppressions.xml diff --git a/servicetalk-transport-api/gradle/checkstyle/suppressions.xml b/servicetalk-transport-api/gradle/checkstyle/suppressions.xml deleted file mode 100644 index a23d9450c7..0000000000 --- a/servicetalk-transport-api/gradle/checkstyle/suppressions.xml +++ /dev/null @@ -1,25 +0,0 @@ - - - - - - - - diff --git a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java index 2b049d21c0..e365890c7d 100644 --- a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java +++ b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java @@ -15,6 +15,8 @@ */ package io.servicetalk.transport.api; +import static io.servicetalk.utils.internal.NetworkUtils.isValidIpV4Address; +import static io.servicetalk.utils.internal.NetworkUtils.isValidIpV6Address; import static java.lang.Integer.parseInt; import static java.util.Objects.requireNonNull; @@ -40,7 +42,11 @@ final class DefaultHostAndPort implements HostAndPort { * @param port the port. */ DefaultHostAndPort(String hostName, int port) { - this(hostName, port, isIPv6(hostName) ? '[' + hostName + "]:" + port : hostName + ':' + port); + this(hostName, port, isValidIpV6Address(hostName)); + } + + DefaultHostAndPort(String hostName, int port, boolean isIPv6) { + this(hostName, port, isIPv6 ? '[' + hostName + "]:" + port : hostName + ':' + port); } private DefaultHostAndPort(String hostName, int port, String toString) { @@ -63,27 +69,30 @@ static HostAndPort parseFromIpPort(String ipPort, int startIndex) { int i; if (ipPort.charAt(startIndex) == '[') { // check if ipv6 if (ipPort.length() - startIndex > MAX_IPV6_LEN) { - throw new IllegalArgumentException("Invalid IPv6 address: " + ipPort); + throw new IllegalArgumentException("Invalid IPv6 address: " + ipPort.substring(startIndex)); } i = ipPort.indexOf(']'); if (i <= startIndex) { - throw new IllegalArgumentException("unable to find end ']' of IPv6 address: " + ipPort); + throw new IllegalArgumentException("unable to find end ']' of IPv6 address: " + + ipPort.substring(startIndex)); } inetAddress = ipPort.substring(startIndex + 1, i); ++i; isv6 = true; if (i >= ipPort.length()) { - throw new IllegalArgumentException("no port found after ']' of IPv6 address: " + ipPort); + throw new IllegalArgumentException("no port found after ']' of IPv6 address: " + + ipPort.substring(startIndex)); } else if (ipPort.charAt(i) != ':') { - throw new IllegalArgumentException("':' expected after ']' for IPv6 address: " + ipPort); + throw new IllegalArgumentException("':' expected after ']' for IPv6 address: " + + ipPort.substring(startIndex)); } } else { if (ipPort.length() - startIndex > MAX_IPV4_LEN) { - throw new IllegalArgumentException("Invalid IPv4 address: " + ipPort); + throw new IllegalArgumentException("Invalid IPv4 address: " + ipPort.substring(startIndex)); } i = ipPort.lastIndexOf(':'); if (i < 0) { - throw new IllegalArgumentException("no port found: " + ipPort); + throw new IllegalArgumentException("no port found: " + ipPort.substring(startIndex)); } inetAddress = ipPort.substring(startIndex, i); isv6 = false; @@ -93,18 +102,23 @@ static HostAndPort parseFromIpPort(String ipPort, int startIndex) { try { port = parseInt(ipPort.substring(i + 1)); } catch (NumberFormatException e) { - throw new IllegalArgumentException("invalid port " + ipPort, e); + throw new IllegalArgumentException("invalid port " + ipPort.substring(startIndex), e); } if (!isValidPort(port) || ipPort.charAt(i + 1) == '+') { // parseInt allows '+' but we don't want this - throw new IllegalArgumentException("invalid port " + ipPort); + throw new IllegalArgumentException("invalid port " + ipPort.substring(startIndex)); } if (isv6) { inetAddress = compressIPv6(inetAddress); + if (!isValidIpV6Address(inetAddress)) { + throw new IllegalArgumentException("Invalid IPv6 address: " + inetAddress); + } return new DefaultHostAndPort(inetAddress, port, '[' + inetAddress + "]:" + port); } - validateIPv4(inetAddress, 0, inetAddress.length()); - return new DefaultHostAndPort(inetAddress, port); + if (!isValidIpV4Address(inetAddress)) { + throw new IllegalArgumentException("Invalid IPv4 address: " + inetAddress); + } + return new DefaultHostAndPort(inetAddress, port, false); } @Override @@ -142,7 +156,7 @@ private static boolean isValidPort(int port) { private static String compressIPv6(String rawIp) { if (rawIp.isEmpty()) { - throw new IllegalArgumentException("Empty IP"); + throw new IllegalArgumentException("Empty IPv6 address"); } // https://datatracker.ietf.org/doc/html/rfc5952#section-2 // JDK doesn't do IPv6 compression, or remove leading 0s. This may lead to inconsistent String representation @@ -153,7 +167,6 @@ private static String compressIPv6(String rawIp) { int zerosCount = 0; int zerosBegin = rawIp.charAt(0) != '0' ? -1 : 0; int zerosEnd = -1; - int lastColon = -1; boolean isCompressed = false; char prevChar = '\0'; StringBuilder compressedIPv6Builder = new StringBuilder(rawIp.length()); @@ -177,30 +190,8 @@ private static String compressIPv6(String rawIp) { compressedIPv6Builder.append(':'); zerosBegin = compressedIPv6Builder.length(); } - lastColon = i; break; default: - if (!isValidateIPv6Digit(c)) { - boolean shouldInsertRemainder = false; - if (c == '.') { // check for IPv4 mapped - final int zoneIdIndex = rawIp.lastIndexOf('%'); - validateIPv4(rawIp, lastColon + 1, zoneIdIndex < 0 ? rawIp.length() : zoneIdIndex); - shouldInsertRemainder = true; - } else if (c == '%') { // check for IPv6 zone id - // no constraints enforced on zone id - shouldInsertRemainder = true; - } - if (shouldInsertRemainder) { - compressedIPv6Builder.append(c); - ++i; - for (; i < rawIp.length(); ++i) { - compressedIPv6Builder.append(rawIp.charAt(i)); - } - break; - } else { - throw new IllegalArgumentException("Invalid IPv6 address[" + i + "]=" + c); - } - } // https://datatracker.ietf.org/doc/html/rfc5952#section-4.2.3 // if there is a tie in the longest length, we must choose the first to compress. if (zerosEnd > 0 && zerosCount > longestZerosCount) { @@ -222,39 +213,4 @@ private static String compressIPv6(String rawIp) { } return compressedIPv6Builder.toString(); } - - private static boolean isValidateIPv6Digit(char c) { - return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F'); - } - - private static void validateIPv4(String inetAddress, int startIndex, int endIndex) { - int nibbles = 0; - int digits = 0; - for (int i = startIndex; i < endIndex; ++i) { - final char currChar = inetAddress.charAt(i); - if (currChar >= '0' && currChar <= '9') { - if (++digits > 3) { - throw new IllegalArgumentException("No more than 3 digits per section expected"); - } - } else if (currChar == '.') { - digits = 0; - if (++nibbles > 3) { - throw new IllegalArgumentException("No more than 3 IP section separators expected"); - } - } else { - throw new IllegalArgumentException("Unexpected character in IPv4 address[" + i + "]=" + currChar); - } - } - if (nibbles != 3) { - throw new IllegalArgumentException("3 IP section separators expected, found " + nibbles); - } - } - - private static boolean isIPv6(String address) { - if (address.isEmpty()) { - return false; - } - char firstChar = address.charAt(0); - return firstChar == '[' || firstChar == ':' || address.indexOf(':', 1) >= 0; - } } diff --git a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/HostAndPort.java b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/HostAndPort.java index 443f7c6d27..4a2ccd1574 100644 --- a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/HostAndPort.java +++ b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/HostAndPort.java @@ -15,6 +15,7 @@ */ package io.servicetalk.transport.api; +import java.net.Inet6Address; import java.net.InetSocketAddress; /** @@ -55,7 +56,8 @@ static HostAndPort of(String host, int port) { * @return the {@link HostAndPort}. */ static HostAndPort of(InetSocketAddress address) { - return new DefaultHostAndPort(address.getHostString(), address.getPort()); + return new DefaultHostAndPort(address.getHostString(), address.getPort(), + address.getAddress() instanceof Inet6Address); } /** diff --git a/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java b/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java index 0dcba00071..0343687569 100644 --- a/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java +++ b/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java @@ -75,6 +75,11 @@ public void IPv6NoAddress() { assertThrows(IllegalArgumentException.class, () -> assertIP("[]:80", "[]", 80)); } + @Test + public void IPv6SingleCharAddress() { + assertThrows(IllegalArgumentException.class, () -> assertIP("[a]:80", "[a]", 80)); + } + @Test public void IPv4NegativePort() { assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3.4:-22", "1.2.3.4", 0)); diff --git a/servicetalk-utils-internal/src/main/java/io/servicetalk/utils/internal/NetworkUtils.java b/servicetalk-utils-internal/src/main/java/io/servicetalk/utils/internal/NetworkUtils.java index 8b801126df..98345bfd7b 100644 --- a/servicetalk-utils-internal/src/main/java/io/servicetalk/utils/internal/NetworkUtils.java +++ b/servicetalk-utils-internal/src/main/java/io/servicetalk/utils/internal/NetworkUtils.java @@ -55,6 +55,14 @@ public static boolean isValidIpV4Address(final CharSequence ip) { return isValidIpV4Address(ip, 0, ip.length()); } + /** + * Takes a string and parses it to see if it is a valid IPV4 address. + * + * @param ip the IP-address to validate + * @param from the index in {@code ip} to start validation (inclusive). + * @param toExclusive the index in {@code ip} to end validation (exclusive). + * @return true, if the string represents an IPV4 address in dotted notation, false otherwise. + */ private static boolean isValidIpV4Address(final CharSequence ip, int from, int toExclusive) { int len = toExclusive - from; int i; From 27e0d8a7ec2b1f3d850bf6b06df6044e6ccd33e2 Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Tue, 1 Nov 2022 18:35:35 -0700 Subject: [PATCH 4/9] validate port --- .../transport/api/DefaultHostAndPort.java | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java index e365890c7d..b004b976d1 100644 --- a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java +++ b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java @@ -42,17 +42,13 @@ final class DefaultHostAndPort implements HostAndPort { * @param port the port. */ DefaultHostAndPort(String hostName, int port) { - this(hostName, port, isValidIpV6Address(hostName)); + this(hostName, validatePort(port), isValidIpV6Address(hostName)); } DefaultHostAndPort(String hostName, int port, boolean isIPv6) { - this(hostName, port, isIPv6 ? '[' + hostName + "]:" + port : hostName + ':' + port); - } - - private DefaultHostAndPort(String hostName, int port, String toString) { this.hostName = requireNonNull(hostName); this.port = port; - this.toString = requireNonNull(toString); + this.toString = isIPv6 ? '[' + hostName + "]:" + port : hostName + ':' + port; } /** @@ -113,7 +109,7 @@ static HostAndPort parseFromIpPort(String ipPort, int startIndex) { if (!isValidIpV6Address(inetAddress)) { throw new IllegalArgumentException("Invalid IPv6 address: " + inetAddress); } - return new DefaultHostAndPort(inetAddress, port, '[' + inetAddress + "]:" + port); + return new DefaultHostAndPort(inetAddress, port, true); } if (!isValidIpV4Address(inetAddress)) { throw new IllegalArgumentException("Invalid IPv4 address: " + inetAddress); @@ -154,6 +150,13 @@ private static boolean isValidPort(int port) { return port >= 0 && port <= 65535; } + private static int validatePort(int port) { + if (!isValidPort(port)) { + throw new IllegalArgumentException("Invalid port: " + port); + } + return port; + } + private static String compressIPv6(String rawIp) { if (rawIp.isEmpty()) { throw new IllegalArgumentException("Empty IPv6 address"); From c7293477766311c36d899a8ca1819f5d8ac2b9b5 Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Tue, 1 Nov 2022 19:19:41 -0700 Subject: [PATCH 5/9] revert port validation to avoid behavior change --- .../io/servicetalk/transport/api/DefaultHostAndPort.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java index b004b976d1..3e1fcfc030 100644 --- a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java +++ b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java @@ -42,7 +42,7 @@ final class DefaultHostAndPort implements HostAndPort { * @param port the port. */ DefaultHostAndPort(String hostName, int port) { - this(hostName, validatePort(port), isValidIpV6Address(hostName)); + this(hostName, port, isValidIpV6Address(hostName)); } DefaultHostAndPort(String hostName, int port, boolean isIPv6) { @@ -150,13 +150,6 @@ private static boolean isValidPort(int port) { return port >= 0 && port <= 65535; } - private static int validatePort(int port) { - if (!isValidPort(port)) { - throw new IllegalArgumentException("Invalid port: " + port); - } - return port; - } - private static String compressIPv6(String rawIp) { if (rawIp.isEmpty()) { throw new IllegalArgumentException("Empty IPv6 address"); From 326ce16f16a9c443d2af4b1a96f4303d348bd00c Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Tue, 1 Nov 2022 19:54:27 -0700 Subject: [PATCH 6/9] pmd --- .../transport/api/HostAndPortTest.java | 77 ++++++++++--------- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java b/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java index 0343687569..468b785aab 100644 --- a/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java +++ b/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java @@ -24,185 +24,186 @@ import static org.hamcrest.Matchers.equalTo; import static org.junit.jupiter.api.Assertions.assertThrows; +@SuppressWarnings("PMD.AvoidUsingHardCodedIP") final class HostAndPortTest { @Test - public void IPv6LoopBack() { + void IPv6LoopBack() { assertIP("[::1]:9999", "::1", 9999); } @Test - public void IPv6Compressed() { + void IPv6Compressed() { assertIP("[2001:1234::1b12:0:0:1a13]:0", "2001:1234::1b12:0:0:1a13", 0); } @Test - public void IPv6MappedIPv4() { + void IPv6MappedIPv4() { assertIP("[::FFFF:129.144.52.38]:443", "::FFFF:129.144.52.38", 443); } @Test - public void IPv6WithScope() { + void IPv6WithScope() { assertIP("[::FFFF:129.144.52.38%2]:65535", "::FFFF:129.144.52.38%2", 65535); } @Test - public void IPv4() { + void IPv4() { assertIP("1.2.3.4:8080", "1.2.3.4", 8080); } @Test - public void IPv4MissingComponents() { + void IPv4MissingComponents() { assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3:80", "1.2.3", 80)); } @Test - public void IPv4NoAddress() { + void IPv4NoAddress() { assertThrows(IllegalArgumentException.class, () -> assertIP(":80", "", 80)); } @Test - public void IPv4NoPort() { + void IPv4NoPort() { assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3.4", "1.2.3.4", 0)); } @Test - public void IPv6NoPort() { + void IPv6NoPort() { assertThrows(IllegalArgumentException.class, () -> assertIP("[::1]", "[::1]", 0)); } @Test - public void IPv6NoAddress() { + void IPv6NoAddress() { assertThrows(IllegalArgumentException.class, () -> assertIP("[]:80", "[]", 80)); } @Test - public void IPv6SingleCharAddress() { + void IPv6SingleCharAddress() { assertThrows(IllegalArgumentException.class, () -> assertIP("[a]:80", "[a]", 80)); } @Test - public void IPv4NegativePort() { + void IPv4NegativePort() { assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3.4:-22", "1.2.3.4", 0)); } @Test - public void IPv6NegativePort() { + void IPv6NegativePort() { assertThrows(IllegalArgumentException.class, () -> assertIP("[::1]-1", "[::1]", 0)); } @Test - public void IPv4PlusPort() { + void IPv4PlusPort() { assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3.4:+22", "1.2.3.4", 0)); } @Test - public void IPv6PlusPort() { + void IPv6PlusPort() { assertThrows(IllegalArgumentException.class, () -> assertIP("[::1]:+22", "[::1]", 0)); } @Test - public void IPv4PortTooHigh() { + void IPv4PortTooHigh() { assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3.4:65536", "1.2.3.4", 0)); } @Test - public void IPv6PortTooHigh() { + void IPv6PortTooHigh() { assertThrows(IllegalArgumentException.class, () -> assertIP("[::1]:65536", "[::1]", 0)); } @Test - public void IPv4PortTooLow() { + void IPv4PortTooLow() { assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3.4:-1", "1.2.3.4", 0)); } @Test - public void IPv6PortTooLow() { + void IPv6PortTooLow() { assertThrows(IllegalArgumentException.class, () -> assertIP("[::1]:-3", "[::1]", 0)); } @Test - public void IPv4PortInvalidChar() { + void IPv4PortInvalidChar() { assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.3.4:12x", "1.2.3.4", 0)); } @Test - public void IPv6PortInvalidChar() { + void IPv6PortInvalidChar() { assertThrows(IllegalArgumentException.class, () -> assertIP("[::1]:x", "[::1]", 0)); } @Test - public void IPv6HalfBracketFirst() { + void IPv6HalfBracketFirst() { assertThrows(IllegalArgumentException.class, () -> assertIP("::1]:80", "[::1]", 80)); } @Test - public void IPv6HalfBracketLast() { + void IPv6HalfBracketLast() { assertThrows(IllegalArgumentException.class, () -> assertIP("[::1:80", "[::1]", 80)); } @Test - public void IPv6DoubleBracketFirst() { + void IPv6DoubleBracketFirst() { assertThrows(IllegalArgumentException.class, () -> assertIP("[[::1]:80", "[::1]", 80)); } @Test - public void IPv6DoubleBracketLast() { + void IPv6DoubleBracketLast() { assertThrows(IllegalArgumentException.class, () -> assertIP("[::1]]:80", "[::1]", 80)); } @Test - public void IPv6ChineseChar() { + void IPv6ChineseChar() { assertThrows(IllegalArgumentException.class, () -> assertIP("[\u4F60\u597D]:8080", "", 8080)); } @Test - public void IPv4ChineseChar() { + void IPv4ChineseChar() { assertThrows(IllegalArgumentException.class, () -> assertIP("\u4F60.2.3.4:8080", "", 8080)); } @Test - public void IPv6UTF8() { + void IPv6UTF8() { assertThrows(IllegalArgumentException.class, () -> assertIP("[::❤]:8080", "", 8080)); } @Test - public void IPv4UTF8() { + void IPv4UTF8() { assertThrows(IllegalArgumentException.class, () -> assertIP("1.❤.3.4:8080", "", 8080)); } @Test - public void IPv6Latin1AsUTF8() { + void IPv6Latin1AsUTF8() { assertThrows(IllegalArgumentException.class, () -> assertIP("[::ö]:8080", "", 8080)); } @Test - public void IPv4Latin1AsUTF8() { + void IPv4Latin1AsUTF8() { assertThrows(IllegalArgumentException.class, () -> assertIP("1.2.ö.4:8080", "", 8080)); } @Test - public void IPv6CompressAndRemoveLeadingZeros() { + void IPv6CompressAndRemoveLeadingZeros() { assertIP("[0000:0000:0000:0000:0000:0000:0000:0001]:234", "::1", 234); } @Test - public void IPv6CompressAndRemoveLeadingZerosPreserveLastZero() { + void IPv6CompressAndRemoveLeadingZerosPreserveLastZero() { assertIP("[1000:0200:0030:0004:0000:0000:0050:0000]:234", "1000:200:30:4::50:0", 234); } @Test - public void IPv6CompressAndRemoveLeadingZeroTwo() { + void IPv6CompressAndRemoveLeadingZeroTwo() { assertIP("[00:000:0030:0004:0001:2000:0050:0000]:234", "::30:4:1:2000:50:0", 234); } @Test - public void IPv6RemoveLeadingZerosEvenIfAlreadyCompressed() { + void IPv6RemoveLeadingZerosEvenIfAlreadyCompressed() { // https://datatracker.ietf.org/doc/html/rfc5952#section-4.2.1 assertIP("[2001:0db8::0001]:1234", "2001:db8::1", 1234); } @Test - public void IPv6NoCompressOneBitField() { + void IPv6NoCompressOneBitField() { // https://datatracker.ietf.org/doc/html/rfc5952#section-4.2.2 assertIP("[2001:db8:0:1:1:1:1:1]:1234", "2001:db8:0:1:1:1:1:1", 1234); assertIP("[2001:db8:0000:1:1:1:1:1]:1234", "2001:db8:0:1:1:1:1:1", 1234); @@ -210,13 +211,13 @@ public void IPv6NoCompressOneBitField() { } @Test - public void IPv6CompressLongestStreak() { + void IPv6CompressLongestStreak() { // https://datatracker.ietf.org/doc/html/rfc5952#section-4.2.3 assertIP("[2001:0:0:1:0:0:0:1]:1234", "2001:0:0:1::1", 1234); } @Test - public void IPv6CompressFirstIfTie() { + void IPv6CompressFirstIfTie() { // https://datatracker.ietf.org/doc/html/rfc5952#section-4.2.3 assertIP("[2001:db8:0:0:1:0:0:1]:1234", "2001:db8::1:0:0:1", 1234); } From 05a6f63683d395b40f689196ccab0da86ffe1981 Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Tue, 1 Nov 2022 20:05:03 -0700 Subject: [PATCH 7/9] lazy string creation --- .../transport/api/DefaultHostAndPort.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java index 3e1fcfc030..719ef23a80 100644 --- a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java +++ b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java @@ -15,6 +15,8 @@ */ package io.servicetalk.transport.api; +import javax.annotation.Nullable; + import static io.servicetalk.utils.internal.NetworkUtils.isValidIpV4Address; import static io.servicetalk.utils.internal.NetworkUtils.isValidIpV6Address; import static java.lang.Integer.parseInt; @@ -32,8 +34,10 @@ final class DefaultHostAndPort implements HostAndPort { * {@code [xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx]:yyyyy} = 47 chars w/out zone id */ private static final int MAX_IPV6_LEN = 47 + 12 /* some limit for zone id length */; + private static final String STR_IPV6 = "_ipv6_"; private final String hostName; - private final String toString; + @Nullable + private String toString; private final int port; /** @@ -48,7 +52,7 @@ final class DefaultHostAndPort implements HostAndPort { DefaultHostAndPort(String hostName, int port, boolean isIPv6) { this.hostName = requireNonNull(hostName); this.port = port; - this.toString = isIPv6 ? '[' + hostName + "]:" + port : hostName + ':' + port; + this.toString = isIPv6 ? STR_IPV6 : null; } /** @@ -129,7 +133,13 @@ public int port() { @Override public String toString() { - return toString; + String str = toString; + if (str == null) { + toString = str = hostName + ':' + port; + } else if (str == STR_IPV6) { + toString = str = '[' + hostName + "]:" + port; + } + return str; } @Override From 0899f10337d9357abce53c0a519c42abd2f58fbf Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Wed, 2 Nov 2022 12:52:45 -0700 Subject: [PATCH 8/9] Normalize ipv6 for regular constructor --- .../api/AbstractHttpRequestMetaDataTest.java | 11 +++++++-- .../transport/api/DefaultHostAndPort.java | 23 ++++++++++++------- .../transport/api/HostAndPortTest.java | 18 +++++++++++++++ 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/servicetalk-http-api/src/test/java/io/servicetalk/http/api/AbstractHttpRequestMetaDataTest.java b/servicetalk-http-api/src/test/java/io/servicetalk/http/api/AbstractHttpRequestMetaDataTest.java index fbc95956a7..9c7b6854f1 100644 --- a/servicetalk-http-api/src/test/java/io/servicetalk/http/api/AbstractHttpRequestMetaDataTest.java +++ b/servicetalk-http-api/src/test/java/io/servicetalk/http/api/AbstractHttpRequestMetaDataTest.java @@ -29,6 +29,7 @@ import static io.servicetalk.http.api.HttpHeaderNames.AUTHORIZATION; import static io.servicetalk.http.api.HttpHeaderNames.HOST; import static io.servicetalk.http.api.HttpRequestMethod.CONNECT; +import static io.servicetalk.utils.internal.NetworkUtils.isValidIpV6Address; import static java.lang.System.lineSeparator; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Arrays.asList; @@ -39,6 +40,7 @@ import static java.util.Spliterators.spliteratorUnknownSize; import static java.util.stream.Collectors.toList; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.lessThan; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -208,19 +210,21 @@ void testParseHttpUriAbsoluteFormWithHost() { } @Test + @SuppressWarnings("PMD.AvoidUsingHardCodedIP") void testEffectiveHostIPv6NoPort() { createFixture("some/path?foo=bar&abc=def&foo=baz"); fixture.headers().set(HOST, "[1:2:3::5]"); - assertEffectiveHostAndPort("[1:2:3::5]"); + assertEffectiveHostAndPort("1:2:3::5"); } @Test + @SuppressWarnings("PMD.AvoidUsingHardCodedIP") void testEffectiveHostIPv6WithPort() { createFixture("some/path?foo=bar&abc=def&foo=baz"); fixture.headers().set(HOST, "[1:2:3::5]:8080"); - assertEffectiveHostAndPort("[1:2:3::5]", 8080); + assertEffectiveHostAndPort("1:2:3::5", 8080); } @Test @@ -839,6 +843,9 @@ private void assertEffectiveHostAndPort(String hostName, int port) { assertNotNull(effectiveHostAndPort); assertEquals(hostName, effectiveHostAndPort.hostName()); assertEquals(port, effectiveHostAndPort.port()); + assertThat(effectiveHostAndPort.toString(), isValidIpV6Address(hostName) ? + equalTo('[' + hostName + "]:" + port) : + equalTo(hostName + ':' + port)); } @SuppressWarnings("unchecked") diff --git a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java index 719ef23a80..bcc736ec06 100644 --- a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java +++ b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java @@ -46,7 +46,14 @@ final class DefaultHostAndPort implements HostAndPort { * @param port the port. */ DefaultHostAndPort(String hostName, int port) { - this(hostName, port, isValidIpV6Address(hostName)); + if (isValidIpV6Address(requireNonNull(hostName))) { // Normalize ipv6 so equals/hashCode works as expected + this.hostName = hostName.charAt(0) == '[' ? + compressIPv6(hostName, 1, hostName.length() - 1) : compressIPv6(hostName, 0, hostName.length()); + this.toString = STR_IPV6; + } else { + this.hostName = hostName; + } + this.port = port; } DefaultHostAndPort(String hostName, int port, boolean isIPv6) { @@ -109,7 +116,7 @@ static HostAndPort parseFromIpPort(String ipPort, int startIndex) { } if (isv6) { - inetAddress = compressIPv6(inetAddress); + inetAddress = compressIPv6(inetAddress, 0, inetAddress.length()); if (!isValidIpV6Address(inetAddress)) { throw new IllegalArgumentException("Invalid IPv6 address: " + inetAddress); } @@ -160,8 +167,8 @@ private static boolean isValidPort(int port) { return port >= 0 && port <= 65535; } - private static String compressIPv6(String rawIp) { - if (rawIp.isEmpty()) { + private static String compressIPv6(String rawIp, int start, int end) { + if (end - start <= 0) { throw new IllegalArgumentException("Empty IPv6 address"); } // https://datatracker.ietf.org/doc/html/rfc5952#section-2 @@ -171,16 +178,16 @@ private static String compressIPv6(String rawIp) { int longestZerosBegin = -1; int longestZerosEnd = -1; int zerosCount = 0; - int zerosBegin = rawIp.charAt(0) != '0' ? -1 : 0; + int zerosBegin = rawIp.charAt(start) != '0' ? -1 : 0; int zerosEnd = -1; boolean isCompressed = false; char prevChar = '\0'; - StringBuilder compressedIPv6Builder = new StringBuilder(rawIp.length()); - for (int i = 0; i < rawIp.length(); ++i) { + StringBuilder compressedIPv6Builder = new StringBuilder(end - start); + for (int i = start; i < end; ++i) { final char c = rawIp.charAt(i); switch (c) { case '0': - if (zerosBegin < 0 || i == rawIp.length() - 1) { + if (zerosBegin < 0 || i == end - 1) { compressedIPv6Builder.append('0'); } break; diff --git a/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java b/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java index 468b785aab..9e549d109e 100644 --- a/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java +++ b/servicetalk-transport-api/src/test/java/io/servicetalk/transport/api/HostAndPortTest.java @@ -26,6 +26,18 @@ @SuppressWarnings("PMD.AvoidUsingHardCodedIP") final class HostAndPortTest { + @Test + void hostConstructorNormalizesIpv6() { + HostAndPort hp1 = HostAndPort.of("[::1]", 9999); + HostAndPort hp2 = HostAndPort.of("::1", 9999); + HostAndPort hp3 = HostAndPort.of("[0000:0000:0000:0000:0000:0000:0000:0001]", 9999); + HostAndPort hp4 = HostAndPort.of("0000:0000:0000:0000:0000:0000:0000:0001", 9999); + + assertHpEqualTo(hp1, hp2); + assertHpEqualTo(hp2, hp3); + assertHpEqualTo(hp3, hp4); + } + @Test void IPv6LoopBack() { assertIP("[::1]:9999", "::1", 9999); @@ -244,4 +256,10 @@ private static void assertIP(HostAndPort result, String expectedAddress, int exp assertThat(result.toString(), equalTo(expectedAddress + ':' + expectedPort)); } } + + private static void assertHpEqualTo(HostAndPort hp1, HostAndPort hp2) { + assertThat(hp1, equalTo(hp2)); + assertThat(hp1.hashCode(), equalTo(hp2.hashCode())); + assertThat(hp1.toString(), equalTo(hp2.toString())); + } } From aed29e93a37efc9931e47aeffe13f5a9eb8cb3e0 Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Thu, 3 Nov 2022 16:52:37 -0700 Subject: [PATCH 9/9] validate first --- .../java/io/servicetalk/transport/api/DefaultHostAndPort.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java index bcc736ec06..bef75231bf 100644 --- a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java +++ b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/DefaultHostAndPort.java @@ -116,10 +116,10 @@ static HostAndPort parseFromIpPort(String ipPort, int startIndex) { } if (isv6) { - inetAddress = compressIPv6(inetAddress, 0, inetAddress.length()); if (!isValidIpV6Address(inetAddress)) { throw new IllegalArgumentException("Invalid IPv6 address: " + inetAddress); } + inetAddress = compressIPv6(inetAddress, 0, inetAddress.length()); return new DefaultHostAndPort(inetAddress, port, true); } if (!isValidIpV4Address(inetAddress)) {