anmolnar commented on code in PR #2005:
URL: https://github.com/apache/zookeeper/pull/2005#discussion_r1225379186


##########
zookeeper-server/src/main/java/org/apache/zookeeper/common/NetUtils.java:
##########
@@ -27,17 +27,18 @@
  */
 public class NetUtils {
 
+    /**
+     * Prefer using the hostname for formatting, but without requesting 
reverse DNS lookup.
+     * Fall back to IP address if hostname is unavailable and use [] brackets 
for IPv6 literal.
+     */
     public static String formatInetAddr(InetSocketAddress addr) {
+        String hostString = addr.getHostString();
         InetAddress ia = addr.getAddress();
 
-        if (ia == null) {
-            return String.format("%s:%s", addr.getHostString(), 
addr.getPort());
-        }
-
-        if (ia instanceof Inet6Address) {
-            return String.format("[%s]:%s", ia.getHostAddress(), 
addr.getPort());
+        if (ia instanceof Inet6Address && hostString.contains(":")) {

Review Comment:
   Good point, need to check the source, but javadoc says:
   
   > Returns the hostname, or the String form of the address if it doesn't have 
a hostname (it was created using a literal). This has the benefit of not 
attempting a reverse lookup.
   
   
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/InetSocketAddress.html#getHostString()
   
   How could be null? No hostname and no IP address either in the 
InetSocketAddress. What sort of socket address does it represent in such case?
   
   I think we're good without the null check, but I'll take a look at the 
source.



-- 
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]

Reply via email to