Copilot commented on code in PR #6456:
URL: https://github.com/apache/hbase/pull/6456#discussion_r2282979848
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/ServerName.java:
##########
@@ -327,7 +336,49 @@ public static ServerName parseVersionedServerName(final
byte[] versionedBytes) {
* @return A ServerName instance.
*/
public static ServerName parseServerName(final String str) {
- return SERVERNAME_PATTERN.matcher(str).matches() ? valueOf(str) :
valueOf(str, NON_STARTCODE);
+ ServerName sn =
+ SERVERNAME_PATTERN.matcher(str).matches() ? valueOf(str) : valueOf(str,
NON_STARTCODE);
+ String hostname = sn.getHostname();
+ // if IPV6 address is in encoded format, need to check and validate its
address length
+ // eg: if address is like
"0%3a0%3a0%3a0%3a0%3a0%3a0%3a0%3a1,16020,1720673488765" decode to
+ // "0:0:0:0:0:0:0:1,16020,1720673488765"
+ if (isIpv6ServerName(hostname, COLON_ENCODED_VALUE)) {
+ try {
+ hostname = URLDecoder.decode(sn.getHostname(), "UTF8");
Review Comment:
The charset name 'UTF8' should be 'UTF-8' with a hyphen for proper charset
specification according to IANA standards.
```suggestion
hostname = URLDecoder.decode(sn.getHostname(), "UTF-8");
```
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/ServerName.java:
##########
@@ -327,7 +336,49 @@ public static ServerName parseVersionedServerName(final
byte[] versionedBytes) {
* @return A ServerName instance.
*/
public static ServerName parseServerName(final String str) {
- return SERVERNAME_PATTERN.matcher(str).matches() ? valueOf(str) :
valueOf(str, NON_STARTCODE);
+ ServerName sn =
+ SERVERNAME_PATTERN.matcher(str).matches() ? valueOf(str) : valueOf(str,
NON_STARTCODE);
+ String hostname = sn.getHostname();
+ // if IPV6 address is in encoded format, need to check and validate its
address length
+ // eg: if address is like
"0%3a0%3a0%3a0%3a0%3a0%3a0%3a0%3a1,16020,1720673488765" decode to
+ // "0:0:0:0:0:0:0:1,16020,1720673488765"
+ if (isIpv6ServerName(hostname, COLON_ENCODED_VALUE)) {
+ try {
+ hostname = URLDecoder.decode(sn.getHostname(), "UTF8");
+ return ServerName.valueOf(hostname, sn.getPort(), sn.getStartCode());
+ } catch (UnsupportedEncodingException e) {
+ throw new IllegalArgumentException("Exception occurred while decoding
server name", e);
+ }
+ }
+ return sn;
+ }
+
+ /**
+ * Verify the ServerAddress is in IPV6 format or not
+ * @param serverAddress IP address
+ * @param addressSeparator IPV6 address separator
+ * @return true if server address is in IPV6 format
+ */
+ public static boolean isIpv6ServerName(String serverAddress, String
addressSeparator) {
+ return serverAddress.contains(addressSeparator)
+ && serverAddress.split(addressSeparator).length ==
IPV6_SPLIT_COLON_LENGTH;
+ }
+
+ /**
+ * Encode the Server Address
+ * @param str Either an instance of {@link #toString()} or a
"'<hostname>' ':'
+ * '<port>'".
+ * @return ServerName instance
+ */
+ public static ServerName getEncodedServerName(final String str) {
+ ServerName sn =
+ SERVERNAME_PATTERN.matcher(str).matches() ? valueOf(str) : valueOf(str,
NON_STARTCODE);
+ try {
+ return ServerName.valueOf(URLEncoder.encode(sn.getHostname(), "UTF8"),
sn.getPort(),
Review Comment:
The charset name 'UTF8' should be 'UTF-8' with a hyphen for proper charset
specification according to IANA standards.
```suggestion
return ServerName.valueOf(URLEncoder.encode(sn.getHostname(),
"UTF-8"), sn.getPort(),
```
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/ServerName.java:
##########
@@ -62,6 +65,12 @@ public class ServerName implements Comparable<ServerName>,
Serializable {
*/
private static final short VERSION = 0;
static final byte[] VERSION_BYTES = Bytes.toBytes(VERSION);
+ private static final String COLON_ENCODED_VALUE = "%3a";
+
+ public static final String COLON = ":";
+
+ // IPV6 address length separated by COLON(:), eg:
"0:0:0:0:0:0:0:1".split(colon).length
+ private static final int IPV6_SPLIT_COLON_LENGTH = 8;
Review Comment:
The magic number 8 should be documented more clearly. IPv6 addresses can
have compressed notation (::) which would result in fewer than 8 segments. This
validation may incorrectly reject valid IPv6 addresses.
```suggestion
// Use a standard library method to validate IPv6 addresses, as compressed
notation (::) can result in fewer than 8 segments.
```
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/ServerName.java:
##########
@@ -327,7 +336,49 @@ public static ServerName parseVersionedServerName(final
byte[] versionedBytes) {
* @return A ServerName instance.
*/
public static ServerName parseServerName(final String str) {
- return SERVERNAME_PATTERN.matcher(str).matches() ? valueOf(str) :
valueOf(str, NON_STARTCODE);
+ ServerName sn =
+ SERVERNAME_PATTERN.matcher(str).matches() ? valueOf(str) : valueOf(str,
NON_STARTCODE);
+ String hostname = sn.getHostname();
+ // if IPV6 address is in encoded format, need to check and validate its
address length
+ // eg: if address is like
"0%3a0%3a0%3a0%3a0%3a0%3a0%3a0%3a1,16020,1720673488765" decode to
+ // "0:0:0:0:0:0:0:1,16020,1720673488765"
+ if (isIpv6ServerName(hostname, COLON_ENCODED_VALUE)) {
+ try {
+ hostname = URLDecoder.decode(sn.getHostname(), "UTF8");
+ return ServerName.valueOf(hostname, sn.getPort(), sn.getStartCode());
+ } catch (UnsupportedEncodingException e) {
+ throw new IllegalArgumentException("Exception occurred while decoding
server name", e);
+ }
+ }
+ return sn;
+ }
+
+ /**
+ * Verify the ServerAddress is in IPV6 format or not
+ * @param serverAddress IP address
+ * @param addressSeparator IPV6 address separator
+ * @return true if server address is in IPV6 format
+ */
+ public static boolean isIpv6ServerName(String serverAddress, String
addressSeparator) {
+ return serverAddress.contains(addressSeparator)
+ && serverAddress.split(addressSeparator).length ==
IPV6_SPLIT_COLON_LENGTH;
Review Comment:
The split() method creates a regex Pattern internally on each call, which is
inefficient. Consider using a pre-compiled Pattern or
StringUtils.countMatches() for better performance in IPv6 detection.
```suggestion
if (!serverAddress.contains(addressSeparator)) {
return false;
}
int count;
if (":".equals(addressSeparator)) {
// Use pre-compiled Splitter for colon
count = COLON_SPLITTER.splitToList(serverAddress).size();
} else {
// Fallback for encoded separator, count occurrences manually
int idx = 0, c = 0;
while ((idx = serverAddress.indexOf(addressSeparator, idx)) != -1) {
c++;
idx += addressSeparator.length();
}
count = c + 1;
}
return count == IPV6_SPLIT_COLON_LENGTH;
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
