Skip to content

Commit

Permalink
Replace URL with String as the key (#1038)
Browse files Browse the repository at this point in the history 
The equals() implementation of URL resolves the hostname into an IP
address. When multiple URLs are mapped into the same IP (especially with
1.1.1.1 DNS server), it will cause the following exception:

```
java.lang.IllegalArgumentException: Multiple entries with same key: https://ifconfig.co/ip=Success(x.x.x.x) and https://www.trackip.net/ip=Success(x.x.x.x)
        at com.google.common.collect.ImmutableMap.conflictException(ImmutableMap.java:377)
        at com.google.common.collect.ImmutableMap.checkNoConflict(ImmutableMap.java:371)
        at com.google.common.collect.RegularImmutableMap.checkNoConflictInKeyBucket(RegularImmutableMap.java:241)
        at com.google.common.collect.RegularImmutableMap.fromEntryArrayCheckingBucketOverflow(RegularImmutableMap.java:132)
        at com.google.common.collect.RegularImmutableMap.fromEntryArray(RegularImmutableMap.java:94)
        at com.google.common.collect.ImmutableMap$Builder.build(ImmutableMap.java:573)
        at com.google.common.collect.ImmutableMap$Builder.buildOrThrow(ImmutableMap.java:601)
        at com.google.common.collect.ImmutableMap$Builder.build(ImmutableMap.java:588)
        at com.radixdlt.p2p.hostip.NetworkQueryHostIp.get(NetworkQueryHostIp.java:191)
```
  • Loading branch information
@iamyulong
iamyulong authored Dec 2, 2024
2 parents 3eb74c0 + 02ea27b commit 7440dd4
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 65 deletions.
58 changes: 27 additions & 31 deletions core/src/main/java/com/radixdlt/p2p/hostip/NetworkQueryHostIp.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@
import com.radixdlt.lang.Result;
import com.radixdlt.utils.properties.RuntimeProperties;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand All @@ -98,21 +96,21 @@ final class NetworkQueryHostIp {

public record VotedResult(
Optional<HostIp> conclusiveHostIp,
ImmutableMap<URL, Result<HostIp, IOException>> individualQueryResults) {}
ImmutableMap<String, Result<HostIp, IOException>> individualQueryResults) {}

@VisibleForTesting static final String QUERY_URLS_PROPERTY = "network.host_ip_query_urls";

@VisibleForTesting
static final ImmutableList<URL> DEFAULT_QUERY_URLS =
static final ImmutableList<String> DEFAULT_QUERY_URLS =
ImmutableList.of(
makeurl("https://checkip.amazonaws.com/"),
makeurl("https://ipv4.icanhazip.com/"),
makeurl("https://myexternalip.com/raw"),
makeurl("https://ipecho.net/plain"),
makeurl("https://www.trackip.net/ip"),
makeurl("https://ifconfig.co/ip"));

static NetworkQueryHostIp create(Collection<URL> urls) {
"https://checkip.amazonaws.com/",
"https://ipv4.icanhazip.com/",
"https://myexternalip.com/raw",
"https://ipecho.net/plain",
"https://www.trackip.net/ip",
"https://ifconfig.co/ip");

static NetworkQueryHostIp create(Collection<String> urls) {
return new NetworkQueryHostIp(urls);
}

Expand All @@ -121,18 +119,20 @@ static NetworkQueryHostIp create(RuntimeProperties properties) {
if (urlsProperty == null || urlsProperty.trim().isEmpty()) {
return create(DEFAULT_QUERY_URLS);
}
ImmutableList<URL> urls =
Arrays.asList(urlsProperty.split(",")).stream()
.map(NetworkQueryHostIp::makeurl)
.collect(ImmutableList.toImmutableList());
ImmutableList<String> urls =
Arrays.asList(urlsProperty.split(",")).stream().collect(ImmutableList.toImmutableList());
return create(urls);
}

private final List<URL> hosts;
private final List<String> hosts;
private final OkHttpClient okHttpClient;
private final Supplier<VotedResult> result = Suppliers.memoize(this::get);

NetworkQueryHostIp(Collection<URL> urls) {
NetworkQueryHostIp() {
this(DEFAULT_QUERY_URLS);
}

NetworkQueryHostIp(Collection<String> urls) {
if (urls.isEmpty()) {
throw new IllegalArgumentException("At least one URL must be specified");
}
Expand All @@ -144,20 +144,24 @@ int count() {
return this.hosts.size();
}

List<String> hosts() {
return this.hosts;
}

public VotedResult queryNetworkHosts() {
return result.get();
}

VotedResult get() {
// Make sure we don't DoS the first one on the list
Collections.shuffle(this.hosts);
log.debug("Using hosts {}", this.hosts);
Collections.shuffle(this.hosts());
log.debug("Using hosts {}", this.hosts());
final Map<HostIp, AtomicInteger> successCounts = Maps.newHashMap();
final ImmutableMap.Builder<URL, Result<HostIp, IOException>> queryResults =
final ImmutableMap.Builder<String, Result<HostIp, IOException>> queryResults =
ImmutableMap.builder();
int maxCount = 0;
Optional<HostIp> maxResult = Optional.empty();
for (URL url : this.hosts) {
for (String url : this.hosts()) {
final Result<HostIp, IOException> result = query(url);
queryResults.put(url, result);
if (result.isSuccess()) {
Expand Down Expand Up @@ -191,7 +195,7 @@ VotedResult get() {
return new VotedResult(maxResult, queryResults.build());
}

Result<HostIp, IOException> query(URL url) {
Result<HostIp, IOException> query(String url) {
try {
// Some services simply require the headers we set here:
final var request =
Expand All @@ -214,12 +218,4 @@ Result<HostIp, IOException> query(URL url) {
return Result.error(ex);
}
}

private static URL makeurl(String s) {
try {
return new URL(s);
} catch (MalformedURLException ex) {
throw new IllegalStateException("While constructing URL for " + s, ex);
}
}
}
57 changes: 23 additions & 34 deletions core/src/test/java/com/radixdlt/p2p/hostip/NetworkQueryHostIpTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,19 @@
package com.radixdlt.p2p.hostip;

import static org.junit.Assert.*;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.*;

import com.radixdlt.lang.Result;
import com.radixdlt.utils.properties.RuntimeProperties;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import org.junit.Test;
import org.mockito.Mockito;

public class NetworkQueryHostIpTest {

Expand Down Expand Up @@ -112,43 +113,31 @@ public void testCollectionEmpty() {

@Test
public void testCollectionNotEmptyQueryNotSuccessful() throws IOException {
NetworkQueryHostIp nqhip = NetworkQueryHostIp.create(List.of(makeUrl(404, "not found")));
Optional<HostIp> host = nqhip.queryNetworkHosts().conclusiveHostIp();
assertFalse(host.isPresent());
}

@Test
public void testCollectionNotEmptyQueryIoException() throws IOException {
URL url = mock(URL.class);
doReturn("https://mock.url/with-io-problems").when(url).toString();
doThrow(new IOException("test exception")).when(url).openConnection();
NetworkQueryHostIp nqhip = NetworkQueryHostIp.create(List.of(url));
NetworkQueryHostIp nqhip =
makeNetworkQueryHostIp(Map.of("a", Result.error(new IOException(""))));
Optional<HostIp> host = nqhip.queryNetworkHosts().conclusiveHostIp();
assertFalse(host.isPresent());
}

@Test
public void testCollectionAllDifferent() throws IOException {
List<URL> urls =
List.of(
makeUrl(200, "127.0.0.1"),
makeUrl(200, "127.0.0.2"),
makeUrl(200, "127.0.0.3"),
makeUrl(200, "127.0.0.4"));
NetworkQueryHostIp nqhip = NetworkQueryHostIp.create(urls);
NetworkQueryHostIp nqhip =
makeNetworkQueryHostIp(
Map.of(
"a", Result.success(new HostIp("1")),
"b", Result.success(new HostIp("2")),
"c", Result.success(new HostIp("2")),
"d", Result.success(new HostIp("4"))));
Optional<HostIp> host = nqhip.queryNetworkHosts().conclusiveHostIp();
assertFalse(host.isPresent());
assertEquals(Optional.of(new HostIp("2")), host);
}

private static URL makeUrl(int status, String body) throws IOException {
HttpURLConnection conn = mock(HttpURLConnection.class);
doReturn(status).when(conn).getResponseCode();
doReturn(new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8)))
.when(conn)
.getInputStream();
URL url = mock(URL.class);
doReturn(conn).when(url).openConnection();
doReturn("https://mock.url/?result=%s".formatted(body)).when(url).toString();
return url;
NetworkQueryHostIp makeNetworkQueryHostIp(Map<String, Result<HostIp, IOException>> responses) {
NetworkQueryHostIp nqhip = spy(NetworkQueryHostIp.class);
doReturn(new ArrayList<String>(responses.keySet())).when(nqhip).hosts();
for (var entry : responses.entrySet()) {
doReturn(entry.getValue()).when(nqhip).query(Mockito.eq(entry.getKey()));
}
return nqhip;
}
}

0 comments on commit 7440dd4

Please sign in to comment.