From 214d303e6a4f2ece9c562bc24be0c66b80a68a84 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 20 Aug 2015 20:57:26 +0200 Subject: [PATCH] Remove support for address resolving in InetSocketTransportAddress this commit removes all support for reverse host name resolving from InetSocketTransportAddress. This class now only returns IP addresses. Closes #13014 --- .../common/network/NetworkService.java | 1 - .../transport/InetSocketTransportAddress.java | 74 ++++++------------- .../elasticsearch/test/ESIntegTestCase.java | 56 +++++++------- 3 files changed, 48 insertions(+), 83 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/network/NetworkService.java b/core/src/main/java/org/elasticsearch/common/network/NetworkService.java index 0e49c1b1118..1ce318ce6e7 100644 --- a/core/src/main/java/org/elasticsearch/common/network/NetworkService.java +++ b/core/src/main/java/org/elasticsearch/common/network/NetworkService.java @@ -83,7 +83,6 @@ public class NetworkService extends AbstractComponent { public NetworkService(Settings settings) { super(settings); IfConfig.logIfNecessary(); - InetSocketTransportAddress.setResolveAddress(settings.getAsBoolean("network.address.serialization.resolve", false)); } /** diff --git a/core/src/main/java/org/elasticsearch/common/transport/InetSocketTransportAddress.java b/core/src/main/java/org/elasticsearch/common/transport/InetSocketTransportAddress.java index 29c3f2560fa..936ef1ab544 100644 --- a/core/src/main/java/org/elasticsearch/common/transport/InetSocketTransportAddress.java +++ b/core/src/main/java/org/elasticsearch/common/transport/InetSocketTransportAddress.java @@ -19,7 +19,6 @@ package org.elasticsearch.common.transport; -import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.network.NetworkAddress; @@ -34,41 +33,22 @@ import java.net.InetSocketAddress; */ public final class InetSocketTransportAddress implements TransportAddress { - // TODO: do we really need this option, why do resolving? - remove this as a follow-up - private static boolean resolveAddress = false; - - public static void setResolveAddress(boolean resolveAddress) { - InetSocketTransportAddress.resolveAddress = resolveAddress; - } - - public static boolean getResolveAddress() { - return resolveAddress; - } - - public static final InetSocketTransportAddress PROTO = new InetSocketTransportAddress(); + public static final InetSocketTransportAddress PROTO = new InetSocketTransportAddress(new InetSocketAddress("127.0.0.1", 0)); private final InetSocketAddress address; public InetSocketTransportAddress(StreamInput in) throws IOException { - if (in.readByte() == 0) { - int len = in.readByte(); - byte[] a = new byte[len]; // 4 bytes (IPv4) or 16 bytes (IPv6) - in.readFully(a); - InetAddress inetAddress; - if (len == 16) { - inetAddress = Inet6Address.getByAddress(null, a); - } else { - inetAddress = InetAddress.getByAddress(a); - } - int port = in.readInt(); - this.address = new InetSocketAddress(inetAddress, port); + final int len = in.readByte(); + final byte[] a = new byte[len]; // 4 bytes (IPv4) or 16 bytes (IPv6) + in.readFully(a); + InetAddress inetAddress; + if (len == 16) { + inetAddress = Inet6Address.getByAddress(null, a); } else { - this.address = new InetSocketAddress(InetAddress.getByName(in.readString()), in.readInt()); + inetAddress = InetAddress.getByAddress(a); } - } - - private InetSocketTransportAddress() { - address = null; + int port = in.readInt(); + this.address = new InetSocketAddress(inetAddress, port); } public InetSocketTransportAddress(InetAddress address, int port) { @@ -76,6 +56,12 @@ public final class InetSocketTransportAddress implements TransportAddress { } public InetSocketTransportAddress(InetSocketAddress address) { + if (address == null) { + throw new IllegalArgumentException("InetSocketAddress must not be null"); + } + if (address.getAddress() == null) { + throw new IllegalArgumentException("Address must be resolved but wasn't - InetSocketAddress#getAddress() returned null"); + } this.address = address; } @@ -92,7 +78,7 @@ public final class InetSocketTransportAddress implements TransportAddress { @Override public String getHost() { - return maybeLookupHostname(); + return getAddress(); // just delegate no resolving } @Override @@ -116,29 +102,15 @@ public final class InetSocketTransportAddress implements TransportAddress { @Override public void writeTo(StreamOutput out) throws IOException { - if (!resolveAddress && address.getAddress() != null) { - out.writeByte((byte) 0); - byte[] bytes = address().getAddress().getAddress(); // 4 bytes (IPv4) or 16 bytes (IPv6) - out.writeByte((byte) bytes.length); // 1 byte - out.write(bytes, 0, bytes.length); - // don't serialize scope ids over the network!!!! - // these only make sense with respect to the local machine, and will only formulate - // the address incorrectly remotely. - } else { - out.writeByte((byte) 1); - out.writeString(maybeLookupHostname()); - } + byte[] bytes = address().getAddress().getAddress(); // 4 bytes (IPv4) or 16 bytes (IPv6) + out.writeByte((byte) bytes.length); // 1 byte + out.write(bytes, 0, bytes.length); + // don't serialize scope ids over the network!!!! + // these only make sense with respect to the local machine, and will only formulate + // the address incorrectly remotely. out.writeInt(address.getPort()); } - @SuppressForbidden(reason = "if explicitly configured we do hostName reverse lookup") // TODO remove this? - private String maybeLookupHostname() { - if (resolveAddress) { - return address.getHostName(); - } else { - return getAddress(); - } - } @Override public boolean equals(Object o) { diff --git a/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java b/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java index f29972f9c24..9da5a3e1663 100644 --- a/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java @@ -1046,40 +1046,34 @@ public abstract class ESIntegTestCase extends ESTestCase { */ protected void ensureClusterStateConsistency() throws IOException { if (cluster() != null) { - boolean getResolvedAddress = InetSocketTransportAddress.getResolveAddress(); - try { - InetSocketTransportAddress.setResolveAddress(false); - ClusterState masterClusterState = client().admin().cluster().prepareState().all().get().getState(); - byte[] masterClusterStateBytes = ClusterState.Builder.toBytes(masterClusterState); + ClusterState masterClusterState = client().admin().cluster().prepareState().all().get().getState(); + byte[] masterClusterStateBytes = ClusterState.Builder.toBytes(masterClusterState); + // remove local node reference + masterClusterState = ClusterState.Builder.fromBytes(masterClusterStateBytes, null); + Map masterStateMap = convertToMap(masterClusterState); + int masterClusterStateSize = masterClusterState.toString().length(); + String masterId = masterClusterState.nodes().masterNodeId(); + for (Client client : cluster()) { + ClusterState localClusterState = client.admin().cluster().prepareState().all().setLocal(true).get().getState(); + byte[] localClusterStateBytes = ClusterState.Builder.toBytes(localClusterState); // remove local node reference - masterClusterState = ClusterState.Builder.fromBytes(masterClusterStateBytes, null); - Map masterStateMap = convertToMap(masterClusterState); - int masterClusterStateSize = masterClusterState.toString().length(); - String masterId = masterClusterState.nodes().masterNodeId(); - for (Client client : cluster()) { - ClusterState localClusterState = client.admin().cluster().prepareState().all().setLocal(true).get().getState(); - byte[] localClusterStateBytes = ClusterState.Builder.toBytes(localClusterState); - // remove local node reference - localClusterState = ClusterState.Builder.fromBytes(localClusterStateBytes, null); - final Map localStateMap = convertToMap(localClusterState); - final int localClusterStateSize = localClusterState.toString().length(); - // Check that the non-master node has the same version of the cluster state as the master and that this node didn't disconnect from the master - if (masterClusterState.version() == localClusterState.version() && localClusterState.nodes().nodes().containsKey(masterId)) { - try { - assertEquals("clusterstate UUID does not match", masterClusterState.stateUUID(), localClusterState.stateUUID()); - // We cannot compare serialization bytes since serialization order of maps is not guaranteed - // but we can compare serialization sizes - they should be the same - assertEquals("clusterstate size does not match", masterClusterStateSize, localClusterStateSize); - // Compare JSON serialization - assertNull("clusterstate JSON serialization does not match", differenceBetweenMapsIgnoringArrayOrder(masterStateMap, localStateMap)); - } catch (AssertionError error) { - logger.error("Cluster state from master:\n{}\nLocal cluster state:\n{}", masterClusterState.toString(), localClusterState.toString()); - throw error; - } + localClusterState = ClusterState.Builder.fromBytes(localClusterStateBytes, null); + final Map localStateMap = convertToMap(localClusterState); + final int localClusterStateSize = localClusterState.toString().length(); + // Check that the non-master node has the same version of the cluster state as the master and that this node didn't disconnect from the master + if (masterClusterState.version() == localClusterState.version() && localClusterState.nodes().nodes().containsKey(masterId)) { + try { + assertEquals("clusterstate UUID does not match", masterClusterState.stateUUID(), localClusterState.stateUUID()); + // We cannot compare serialization bytes since serialization order of maps is not guaranteed + // but we can compare serialization sizes - they should be the same + assertEquals("clusterstate size does not match", masterClusterStateSize, localClusterStateSize); + // Compare JSON serialization + assertNull("clusterstate JSON serialization does not match", differenceBetweenMapsIgnoringArrayOrder(masterStateMap, localStateMap)); + } catch (AssertionError error) { + logger.error("Cluster state from master:\n{}\nLocal cluster state:\n{}", masterClusterState.toString(), localClusterState.toString()); + throw error; } } - } finally { - InetSocketTransportAddress.setResolveAddress(getResolvedAddress); } }