format addresses correctly so I can actually read what comes out of our logs and stats apis

core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java

@@ -360,16 +360,16 @@ public class DiscoveryNode implements Streamable, ToXContent {
     public String toString() {
         StringBuilder sb = new StringBuilder();
         if (nodeName.length() > 0) {
-            sb.append('[').append(nodeName).append(']');
+            sb.append('{').append(nodeName).append('}');
         }
         if (nodeId != null) {
-            sb.append('[').append(nodeId).append(']');
+            sb.append('{').append(nodeId).append('}');
         }
         if (Strings.hasLength(hostName)) {
-            sb.append('[').append(hostName).append(']');
+            sb.append('{').append(hostName).append('}');
         }
         if (address != null) {
-            sb.append('[').append(address).append(']');
+            sb.append('{').append(address).append('}');
         }
         if (!attributes.isEmpty()) {
             sb.append(attributes);

core/src/main/java/org/elasticsearch/common/logging/Loggers.java

@@ -84,6 +84,7 @@ public class Loggers {
         }
     }
 
+    @SuppressForbidden(reason = "do not know what this method does")
     public static ESLogger getLogger(String loggerName, Settings settings, String... prefixes) {
         List<String> prefixesList = newArrayList();
         if (settings.getAsBoolean("logger.logHostAddress", false)) {

core/src/main/java/org/elasticsearch/common/network/IfConfig.java

@@ -114,22 +114,22 @@ final class IfConfig {
         InetAddress address = interfaceAddress.getAddress();
         if (address instanceof Inet6Address) {
             sb.append("inet6 ");
-            sb.append(address.toString().substring(1));
+            sb.append(NetworkAddress.formatAddress(address));
             sb.append(" prefixlen:");
             sb.append(interfaceAddress.getNetworkPrefixLength());
         } else {
             sb.append("inet ");
-            sb.append(address.toString().substring(1));
+            sb.append(NetworkAddress.formatAddress(address));
             int netmask = 0xFFFFFFFF << (32 - interfaceAddress.getNetworkPrefixLength());
-            sb.append(" netmask:" + InetAddress.getByAddress(new byte[] {
+            sb.append(" netmask:" + NetworkAddress.formatAddress(InetAddress.getByAddress(new byte[] {
                     (byte)(netmask >>> 24), 
                     (byte)(netmask >>> 16 & 0xFF), 
                     (byte)(netmask >>> 8 & 0xFF), 
                     (byte)(netmask & 0xFF) 
-            }).toString().substring(1));
+            })));
             InetAddress broadcast = interfaceAddress.getBroadcast();
             if (broadcast != null) {
-                sb.append(" broadcast:" + broadcast.toString().substring(1));
+                sb.append(" broadcast:" + NetworkAddress.formatAddress(broadcast));
             }
         }
         if (address.isLoopbackAddress()) {

core/src/main/java/org/elasticsearch/common/network/NetworkAddress.java

@@ -0,0 +1,183 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you 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 org.elasticsearch.common.network;
+
+import com.google.common.net.InetAddresses;
+
+import org.elasticsearch.common.SuppressForbidden;
+
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.util.Objects;
+
+/** 
+ * Utility functions for presentation of network addresses.
+ * <p>
+ * Java's address formatting is particularly bad, every address
+ * has an optional host if its resolved, so IPv4 addresses often
+ * look like this (note the confusing leading slash):
+ * <pre>
+ *    {@code /127.0.0.1}
+ * </pre>
+ * IPv6 addresses are even worse, with no IPv6 address compression,
+ * and often containing things like numeric scopeids, which are even
+ * more confusing (e.g. not going to work in any user's browser, refer
+ * to an interface on <b>another</b> machine, etc):
+ * <pre>
+ *    {@code /0:0:0:0:0:0:0:1%1}
+ * </pre>
+ * This class provides sane address formatting instead, e.g. 
+ * {@code 127.0.0.1} and {@code ::1} respectively. No methods do reverse
+ * lookups.
+ */
+public final class NetworkAddress {
+    /** No instantiation */
+    private NetworkAddress() {}
+
+    /**
+     * Formats a network address (with optional host) for display purposes.
+     * <p>
+     * If the host is already resolved (typically because, we looked up
+     * a name to do that), then we include it, otherwise it is
+     * omitted. See {@link #formatAddress(InetAddress)} if you only
+     * want the address.
+     * <p>
+     * IPv6 addresses are compressed and without scope
+     * identifiers.
+     * <p>
+     * Example output with already-resolved hostnames:
+     * <ul>
+     *   <li>IPv4: {@code localhost/127.0.0.1}</li>
+     *   <li>IPv6: {@code localhost/::1}</li>
+     * </ul>
+     * <p>
+     * Example output with just an address:
+     * <ul>
+     *   <li>IPv4: {@code 127.0.0.1}</li>
+     *   <li>IPv6: {@code ::1}</li>
+     * </ul>
+     * @param address IPv4 or IPv6 address
+     * @return formatted string
+     * @see #formatAddress(InetAddress)
+     */
+    public static String format(InetAddress address) {
+        return format(address, -1, true);
+    }
+
+    /**
+     * Formats a network address and port for display purposes.
+     * <p>
+     * If the host is already resolved (typically because, we looked up
+     * a name to do that), then we include it, otherwise it is
+     * omitted. See {@link #formatAddress(InetSocketAddress)} if you only
+     * want the address.
+     * <p>
+     * This formats the address with {@link #format(InetAddress)}
+     * and appends the port number. IPv6 addresses will be bracketed.
+     * <p>
+     * Example output with already-resolved hostnames:
+     * <ul>
+     *   <li>IPv4: {@code localhost/127.0.0.1:9300}</li>
+     *   <li>IPv6: {@code localhost/[::1]:9300}</li>
+     * </ul>
+     * <p>
+     * Example output with just an address:
+     * <ul>
+     *   <li>IPv4: {@code 127.0.0.1:9300}</li>
+     *   <li>IPv6: {@code [::1]:9300}</li>
+     * </ul>
+     * @param address IPv4 or IPv6 address with port
+     * @return formatted string
+     * @see #formatAddress(InetSocketAddress)
+     */
+    public static String format(InetSocketAddress address) {
+        return format(address.getAddress(), address.getPort(), true);
+    }
+    
+    /**
+     * Formats a network address for display purposes.
+     * <p>
+     * This formats only the address, any hostname information,
+     * if present, is ignored. IPv6 addresses are compressed 
+     * and without scope identifiers.
+     * <p>
+     * Example output with just an address:
+     * <ul>
+     *   <li>IPv4: {@code 127.0.0.1}</li>
+     *   <li>IPv6: {@code ::1}</li>
+     * </ul>
+     * @param address IPv4 or IPv6 address
+     * @return formatted string
+     */
+    public static String formatAddress(InetAddress address) {
+        return format(address, -1, false);
+    }
+    
+    /**
+     * Formats a network address and port for display purposes.
+     * <p>
+     * This formats the address with {@link #formatAddress(InetAddress)}
+     * and appends the port number. IPv6 addresses will be bracketed.
+     * Any host information, if present is ignored.
+     * <p>
+     * Example output:
+     * <ul>
+     *   <li>IPv4: {@code 127.0.0.1:9300}</li>
+     *   <li>IPv6: {@code [::1]:9300}</li>
+     * </ul>
+     * @param address IPv4 or IPv6 address with port
+     * @return formatted string
+     */
+    public static String formatAddress(InetSocketAddress address) {
+        return format(address.getAddress(), address.getPort(), false);
+    }
+    
+    // note, we don't validate port, because we only allow InetSocketAddress
+    @SuppressForbidden(reason = "we call toString to avoid a DNS lookup")
+    static String format(InetAddress address, int port, boolean includeHost) {
+        Objects.requireNonNull(address);
+        
+        StringBuilder builder = new StringBuilder();
+
+        if (includeHost) {
+            // must use toString, to avoid DNS lookup. but the format is specified in the spec
+            String toString = address.toString();
+            int separator = toString.indexOf('/');
+            if (separator > 0) {
+                // append hostname, with the slash too
+                builder.append(toString, 0, separator + 1);
+            }
+        }
+                
+        if (port != -1 && address instanceof Inet6Address) {
+            builder.append(InetAddresses.toUriString(address));
+        } else {
+            builder.append(InetAddresses.toAddrString(address));
+        }
+        
+        if (port != -1) {
+            builder.append(':');
+            builder.append(port);
+        }
+        
+        return builder.toString();
+    }
+}

core/src/main/java/org/elasticsearch/common/transport/InetSocketTransportAddress.java

@@ -21,6 +21,7 @@ package org.elasticsearch.common.transport;
 
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.network.NetworkAddress;
 
 import java.io.IOException;
 import java.net.Inet6Address;
@@ -100,7 +101,7 @@ public final class InetSocketTransportAddress implements TransportAddress {
 
     @Override
     public String getAddress() {
-        return address.getAddress().getHostAddress();
+        return NetworkAddress.formatAddress(address.getAddress());
     }
 
     @Override
@@ -148,6 +149,6 @@ public final class InetSocketTransportAddress implements TransportAddress {
 
     @Override
     public String toString() {
-        return "inet[" + address + "]";
+        return NetworkAddress.format(address);
     }
 }

core/src/main/java/org/elasticsearch/http/netty/NettyHttpServerTransport.java

@@ -24,6 +24,7 @@ import org.elasticsearch.common.component.AbstractLifecycleComponent;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.netty.NettyUtils;
 import org.elasticsearch.common.netty.OpenChannelsHandler;
+import org.elasticsearch.common.network.NetworkAddress;
 import org.elasticsearch.common.network.NetworkService;
 import org.elasticsearch.common.network.NetworkUtils;
 import org.elasticsearch.common.settings.Settings;
@@ -274,7 +275,7 @@ public class NettyHttpServerTransport extends AbstractLifecycleComponent<HttpSer
     private void bindAddress(final InetAddress hostAddress) {
         PortsRange portsRange = new PortsRange(port);
         final AtomicReference<Exception> lastException = new AtomicReference<>();
-        final AtomicReference<SocketAddress> boundSocket = new AtomicReference<>();
+        final AtomicReference<InetSocketAddress> boundSocket = new AtomicReference<>();
         boolean success = portsRange.iterate(new PortsRange.PortCallback() {
             @Override
             public boolean onPortNumber(int portNumber) {
@@ -282,7 +283,7 @@ public class NettyHttpServerTransport extends AbstractLifecycleComponent<HttpSer
                     synchronized (serverChannels) {
                         Channel channel = serverBootstrap.bind(new InetSocketAddress(hostAddress, portNumber));
                         serverChannels.add(channel);
-                        boundSocket.set(channel.getLocalAddress());
+                        boundSocket.set((InetSocketAddress) channel.getLocalAddress());
                     }
                 } catch (Exception e) {
                     lastException.set(e);
@@ -294,7 +295,7 @@ public class NettyHttpServerTransport extends AbstractLifecycleComponent<HttpSer
         if (!success) {
             throw new BindHttpException("Failed to bind to [" + port + "]", lastException.get());
         }
-        logger.info("Bound http to address [{}]", boundSocket.get());
+        logger.info("Bound http to address {{}}", NetworkAddress.format(boundSocket.get()));
     }
 
     @Override

core/src/main/java/org/elasticsearch/transport/netty/NettyTransport.java

@@ -42,6 +42,7 @@ import org.elasticsearch.common.metrics.CounterMetric;
 import org.elasticsearch.common.netty.NettyUtils;
 import org.elasticsearch.common.netty.OpenChannelsHandler;
 import org.elasticsearch.common.netty.ReleaseChannelFutureListener;
+import org.elasticsearch.common.network.NetworkAddress;
 import org.elasticsearch.common.network.NetworkService;
 import org.elasticsearch.common.network.NetworkUtils;
 import org.elasticsearch.common.settings.Settings;
@@ -409,7 +410,11 @@ public class NettyTransport extends AbstractLifecycleComponent<Transport> implem
             throw new BindTransportException("Failed to resolve host [" + bindHost + "]", e);
         }
         if (logger.isDebugEnabled()) {
-            logger.debug("binding server bootstrap to: {}", hostAddresses);
+            String[] addresses = new String[hostAddresses.length];
+            for (int i = 0; i < hostAddresses.length; i++) {
+                addresses[i] = NetworkAddress.format(hostAddresses[i]);
+            }
+            logger.debug("binding server bootstrap to: {}", addresses);
         }
         for (InetAddress hostAddress : hostAddresses) {
             bindServerBootstrap(name, hostAddress, settings);
@@ -421,7 +426,7 @@ public class NettyTransport extends AbstractLifecycleComponent<Transport> implem
         String port = settings.get("port");
         PortsRange portsRange = new PortsRange(port);
         final AtomicReference<Exception> lastException = new AtomicReference<>();
-        final AtomicReference<SocketAddress> boundSocket = new AtomicReference<>();
+        final AtomicReference<InetSocketAddress> boundSocket = new AtomicReference<>();
         boolean success = portsRange.iterate(new PortsRange.PortCallback() {
             @Override
             public boolean onPortNumber(int portNumber) {
@@ -434,7 +439,7 @@ public class NettyTransport extends AbstractLifecycleComponent<Transport> implem
                             serverChannels.put(name, list);
                         }
                         list.add(channel);
-                        boundSocket.set(channel.getLocalAddress());
+                        boundSocket.set((InetSocketAddress)channel.getLocalAddress());
                     }
                 } catch (Exception e) {
                     lastException.set(e);
@@ -448,7 +453,7 @@ public class NettyTransport extends AbstractLifecycleComponent<Transport> implem
         }
 
         if (!DEFAULT_PROFILE.equals(name)) {
-            InetSocketAddress boundAddress = (InetSocketAddress) boundSocket.get();
+            InetSocketAddress boundAddress = boundSocket.get();
             int publishPort = settings.getAsInt("publish_port", boundAddress.getPort());
             String publishHost = settings.get("publish_host", boundAddress.getHostString());
             InetSocketAddress publishAddress = createPublishAddress(publishHost, publishPort);
@@ -456,7 +461,7 @@ public class NettyTransport extends AbstractLifecycleComponent<Transport> implem
             profileBoundAddresses.putIfAbsent(name, new BoundTransportAddress(new InetSocketTransportAddress(boundAddress), new InetSocketTransportAddress(publishAddress)));
         }
 
-        logger.info("Bound profile [{}] to address [{}]", name, boundSocket.get());
+        logger.info("Bound profile [{}] to address {{}}", name, NetworkAddress.format(boundSocket.get()));
     }
 
     private void createServerBootstrap(String name, Settings settings) {

core/src/test/java/org/elasticsearch/discovery/zen/ping/unicast/UnicastZenPingIT.java

@@ -24,6 +24,7 @@ import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.node.DiscoveryNodes;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.network.NetworkAddress;
 import org.elasticsearch.common.network.NetworkService;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.transport.InetSocketTransportAddress;
@@ -39,6 +40,8 @@ import org.elasticsearch.transport.TransportService;
 import org.elasticsearch.transport.netty.NettyTransport;
 import org.junit.Test;
 
+import java.net.InetSocketAddress;
+
 import static org.hamcrest.Matchers.equalTo;
 
 public class UnicastZenPingIT extends ESTestCase {
@@ -68,8 +71,8 @@ public class UnicastZenPingIT extends ESTestCase {
         InetSocketTransportAddress addressB = (InetSocketTransportAddress) transportB.boundAddress().publishAddress();
 
         Settings hostsSettings = Settings.settingsBuilder().putArray("discovery.zen.ping.unicast.hosts",
-                addressA.address().getAddress().getHostAddress() + ":" + addressA.address().getPort(),
+                NetworkAddress.formatAddress(new InetSocketAddress(addressA.address().getAddress(), addressA.address().getPort())),
-                addressB.address().getAddress().getHostAddress() + ":" + addressB.address().getPort())
+                NetworkAddress.formatAddress(new InetSocketAddress(addressB.address().getAddress(), addressB.address().getPort())))
                 .build();
 
         UnicastZenPing zenPingA = new UnicastZenPing(hostsSettings, threadPool, transportServiceA, clusterName, Version.CURRENT, electMasterService, null);

core/src/test/java/org/elasticsearch/http/netty/pipelining/HttpPipeliningHandlerTest.java

@@ -18,6 +18,7 @@
  */
 package org.elasticsearch.http.netty.pipelining;
 
+import org.elasticsearch.common.network.NetworkAddress;
 import org.elasticsearch.test.ESTestCase;
 import org.jboss.netty.bootstrap.ClientBootstrap;
 import org.jboss.netty.bootstrap.ServerBootstrap;
@@ -124,13 +125,14 @@ public class HttpPipeliningHandlerTest extends ESTestCase {
         assertTrue(connectionFuture.await(CONNECTION_TIMEOUT));
         final Channel clientChannel = connectionFuture.getChannel();
 
+        // NetworkAddress.formatAddress makes a proper HOST header.
         final HttpRequest request1 = new DefaultHttpRequest(
                 HTTP_1_1, HttpMethod.GET, PATH1);
-        request1.headers().add(HOST, HOST_ADDR.toString());
+        request1.headers().add(HOST, NetworkAddress.formatAddress(HOST_ADDR));
 
         final HttpRequest request2 = new DefaultHttpRequest(
                 HTTP_1_1, HttpMethod.GET, PATH2);
-        request2.headers().add(HOST, HOST_ADDR.toString());
+        request2.headers().add(HOST, NetworkAddress.formatAddress(HOST_ADDR));
 
         clientChannel.write(request1);
         clientChannel.write(request2);

core/src/test/java/org/elasticsearch/transport/netty/NettyTransportTests.java

@@ -22,8 +22,6 @@ package org.elasticsearch.transport.netty;
 import org.elasticsearch.common.transport.TransportAddress;
 import org.elasticsearch.test.ESTestCase;
 
-import java.net.InetAddress;
-
 /** Unit tests for NettyTransport */
 public class NettyTransportTests extends ESTestCase {
     
@@ -32,7 +30,7 @@ public class NettyTransportTests extends ESTestCase {
         TransportAddress[] addresses = NettyTransport.parse("127.0.0.1", "1234");
         assertEquals(1, addresses.length);
 
-        assertEquals(InetAddress.getByName("127.0.0.1").getHostAddress(), addresses[0].getAddress());
+        assertEquals("127.0.0.1", addresses[0].getAddress());
         assertEquals(1234, addresses[0].getPort());
     }
 
@@ -41,10 +39,10 @@ public class NettyTransportTests extends ESTestCase {
         TransportAddress[] addresses = NettyTransport.parse("127.0.0.1", "1234-1235");
         assertEquals(2, addresses.length);
 
-        assertEquals(InetAddress.getByName("127.0.0.1").getHostAddress(), addresses[0].getAddress());
+        assertEquals("127.0.0.1", addresses[0].getAddress());
         assertEquals(1234, addresses[0].getPort());
         
-        assertEquals(InetAddress.getByName("127.0.0.1").getHostAddress(), addresses[1].getAddress());
+        assertEquals("127.0.0.1", addresses[1].getAddress());
         assertEquals(1235, addresses[1].getPort());
     }
 
@@ -53,7 +51,7 @@ public class NettyTransportTests extends ESTestCase {
         TransportAddress[] addresses = NettyTransport.parse("127.0.0.1:2345", "1234");
         assertEquals(1, addresses.length);
 
-        assertEquals(InetAddress.getByName("127.0.0.1").getHostAddress(), addresses[0].getAddress());
+        assertEquals("127.0.0.1", addresses[0].getAddress());
         assertEquals(2345, addresses[0].getPort());
     }
 
@@ -62,10 +60,10 @@ public class NettyTransportTests extends ESTestCase {
         TransportAddress[] addresses = NettyTransport.parse("127.0.0.1:2345-2346", "1234");
         assertEquals(2, addresses.length);
 
-        assertEquals(InetAddress.getByName("127.0.0.1").getHostAddress(), addresses[0].getAddress());
+        assertEquals("127.0.0.1", addresses[0].getAddress());
         assertEquals(2345, addresses[0].getPort());
 
-        assertEquals(InetAddress.getByName("127.0.0.1").getHostAddress(), addresses[1].getAddress());
+        assertEquals("127.0.0.1", addresses[1].getAddress());
         assertEquals(2346, addresses[1].getPort());
     }
 
@@ -84,7 +82,7 @@ public class NettyTransportTests extends ESTestCase {
         TransportAddress[] addresses = NettyTransport.parse("[::1]", "1234");
         assertEquals(1, addresses.length);
 
-        assertEquals(InetAddress.getByName("::1").getHostAddress(), addresses[0].getAddress());
+        assertEquals("::1", addresses[0].getAddress());
         assertEquals(1234, addresses[0].getPort());
     }
 
@@ -93,10 +91,10 @@ public class NettyTransportTests extends ESTestCase {
         TransportAddress[] addresses = NettyTransport.parse("[::1]", "1234-1235");
         assertEquals(2, addresses.length);
 
-        assertEquals(InetAddress.getByName("::1").getHostAddress(), addresses[0].getAddress());
+        assertEquals("::1", addresses[0].getAddress());
         assertEquals(1234, addresses[0].getPort());
         
-        assertEquals(InetAddress.getByName("::1").getHostAddress(), addresses[1].getAddress());
+        assertEquals("::1", addresses[1].getAddress());
         assertEquals(1235, addresses[1].getPort());
     }
 
@@ -105,7 +103,7 @@ public class NettyTransportTests extends ESTestCase {
         TransportAddress[] addresses = NettyTransport.parse("[::1]:2345", "1234");
         assertEquals(1, addresses.length);
 
-        assertEquals(InetAddress.getByName("::1").getHostAddress(), addresses[0].getAddress());
+        assertEquals("::1", addresses[0].getAddress());
         assertEquals(2345, addresses[0].getPort());
     }
 
@@ -114,10 +112,10 @@ public class NettyTransportTests extends ESTestCase {
         TransportAddress[] addresses = NettyTransport.parse("[::1]:2345-2346", "1234");
         assertEquals(2, addresses.length);
 
-        assertEquals(InetAddress.getByName("::1").getHostAddress(), addresses[0].getAddress());
+        assertEquals("::1", addresses[0].getAddress());
         assertEquals(2345, addresses[0].getPort());
 
-        assertEquals(InetAddress.getByName("::1").getHostAddress(), addresses[1].getAddress());
+        assertEquals("::1", addresses[1].getAddress());
         assertEquals(2346, addresses[1].getPort());
     }
 }

dev-tools/src/main/resources/forbidden/all-signatures.txt

@@ -73,3 +73,10 @@ java.net.MulticastSocket#<init>()
 java.net.MulticastSocket#<init>(int)
 java.net.ServerSocket#<init>(int)
 java.net.ServerSocket#<init>(int,int)
+
+@defaultMessage use NetworkAddress format/formatAddress to print IP or IP+ports
+java.net.InetAddress#toString()
+java.net.InetAddress#getHostAddress()
+java.net.Inet4Address#getHostAddress()
+java.net.Inet6Address#getHostAddress()
+java.net.InetSocketAddress#toString()