Skip to content

Commit

Permalink
Remove hostname from NetworkAddress.format
Browse files Browse the repository at this point in the history 
This removes the inconsistent output of IP addresses. The format was parsing-unfriendly and it makes it hard
to reason about API responses, such as to _nodes.

With this change in place, it will never print the hostname as part of the default format, which has the
added benefit that it can be used consistently for URIs, which was not the case when the hostname might
appear at the front with "hostname/ip:port".
  • Loading branch information
@pickypg
pickypg committed Apr 7, 2016
1 parent ee3c158 commit d97d5eb
Show file tree
Hide file tree
Showing 15 changed files with 67 additions and 162 deletions.
8 changes: 4 additions & 4 deletions core/src/main/java/org/elasticsearch/common/network/IfConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,22 +114,22 @@ private static String formatAddress(InterfaceAddress interfaceAddress) throws IO
InetAddress address = interfaceAddress.getAddress();
if (address instanceof Inet6Address) {
sb.append("inet6 ");
sb.append(NetworkAddress.formatAddress(address));
sb.append(NetworkAddress.format(address));
sb.append(" prefixlen:");
sb.append(interfaceAddress.getNetworkPrefixLength());
} else {
sb.append("inet ");
sb.append(NetworkAddress.formatAddress(address));
sb.append(NetworkAddress.format(address));
int netmask = 0xFFFFFFFF << (32 - interfaceAddress.getNetworkPrefixLength());
sb.append(" netmask:" + NetworkAddress.formatAddress(InetAddress.getByAddress(new byte[] {
sb.append(" netmask:" + NetworkAddress.format(InetAddress.getByAddress(new byte[] {
(byte)(netmask >>> 24),
(byte)(netmask >>> 16 & 0xFF),
(byte)(netmask >>> 8 & 0xFF),
(byte)(netmask & 0xFF)
})));
InetAddress broadcast = interfaceAddress.getBroadcast();
if (broadcast != null) {
sb.append(" broadcast:" + NetworkAddress.formatAddress(broadcast));
sb.append(" broadcast:" + NetworkAddress.format(broadcast));
}
}
if (address.isLoopbackAddress()) {
Expand Down
103 changes: 16 additions & 87 deletions core/src/main/java/org/elasticsearch/common/network/NetworkAddress.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,12 @@

package org.elasticsearch.common.network;

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
Expand All @@ -42,79 +40,21 @@
* <pre>
* {@code /0:0:0:0:0:0:0:1%1}
* </pre>
* This class provides sane address formatting instead, e.g.
* Note: the {@code %1} is the "scopeid".
* <p>
* 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
* if present, is ignored. IPv6 addresses are compressed
* and without scope identifiers.
* <p>
* Example output with just an address:
Expand All @@ -125,14 +65,14 @@ public static String format(InetSocketAddress address) {
* @param address IPv4 or IPv6 address
* @return formatted string
*/
public static String formatAddress(InetAddress address) {
return format(address, -1, false);
public static String format(InetAddress address) {
return format(address, -1);
}

/**
* Formats a network address and port for display purposes.
* <p>
* This formats the address with {@link #formatAddress(InetAddress)}
* This formats the address with {@link #format(InetAddress)}
* and appends the port number. IPv6 addresses will be bracketed.
* Any host information, if present is ignored.
* <p>
Expand All @@ -144,38 +84,27 @@ public static String formatAddress(InetAddress address) {
* @param address IPv4 or IPv6 address with port
* @return formatted string
*/
public static String formatAddress(InetSocketAddress address) {
return format(address.getAddress(), address.getPort(), false);
public static String format(InetSocketAddress address) {
return format(address.getAddress(), address.getPort());
}

// 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) {
static String format(InetAddress address, int port) {
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();
}
}
2 changes: 1 addition & 1 deletion core/src/main/java/org/elasticsearch/common/transport/InetSocketTransportAddress.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public String getHost() {

@Override
public String getAddress() {
return NetworkAddress.formatAddress(address.getAddress());
return NetworkAddress.format(address.getAddress());
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/elasticsearch/node/Node.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ private void writePortsFile(String type, BoundTransportAddress boundAddress) {
// no link local, just causes problems
continue;
}
writer.write(NetworkAddress.formatAddress(new InetSocketAddress(inetAddress, address.getPort())) + "\n");
writer.write(NetworkAddress.format(new InetSocketAddress(inetAddress, address.getPort())) + "\n");
}
} catch (IOException e) {
throw new RuntimeException("Failed to write ports file", e);
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/elasticsearch/rest/action/cat/RestNodesAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ private Table buildTable(RestRequest req, ClusterStateResponse state, NodesInfoR
if (httpInfo != null) {
TransportAddress transportAddress = httpInfo.getAddress().publishAddress();
if (transportAddress instanceof InetSocketTransportAddress) {
table.addCell(NetworkAddress.formatAddress(((InetSocketTransportAddress)transportAddress).address()));
table.addCell(NetworkAddress.format(((InetSocketTransportAddress)transportAddress).address()));
} else {
table.addCell(transportAddress.toString());
}
Expand Down
84 changes: 30 additions & 54 deletions core/src/test/java/org/elasticsearch/common/network/NetworkAddressTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.elasticsearch.test.ESTestCase;

import java.io.IOException;
import java.net.Inet4Address;
import java.net.Inet6Address;
import java.net.InetAddress;
import java.net.InetSocketAddress;
Expand All @@ -32,93 +31,70 @@
* Tests for network address formatting. Please avoid using any methods that cause DNS lookups!
*/
public class NetworkAddressTests extends ESTestCase {

public void testFormatV4() throws Exception {
assertEquals("localhost/127.0.0.1", NetworkAddress.format(forge("localhost", "127.0.0.1")));
assertEquals("127.0.0.1", NetworkAddress.format(forge("localhost", "127.0.0.1")));
assertEquals("127.0.0.1", NetworkAddress.format(forge(null, "127.0.0.1")));
}

public void testFormatV6() throws Exception {
assertEquals("localhost/::1", NetworkAddress.format(forge("localhost", "::1")));
assertEquals("::1", NetworkAddress.format(forge("localhost", "::1")));
assertEquals("::1", NetworkAddress.format(forge(null, "::1")));
}

public void testFormatAddressV4() throws Exception {
assertEquals("127.0.0.1", NetworkAddress.formatAddress(forge("localhost", "127.0.0.1")));
assertEquals("127.0.0.1", NetworkAddress.formatAddress(forge(null, "127.0.0.1")));
}

public void testFormatAddressV6() throws Exception {
assertEquals("::1", NetworkAddress.formatAddress(forge("localhost", "::1")));
assertEquals("::1", NetworkAddress.formatAddress(forge(null, "::1")));
}


public void testFormatPortV4() throws Exception {
assertEquals("localhost/127.0.0.1:1234", NetworkAddress.format(new InetSocketAddress(forge("localhost", "127.0.0.1"), 1234)));
assertEquals("127.0.0.1:1234", NetworkAddress.format(new InetSocketAddress(forge("localhost", "127.0.0.1"), 1234)));
assertEquals("127.0.0.1:1234", NetworkAddress.format(new InetSocketAddress(forge(null, "127.0.0.1"), 1234)));
}

public void testFormatPortV6() throws Exception {
assertEquals("localhost/[::1]:1234", NetworkAddress.format(new InetSocketAddress(forge("localhost", "::1"), 1234)));
assertEquals("[::1]:1234",NetworkAddress.format(new InetSocketAddress(forge(null, "::1"), 1234)));
}

public void testFormatAddressPortV4() throws Exception {
assertEquals("127.0.0.1:1234", NetworkAddress.formatAddress(new InetSocketAddress(forge("localhost", "127.0.0.1"), 1234)));
assertEquals("127.0.0.1:1234", NetworkAddress.formatAddress(new InetSocketAddress(forge(null, "127.0.0.1"), 1234)));
}

public void testFormatAddressPortV6() throws Exception {
assertEquals("[::1]:1234", NetworkAddress.formatAddress(new InetSocketAddress(forge("localhost", "::1"), 1234)));
assertEquals("[::1]:1234", NetworkAddress.formatAddress(new InetSocketAddress(forge(null, "::1"), 1234)));
assertEquals("[::1]:1234", NetworkAddress.format(new InetSocketAddress(forge("localhost", "::1"), 1234)));
assertEquals("[::1]:1234", NetworkAddress.format(new InetSocketAddress(forge(null, "::1"), 1234)));
}

public void testNoScopeID() throws Exception {
assertEquals("::1", NetworkAddress.format(forgeScoped(null, "::1", 5)));
assertEquals("localhost/::1", NetworkAddress.format(forgeScoped("localhost", "::1", 5)));

assertEquals("::1", NetworkAddress.formatAddress(forgeScoped(null, "::1", 5)));
assertEquals("::1", NetworkAddress.formatAddress(forgeScoped("localhost", "::1", 5)));

assertEquals("::1", NetworkAddress.format(forgeScoped("localhost", "::1", 5)));

assertEquals("[::1]:1234", NetworkAddress.format(new InetSocketAddress(forgeScoped(null, "::1", 5), 1234)));
assertEquals("localhost/[::1]:1234", NetworkAddress.format(new InetSocketAddress(forgeScoped("localhost", "::1", 5), 1234)));

assertEquals("[::1]:1234", NetworkAddress.formatAddress(new InetSocketAddress(forgeScoped(null, "::1", 5), 1234)));
assertEquals("[::1]:1234", NetworkAddress.formatAddress(new InetSocketAddress(forgeScoped("localhost", "::1", 5), 1234)));
assertEquals("[::1]:1234", NetworkAddress.format(new InetSocketAddress(forgeScoped("localhost", "::1", 5), 1234)));
}

/** Test that ipv4 address formatting round trips */
public void testRoundTripV4() throws Exception {
byte bytes[] = new byte[4];
Random random = random();
for (int i = 0; i < 10000; i++) {
random.nextBytes(bytes);
InetAddress expected = Inet4Address.getByAddress(bytes);
String formatted = NetworkAddress.formatAddress(expected);
InetAddress actual = InetAddress.getByName(formatted);
assertEquals(expected, actual);
}
roundTrip(new byte[4]);
}

/** Test that ipv6 address formatting round trips */
public void testRoundTripV6() throws Exception {
byte bytes[] = new byte[16];
roundTrip(new byte[16]);
}

/**
* Round trip test code for both IPv4 and IPv6. {@link InetAddress} contains the {@code getByAddress} and
* {@code getbyName} methods for both IPv4 and IPv6, unless you also specify a {@code scopeid}, which this does not
* test.
*
* @param bytes 4 (32-bit for IPv4) or 16 bytes (128-bit for IPv6)
* @throws Exception if any error occurs while interacting with the network address
*/
private void roundTrip(byte[] bytes) throws Exception {
Random random = random();
for (int i = 0; i < 10000; i++) {
random.nextBytes(bytes);
InetAddress expected = Inet6Address.getByAddress(bytes);
String formatted = NetworkAddress.formatAddress(expected);
InetAddress expected = InetAddress.getByAddress(bytes);
String formatted = NetworkAddress.format(expected);
InetAddress actual = InetAddress.getByName(formatted);
assertEquals(expected, actual);
}
}

/** creates address without any lookups. hostname can be null, for missing */
private InetAddress forge(String hostname, String address) throws IOException {
byte bytes[] = InetAddress.getByName(address).getAddress();
return InetAddress.getByAddress(hostname, bytes);
}

/** creates scoped ipv6 address without any lookups. hostname can be null, for missing */
private InetAddress forgeScoped(String hostname, String address, int scopeid) throws IOException {
byte bytes[] = InetAddress.getByName(address).getAddress();
Expand Down
4 changes: 2 additions & 2 deletions core/src/test/java/org/elasticsearch/discovery/zen/ping/unicast/UnicastZenPingIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ public void testSimplePings() throws InterruptedException {
InetSocketTransportAddress addressB = (InetSocketTransportAddress) transportB.boundAddress().publishAddress();

Settings hostsSettings = Settings.settingsBuilder().putArray("discovery.zen.ping.unicast.hosts",
NetworkAddress.formatAddress(new InetSocketAddress(addressA.address().getAddress(), addressA.address().getPort())),
NetworkAddress.formatAddress(new InetSocketAddress(addressB.address().getAddress(), addressB.address().getPort())))
NetworkAddress.format(new InetSocketAddress(addressA.address().getAddress(), addressA.address().getPort())),
NetworkAddress.format(new InetSocketAddress(addressB.address().getAddress(), addressB.address().getPort())))
.build();

UnicastZenPing zenPingA = new UnicastZenPing(hostsSettings, threadPool, transportServiceA, clusterName, Version.CURRENT, electMasterService, null);
Expand Down
6 changes: 3 additions & 3 deletions core/src/test/java/org/elasticsearch/http/netty/pipelining/HttpPipeliningHandlerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,14 @@ public void testShouldReturnMessagesInOrder() throws InterruptedException {
assertTrue(connectionFuture.await(CONNECTION_TIMEOUT));
final Channel clientChannel = connectionFuture.getChannel();

// NetworkAddress.formatAddress makes a proper HOST header.
// NetworkAddress.format makes a proper HOST header.
final HttpRequest request1 = new DefaultHttpRequest(
HTTP_1_1, HttpMethod.GET, PATH1);
request1.headers().add(HOST, NetworkAddress.formatAddress(boundAddress));
request1.headers().add(HOST, NetworkAddress.format(boundAddress));

final HttpRequest request2 = new DefaultHttpRequest(
HTTP_1_1, HttpMethod.GET, PATH2);
request2.headers().add(HOST, NetworkAddress.formatAddress(boundAddress));
request2.headers().add(HOST, NetworkAddress.format(boundAddress));

clientChannel.write(request1);
clientChannel.write(request2);
Expand Down
2 changes: 1 addition & 1 deletion ...src/test/java/org/elasticsearch/transport/netty/NettyTransportMultiPortIntegrationIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public void testThatInfosAreExposed() throws Exception {
// publish address
assertThat(nodeInfo.getTransport().getProfileAddresses().get("client1").publishAddress(), instanceOf(InetSocketTransportAddress.class));
InetSocketTransportAddress publishAddress = (InetSocketTransportAddress) nodeInfo.getTransport().getProfileAddresses().get("client1").publishAddress();
assertThat(NetworkAddress.formatAddress(publishAddress.address().getAddress()), is("127.0.0.7"));
assertThat(NetworkAddress.format(publishAddress.address().getAddress()), is("127.0.0.7"));
assertThat(publishAddress.address().getPort(), is(4321));
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/org/elasticsearch/transport/netty/SimpleNettyTransportTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public void testConnectException() throws UnknownHostException {
fail("Expected ConnectTransportException");
} catch (ConnectTransportException e) {
assertThat(e.getMessage(), containsString("connect_timeout"));
assertThat(e.getMessage(), containsString("[localhost/127.0.0.1:9876]"));
assertThat(e.getMessage(), containsString("[127.0.0.1:9876]"));
}
}
}
Loading

0 comments on commit d97d5eb

Please sign in to comment.