Copilot commented on code in PR #8821:
URL: https://github.com/apache/hadoop/pull/8821#discussion_r4269067634
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SecurityUtil.java:
##########
@@ -243,13 +244,25 @@ private static String[] getComponents(String
principalConfig) {
private static String replacePattern(String[] components, String hostname)
throws IOException {
String fqdn = hostname;
- if (fqdn == null || fqdn.isEmpty() || fqdn.equals("0.0.0.0")) {
+ if (fqdn == null || fqdn.isEmpty() || isWildcardAddress(fqdn)) {
fqdn = getLocalHostName(null);
}
return components[0] + "/" +
StringUtils.toLowerCase(fqdn) + "@" + components[2];
}
+ /**
+ * Checks whether a host is the IPv4 or IPv6 wildcard address, in any
+ * spelling and with or without the brackets used around IPv6 literals.
+ */
+ private static boolean isWildcardAddress(String host) {
+ String address = host.startsWith("[") && host.endsWith("]")
+ ? host.substring(1, host.length() - 1) : host;
+ // A scoped address is never the wildcard, and parsing it can fail.
+ return address.indexOf('%') < 0 && InetAddresses.isInetAddress(address)
+ && InetAddresses.forString(address).isAnyLocalAddress();
Review Comment:
`InetAddresses.isInetAddress(address)` and
`InetAddresses.forString(address)` both parse/validate the literal, which is
redundant. Consider parsing once (after the '%' check) and then calling
`isAnyLocalAddress()` on the parsed `InetAddress`, or otherwise avoiding double
parsing to reduce overhead and keep the logic simpler.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SecurityUtil.java:
##########
@@ -180,8 +180,9 @@ protected static boolean isOriginalTGT(KerberosTicket
ticket) {
/**
* Convert Kerberos principal name pattern to valid Kerberos principal
* names. It replaces hostname pattern with hostname, which should be
- * fully-qualified domain name. If hostname is null or "0.0.0.0", it uses
- * dynamically looked-up fqdn of the current host instead.
+ * fully-qualified domain name. If hostname is null or a wildcard address
+ * such as "0.0.0.0" or "::", it uses dynamically looked-up fqdn of the
+ * current host instead.
Review Comment:
The updated Javadoc mentions `"::"` but not the bracketed wildcard form
(`"[::]"`) that is explicitly supported (and tested). Consider explicitly
mentioning bracketed IPv6 literals too, since this method can receive host
strings from configurations/URIs that include brackets.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
