From 417bf3b699b3a4ee0c50487223dd66bed4755b9f Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Sun, 27 Oct 2024 16:02:29 +0900 Subject: [PATCH 01/23] Client respects DNS TTL. --- .../client/proxy/ConnectProxyConfig.java | 28 ++- .../armeria/client/proxy/HAProxyConfig.java | 58 ++++- .../armeria/client/proxy/ProxyConfig.java | 202 ++++++++++++++++++ .../client/proxy/Socks4ProxyConfig.java | 26 ++- .../client/proxy/Socks5ProxyConfig.java | 27 ++- 5 files changed, 336 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java index 085609d2d41..565355bb9e8 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java @@ -18,6 +18,9 @@ import java.net.InetSocketAddress; import java.util.Objects; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.function.BiConsumer; import com.google.common.base.MoreObjects; @@ -29,7 +32,11 @@ */ public final class ConnectProxyConfig extends ProxyConfig { - private final InetSocketAddress proxyAddress; + private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); + + private long lastUpdateTime = System.currentTimeMillis(); + + private InetSocketAddress proxyAddress; @Nullable private final String username; @@ -43,11 +50,30 @@ public final class ConnectProxyConfig extends ProxyConfig { ConnectProxyConfig(InetSocketAddress proxyAddress, @Nullable String username, @Nullable String password, HttpHeaders headers, boolean useTls) { + this(proxyAddress, username, password, headers, useTls, -1); + } + + ConnectProxyConfig(InetSocketAddress proxyAddress, @Nullable String username, + @Nullable String password, HttpHeaders headers, boolean useTls, + long refreshInterval) { this.proxyAddress = proxyAddress; this.username = username; this.password = password; this.headers = headers; this.useTls = useTls; + + if (refreshInterval > 0) { + final BiConsumer callback = (newProxyAddress, updateTime) -> { + this.proxyAddress = newProxyAddress; + this.lastUpdateTime = updateTime; + }; + + ProxyConfig.reserveDNSUpdate(callback, + proxyAddress.getHostName(), + proxyAddress.getPort(), + refreshInterval, + scheduler); + } } @Override diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java index f300e524f80..c62c0e60148 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java @@ -20,6 +20,9 @@ import java.net.InetSocketAddress; import java.util.Objects; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.function.BiConsumer; import com.google.common.base.MoreObjects; @@ -31,21 +34,72 @@ */ public final class HAProxyConfig extends ProxyConfig { - private final InetSocketAddress proxyAddress; + private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); + + private long lastUpdateTime = System.currentTimeMillis(); + + private InetSocketAddress proxyAddress; @Nullable - private final InetSocketAddress sourceAddress; + private InetSocketAddress sourceAddress; HAProxyConfig(InetSocketAddress proxyAddress) { + this(proxyAddress, -1); + } + + HAProxyConfig(InetSocketAddress proxyAddress, long refreshInterval) { this.proxyAddress = proxyAddress; sourceAddress = null; + + if (refreshInterval > 0) { + final BiConsumer callback = (newProxyAddress, updateTime) -> { + this.proxyAddress = newProxyAddress; + this.lastUpdateTime = updateTime; + }; + + ProxyConfig.reserveDNSUpdate(callback, + proxyAddress.getHostName(), + proxyAddress.getPort(), + refreshInterval, + scheduler); + } } HAProxyConfig(InetSocketAddress proxyAddress, InetSocketAddress sourceAddress) { + this(proxyAddress, sourceAddress, -1); + } + + HAProxyConfig(InetSocketAddress proxyAddress, InetSocketAddress sourceAddress, long refreshInterval) { checkArgument(sourceAddress.getAddress().getClass() == proxyAddress.getAddress().getClass(), "sourceAddress and proxyAddress should be the same type"); this.proxyAddress = proxyAddress; this.sourceAddress = sourceAddress; + + if (refreshInterval > 0) { + final BiConsumer callback = (newProxyAddress, updateTime) -> { + this.proxyAddress = newProxyAddress; + this.lastUpdateTime = updateTime; + }; + + ProxyConfig.reserveDNSUpdate(callback, + proxyAddress.getHostName(), + proxyAddress.getPort(), + refreshInterval, + scheduler); + } + + if (refreshInterval > 0) { + final BiConsumer callback = (newSourceAddress, updateTime) -> { + this.sourceAddress = newSourceAddress; + this.lastUpdateTime = updateTime; + }; + + ProxyConfig.reserveDNSUpdate(callback, + sourceAddress.getHostName(), + sourceAddress.getPort(), + refreshInterval, + scheduler); + } } @Override diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java index 736124b8c91..7eb02eba18d 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java @@ -20,7 +20,15 @@ import static com.linecorp.armeria.client.proxy.DirectProxyConfig.DIRECT_PROXY_CONFIG; import static java.util.Objects.requireNonNull; +import java.net.InetAddress; import java.net.InetSocketAddress; +import java.net.UnknownHostException; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.function.BiConsumer; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.linecorp.armeria.client.ClientFactory; import com.linecorp.armeria.common.HttpHeaders; @@ -33,6 +41,8 @@ */ public abstract class ProxyConfig { + private static final Logger logger = LoggerFactory.getLogger(ProxyConfig.class); + /** * Creates a {@code ProxyConfig} configuration for SOCKS4 protocol. * @@ -44,6 +54,18 @@ public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress) { return new Socks4ProxyConfig(proxyAddress, null); } + /** + * Creates a {@code ProxyConfig} configuration for SOCKS4 protocol. + * + * @param proxyAddress the proxy address + * @param refreshInterval the DNS refresh time + */ + public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress, long refreshInterval) { + requireNonNull(proxyAddress, "proxyAddress"); + checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); + return new Socks4ProxyConfig(proxyAddress, null, refreshInterval); + } + /** * Creates a {@code ProxyConfig} configuration for SOCKS4 protocol. * @@ -56,6 +78,20 @@ public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress, String us return new Socks4ProxyConfig(proxyAddress, requireNonNull(username, "username")); } + /** + * Creates a {@code ProxyConfig} configuration for SOCKS4 protocol. + * + * @param proxyAddress the proxy address + * @param username the username + * @param refreshInterval the DNS refresh time + */ + public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress, String username, + long refreshInterval) { + requireNonNull(proxyAddress, "proxyAddress"); + checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); + return new Socks4ProxyConfig(proxyAddress, requireNonNull(username, "username"), refreshInterval); + } + /** * Creates a {@code ProxyConfig} configuration for SOCKS5 protocol. * @@ -67,6 +103,18 @@ public static Socks5ProxyConfig socks5(InetSocketAddress proxyAddress) { return new Socks5ProxyConfig(proxyAddress, null, null); } + /** + * Creates a {@code ProxyConfig} configuration for SOCKS5 protocol. + * + * @param proxyAddress the proxy address + * @param refreshInterval the DNS refresh time + */ + public static Socks5ProxyConfig socks5(InetSocketAddress proxyAddress, long refreshInterval) { + requireNonNull(proxyAddress, "proxyAddress"); + checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); + return new Socks5ProxyConfig(proxyAddress, null, null, refreshInterval); + } + /** * Creates a {@code ProxyConfig} configuration for SOCKS5 protocol. * @@ -82,6 +130,23 @@ public static Socks5ProxyConfig socks5( requireNonNull(password, "password")); } + /** + * Creates a {@code ProxyConfig} configuration for SOCKS5 protocol. + * + * @param proxyAddress the proxy address + * @param username the username + * @param password the password + * @param refreshInterval the DNS refresh time + */ + public static Socks5ProxyConfig socks5( + InetSocketAddress proxyAddress, String username, String password, long refreshInterval) { + requireNonNull(proxyAddress, "proxyAddress"); + checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); + return new Socks5ProxyConfig(proxyAddress, requireNonNull(username, "username"), + requireNonNull(password, "password"), + refreshInterval); + } + /** * Creates a {@code ProxyConfig} configuration for CONNECT protocol. * @@ -93,6 +158,18 @@ public static ConnectProxyConfig connect(InetSocketAddress proxyAddress) { return new ConnectProxyConfig(proxyAddress, null, null, HttpHeaders.of(), false); } + /** + * Creates a {@code ProxyConfig} configuration for CONNECT protocol. + * + * @param proxyAddress the proxy address + * @param refreshInterval the DNS refresh time + */ + public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, long refreshInterval) { + requireNonNull(proxyAddress, "proxyAddress"); + checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); + return new ConnectProxyConfig(proxyAddress, null, null, HttpHeaders.of(), false, refreshInterval); + } + /** * Creates a {@code ProxyConfig} configuration for CONNECT protocol. * @@ -105,6 +182,20 @@ public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, boolean return new ConnectProxyConfig(proxyAddress, null, null, HttpHeaders.of(), useTls); } + /** + * Creates a {@code ProxyConfig} configuration for CONNECT protocol. + * + * @param proxyAddress the proxy address + * @param useTls whether to use TLS to connect to the proxy + * @param refreshInterval the DNS refresh time + */ + public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, boolean useTls, + long refreshInterval) { + requireNonNull(proxyAddress, "proxyAddress"); + checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); + return new ConnectProxyConfig(proxyAddress, null, null, HttpHeaders.of(), useTls, refreshInterval); + } + /** * Creates a {@code ProxyConfig} configuration for CONNECT protocol. * @@ -118,6 +209,21 @@ public static ConnectProxyConfig connect( return connect(proxyAddress, username, password, HttpHeaders.of(), useTls); } + /** + * Creates a {@code ProxyConfig} configuration for CONNECT protocol. + * + * @param proxyAddress the proxy address + * @param username the username + * @param password the password + * @param useTls whether to use TLS to connect to the proxy + * @param refreshInterval the DNS refresh time + */ + public static ConnectProxyConfig connect( + InetSocketAddress proxyAddress, String username, String password, boolean useTls, + long refreshInterval) { + return connect(proxyAddress, username, password, HttpHeaders.of(), useTls, refreshInterval); + } + /** * Creates a {@code ProxyConfig} configuration for CONNECT protocol. * @@ -133,6 +239,22 @@ public static ConnectProxyConfig connect( return new ConnectProxyConfig(proxyAddress, null, null, headers, useTls); } + /** + * Creates a {@code ProxyConfig} configuration for CONNECT protocol. + * + * @param proxyAddress the proxy address + * @param headers the {@link HttpHeaders} to send to the proxy + * @param useTls whether to use TLS to connect to the proxy + * @param refreshInterval the DNS refresh time + */ + @UnstableApi + public static ConnectProxyConfig connect( + InetSocketAddress proxyAddress, HttpHeaders headers, boolean useTls, long refreshInterval) { + requireNonNull(proxyAddress, "proxyAddress"); + checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); + return new ConnectProxyConfig(proxyAddress, null, null, headers, useTls, refreshInterval); + } + /** * Creates a {@code ProxyConfig} configuration for CONNECT protocol. * @@ -153,6 +275,27 @@ public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, String return new ConnectProxyConfig(proxyAddress, username, password, headers, useTls); } + /** + * Creates a {@code ProxyConfig} configuration for CONNECT protocol. + * + * @param proxyAddress the proxy address + * @param username the username + * @param password the password + * @param headers the {@link HttpHeaders} to send to the proxy + * @param useTls whether to use TLS to connect to the proxy + * @param refreshInterval the DNS refresh time + */ + @UnstableApi + public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, String username, String password, + HttpHeaders headers, boolean useTls, long refreshInterval) { + requireNonNull(proxyAddress, "proxyAddress"); + checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); + requireNonNull(username, "username"); + requireNonNull(password, "password"); + requireNonNull(headers, "headers"); + return new ConnectProxyConfig(proxyAddress, username, password, headers, useTls, refreshInterval); + } + /** * Creates a {@link ProxyConfig} configuration for HAProxy protocol. * @@ -168,6 +311,22 @@ public static HAProxyConfig haproxy( return new HAProxyConfig(proxyAddress, sourceAddress); } + /** + * Creates a {@link ProxyConfig} configuration for HAProxy protocol. + * + * @param proxyAddress the proxy address + * @param sourceAddress the source address + * @param refreshInterval the DNS refresh time + */ + public static HAProxyConfig haproxy( + InetSocketAddress proxyAddress, InetSocketAddress sourceAddress, long refreshInterval) { + requireNonNull(proxyAddress, "proxyAddress"); + checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); + requireNonNull(sourceAddress, "sourceAddress"); + checkArgument(!sourceAddress.isUnresolved(), "sourceAddress must be resolved"); + return new HAProxyConfig(proxyAddress, sourceAddress, refreshInterval); + } + /** * Creates a {@link ProxyConfig} configuration for HAProxy protocol. The {@code sourceAddress} will * be inferred from either the {@link ServiceRequestContext} or the local connection address. @@ -180,6 +339,19 @@ public static ProxyConfig haproxy(InetSocketAddress proxyAddress) { return new HAProxyConfig(proxyAddress); } + /** + * Creates a {@link ProxyConfig} configuration for HAProxy protocol. The {@code sourceAddress} will + * be inferred from either the {@link ServiceRequestContext} or the local connection address. + * + * @param proxyAddress the proxy address + * @param refreshInterval the DNS refresh time + */ + public static ProxyConfig haproxy(InetSocketAddress proxyAddress, long refreshInterval) { + requireNonNull(proxyAddress, "proxyAddress"); + checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); + return new HAProxyConfig(proxyAddress, refreshInterval); + } + /** * Returns a {@code ProxyConfig} which signifies that a proxy is absent. */ @@ -205,4 +377,34 @@ public static ProxyConfig direct() { static String maskPassword(@Nullable String username, @Nullable String password) { return username != null ? "****" : null; } + + /** + * Reserves a task to periodically update DNS with a given scheduler. + * + * @param updateCallback The callback to update InetSocketAddress + * @param hostname The hostname + * @param port The port number + * @param refreshInterval The refresh Interval + * @param scheduler The scheduler + */ + protected static void reserveDNSUpdate(BiConsumer updateCallback, + String hostname, + int port, + long refreshInterval, + ScheduledExecutorService scheduler) { + scheduler.scheduleAtFixedRate(() -> { + try { + final String ipAddress = InetAddress.getByName(hostname) + .getHostAddress(); + final InetSocketAddress newInetSocketAddress = new InetSocketAddress(ipAddress, port); + final long lastUpdateTime = System.currentTimeMillis(); + updateCallback.accept(newInetSocketAddress, + lastUpdateTime); + } catch (UnknownHostException e) { + logger.warn("Failed to refresh {}'s ip address. " + + "Use the previous inet address instead.", + hostname); + } + }, 0, refreshInterval, TimeUnit.MILLISECONDS); + } } diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java index 49adc940602..72afa76c197 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java @@ -18,6 +18,9 @@ import java.net.InetSocketAddress; import java.util.Objects; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.function.BiConsumer; import com.google.common.base.MoreObjects; @@ -28,14 +31,35 @@ */ public final class Socks4ProxyConfig extends ProxyConfig { - private final InetSocketAddress proxyAddress; + private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); + + private InetSocketAddress proxyAddress; + + private long lastUpdateTime = System.currentTimeMillis(); @Nullable private final String username; Socks4ProxyConfig(InetSocketAddress proxyAddress, @Nullable String username) { + this(proxyAddress, username, -1); + } + + Socks4ProxyConfig(InetSocketAddress proxyAddress, @Nullable String username, long refreshInterval) { this.proxyAddress = proxyAddress; this.username = username; + + if (refreshInterval > 0) { + final BiConsumer callback = (newProxyAddress, updateTime) -> { + this.proxyAddress = newProxyAddress; + this.lastUpdateTime = updateTime; + }; + + ProxyConfig.reserveDNSUpdate(callback, + proxyAddress.getHostName(), + proxyAddress.getPort(), + refreshInterval, + scheduler); + } } @Override diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java index 8e541b8c041..5fa7f870e52 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java @@ -18,6 +18,9 @@ import java.net.InetSocketAddress; import java.util.Objects; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.function.BiConsumer; import com.google.common.base.MoreObjects; @@ -28,7 +31,11 @@ */ public final class Socks5ProxyConfig extends ProxyConfig { - private final InetSocketAddress proxyAddress; + private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); + + private InetSocketAddress proxyAddress; + + private long lastUpdateTime = System.currentTimeMillis(); @Nullable private final String username; @@ -38,9 +45,27 @@ public final class Socks5ProxyConfig extends ProxyConfig { Socks5ProxyConfig(InetSocketAddress proxyAddress, @Nullable String username, @Nullable String password) { + this(proxyAddress, username, password, -1); + } + + Socks5ProxyConfig(InetSocketAddress proxyAddress, @Nullable String username, + @Nullable String password, long refreshInterval) { this.proxyAddress = proxyAddress; this.username = username; this.password = password; + + if (refreshInterval > 0) { + final BiConsumer callback = (newProxyAddress, updateTime) -> { + this.proxyAddress = newProxyAddress; + this.lastUpdateTime = updateTime; + }; + + ProxyConfig.reserveDNSUpdate(callback, + proxyAddress.getHostName(), + proxyAddress.getPort(), + refreshInterval, + scheduler); + } } @Override From f57259ca171ce6671f2c55d72495339b95b74bc6 Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Wed, 30 Oct 2024 22:08:01 +0900 Subject: [PATCH 02/23] Revert "Client respects DNS TTL." This reverts commit 417bf3b699b3a4ee0c50487223dd66bed4755b9f. --- .../client/proxy/ConnectProxyConfig.java | 28 +-- .../armeria/client/proxy/HAProxyConfig.java | 58 +---- .../armeria/client/proxy/ProxyConfig.java | 202 ------------------ .../client/proxy/Socks4ProxyConfig.java | 26 +-- .../client/proxy/Socks5ProxyConfig.java | 27 +-- 5 files changed, 5 insertions(+), 336 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java index 565355bb9e8..085609d2d41 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java @@ -18,9 +18,6 @@ import java.net.InetSocketAddress; import java.util.Objects; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.function.BiConsumer; import com.google.common.base.MoreObjects; @@ -32,11 +29,7 @@ */ public final class ConnectProxyConfig extends ProxyConfig { - private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); - - private long lastUpdateTime = System.currentTimeMillis(); - - private InetSocketAddress proxyAddress; + private final InetSocketAddress proxyAddress; @Nullable private final String username; @@ -50,30 +43,11 @@ public final class ConnectProxyConfig extends ProxyConfig { ConnectProxyConfig(InetSocketAddress proxyAddress, @Nullable String username, @Nullable String password, HttpHeaders headers, boolean useTls) { - this(proxyAddress, username, password, headers, useTls, -1); - } - - ConnectProxyConfig(InetSocketAddress proxyAddress, @Nullable String username, - @Nullable String password, HttpHeaders headers, boolean useTls, - long refreshInterval) { this.proxyAddress = proxyAddress; this.username = username; this.password = password; this.headers = headers; this.useTls = useTls; - - if (refreshInterval > 0) { - final BiConsumer callback = (newProxyAddress, updateTime) -> { - this.proxyAddress = newProxyAddress; - this.lastUpdateTime = updateTime; - }; - - ProxyConfig.reserveDNSUpdate(callback, - proxyAddress.getHostName(), - proxyAddress.getPort(), - refreshInterval, - scheduler); - } } @Override diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java index c62c0e60148..f300e524f80 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java @@ -20,9 +20,6 @@ import java.net.InetSocketAddress; import java.util.Objects; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.function.BiConsumer; import com.google.common.base.MoreObjects; @@ -34,72 +31,21 @@ */ public final class HAProxyConfig extends ProxyConfig { - private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); - - private long lastUpdateTime = System.currentTimeMillis(); - - private InetSocketAddress proxyAddress; + private final InetSocketAddress proxyAddress; @Nullable - private InetSocketAddress sourceAddress; + private final InetSocketAddress sourceAddress; HAProxyConfig(InetSocketAddress proxyAddress) { - this(proxyAddress, -1); - } - - HAProxyConfig(InetSocketAddress proxyAddress, long refreshInterval) { this.proxyAddress = proxyAddress; sourceAddress = null; - - if (refreshInterval > 0) { - final BiConsumer callback = (newProxyAddress, updateTime) -> { - this.proxyAddress = newProxyAddress; - this.lastUpdateTime = updateTime; - }; - - ProxyConfig.reserveDNSUpdate(callback, - proxyAddress.getHostName(), - proxyAddress.getPort(), - refreshInterval, - scheduler); - } } HAProxyConfig(InetSocketAddress proxyAddress, InetSocketAddress sourceAddress) { - this(proxyAddress, sourceAddress, -1); - } - - HAProxyConfig(InetSocketAddress proxyAddress, InetSocketAddress sourceAddress, long refreshInterval) { checkArgument(sourceAddress.getAddress().getClass() == proxyAddress.getAddress().getClass(), "sourceAddress and proxyAddress should be the same type"); this.proxyAddress = proxyAddress; this.sourceAddress = sourceAddress; - - if (refreshInterval > 0) { - final BiConsumer callback = (newProxyAddress, updateTime) -> { - this.proxyAddress = newProxyAddress; - this.lastUpdateTime = updateTime; - }; - - ProxyConfig.reserveDNSUpdate(callback, - proxyAddress.getHostName(), - proxyAddress.getPort(), - refreshInterval, - scheduler); - } - - if (refreshInterval > 0) { - final BiConsumer callback = (newSourceAddress, updateTime) -> { - this.sourceAddress = newSourceAddress; - this.lastUpdateTime = updateTime; - }; - - ProxyConfig.reserveDNSUpdate(callback, - sourceAddress.getHostName(), - sourceAddress.getPort(), - refreshInterval, - scheduler); - } } @Override diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java index 7eb02eba18d..736124b8c91 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java @@ -20,15 +20,7 @@ import static com.linecorp.armeria.client.proxy.DirectProxyConfig.DIRECT_PROXY_CONFIG; import static java.util.Objects.requireNonNull; -import java.net.InetAddress; import java.net.InetSocketAddress; -import java.net.UnknownHostException; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; -import java.util.function.BiConsumer; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import com.linecorp.armeria.client.ClientFactory; import com.linecorp.armeria.common.HttpHeaders; @@ -41,8 +33,6 @@ */ public abstract class ProxyConfig { - private static final Logger logger = LoggerFactory.getLogger(ProxyConfig.class); - /** * Creates a {@code ProxyConfig} configuration for SOCKS4 protocol. * @@ -54,18 +44,6 @@ public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress) { return new Socks4ProxyConfig(proxyAddress, null); } - /** - * Creates a {@code ProxyConfig} configuration for SOCKS4 protocol. - * - * @param proxyAddress the proxy address - * @param refreshInterval the DNS refresh time - */ - public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress, long refreshInterval) { - requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); - return new Socks4ProxyConfig(proxyAddress, null, refreshInterval); - } - /** * Creates a {@code ProxyConfig} configuration for SOCKS4 protocol. * @@ -78,20 +56,6 @@ public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress, String us return new Socks4ProxyConfig(proxyAddress, requireNonNull(username, "username")); } - /** - * Creates a {@code ProxyConfig} configuration for SOCKS4 protocol. - * - * @param proxyAddress the proxy address - * @param username the username - * @param refreshInterval the DNS refresh time - */ - public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress, String username, - long refreshInterval) { - requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); - return new Socks4ProxyConfig(proxyAddress, requireNonNull(username, "username"), refreshInterval); - } - /** * Creates a {@code ProxyConfig} configuration for SOCKS5 protocol. * @@ -103,18 +67,6 @@ public static Socks5ProxyConfig socks5(InetSocketAddress proxyAddress) { return new Socks5ProxyConfig(proxyAddress, null, null); } - /** - * Creates a {@code ProxyConfig} configuration for SOCKS5 protocol. - * - * @param proxyAddress the proxy address - * @param refreshInterval the DNS refresh time - */ - public static Socks5ProxyConfig socks5(InetSocketAddress proxyAddress, long refreshInterval) { - requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); - return new Socks5ProxyConfig(proxyAddress, null, null, refreshInterval); - } - /** * Creates a {@code ProxyConfig} configuration for SOCKS5 protocol. * @@ -130,23 +82,6 @@ public static Socks5ProxyConfig socks5( requireNonNull(password, "password")); } - /** - * Creates a {@code ProxyConfig} configuration for SOCKS5 protocol. - * - * @param proxyAddress the proxy address - * @param username the username - * @param password the password - * @param refreshInterval the DNS refresh time - */ - public static Socks5ProxyConfig socks5( - InetSocketAddress proxyAddress, String username, String password, long refreshInterval) { - requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); - return new Socks5ProxyConfig(proxyAddress, requireNonNull(username, "username"), - requireNonNull(password, "password"), - refreshInterval); - } - /** * Creates a {@code ProxyConfig} configuration for CONNECT protocol. * @@ -158,18 +93,6 @@ public static ConnectProxyConfig connect(InetSocketAddress proxyAddress) { return new ConnectProxyConfig(proxyAddress, null, null, HttpHeaders.of(), false); } - /** - * Creates a {@code ProxyConfig} configuration for CONNECT protocol. - * - * @param proxyAddress the proxy address - * @param refreshInterval the DNS refresh time - */ - public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, long refreshInterval) { - requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); - return new ConnectProxyConfig(proxyAddress, null, null, HttpHeaders.of(), false, refreshInterval); - } - /** * Creates a {@code ProxyConfig} configuration for CONNECT protocol. * @@ -182,20 +105,6 @@ public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, boolean return new ConnectProxyConfig(proxyAddress, null, null, HttpHeaders.of(), useTls); } - /** - * Creates a {@code ProxyConfig} configuration for CONNECT protocol. - * - * @param proxyAddress the proxy address - * @param useTls whether to use TLS to connect to the proxy - * @param refreshInterval the DNS refresh time - */ - public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, boolean useTls, - long refreshInterval) { - requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); - return new ConnectProxyConfig(proxyAddress, null, null, HttpHeaders.of(), useTls, refreshInterval); - } - /** * Creates a {@code ProxyConfig} configuration for CONNECT protocol. * @@ -209,21 +118,6 @@ public static ConnectProxyConfig connect( return connect(proxyAddress, username, password, HttpHeaders.of(), useTls); } - /** - * Creates a {@code ProxyConfig} configuration for CONNECT protocol. - * - * @param proxyAddress the proxy address - * @param username the username - * @param password the password - * @param useTls whether to use TLS to connect to the proxy - * @param refreshInterval the DNS refresh time - */ - public static ConnectProxyConfig connect( - InetSocketAddress proxyAddress, String username, String password, boolean useTls, - long refreshInterval) { - return connect(proxyAddress, username, password, HttpHeaders.of(), useTls, refreshInterval); - } - /** * Creates a {@code ProxyConfig} configuration for CONNECT protocol. * @@ -239,22 +133,6 @@ public static ConnectProxyConfig connect( return new ConnectProxyConfig(proxyAddress, null, null, headers, useTls); } - /** - * Creates a {@code ProxyConfig} configuration for CONNECT protocol. - * - * @param proxyAddress the proxy address - * @param headers the {@link HttpHeaders} to send to the proxy - * @param useTls whether to use TLS to connect to the proxy - * @param refreshInterval the DNS refresh time - */ - @UnstableApi - public static ConnectProxyConfig connect( - InetSocketAddress proxyAddress, HttpHeaders headers, boolean useTls, long refreshInterval) { - requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); - return new ConnectProxyConfig(proxyAddress, null, null, headers, useTls, refreshInterval); - } - /** * Creates a {@code ProxyConfig} configuration for CONNECT protocol. * @@ -275,27 +153,6 @@ public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, String return new ConnectProxyConfig(proxyAddress, username, password, headers, useTls); } - /** - * Creates a {@code ProxyConfig} configuration for CONNECT protocol. - * - * @param proxyAddress the proxy address - * @param username the username - * @param password the password - * @param headers the {@link HttpHeaders} to send to the proxy - * @param useTls whether to use TLS to connect to the proxy - * @param refreshInterval the DNS refresh time - */ - @UnstableApi - public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, String username, String password, - HttpHeaders headers, boolean useTls, long refreshInterval) { - requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); - requireNonNull(username, "username"); - requireNonNull(password, "password"); - requireNonNull(headers, "headers"); - return new ConnectProxyConfig(proxyAddress, username, password, headers, useTls, refreshInterval); - } - /** * Creates a {@link ProxyConfig} configuration for HAProxy protocol. * @@ -311,22 +168,6 @@ public static HAProxyConfig haproxy( return new HAProxyConfig(proxyAddress, sourceAddress); } - /** - * Creates a {@link ProxyConfig} configuration for HAProxy protocol. - * - * @param proxyAddress the proxy address - * @param sourceAddress the source address - * @param refreshInterval the DNS refresh time - */ - public static HAProxyConfig haproxy( - InetSocketAddress proxyAddress, InetSocketAddress sourceAddress, long refreshInterval) { - requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); - requireNonNull(sourceAddress, "sourceAddress"); - checkArgument(!sourceAddress.isUnresolved(), "sourceAddress must be resolved"); - return new HAProxyConfig(proxyAddress, sourceAddress, refreshInterval); - } - /** * Creates a {@link ProxyConfig} configuration for HAProxy protocol. The {@code sourceAddress} will * be inferred from either the {@link ServiceRequestContext} or the local connection address. @@ -339,19 +180,6 @@ public static ProxyConfig haproxy(InetSocketAddress proxyAddress) { return new HAProxyConfig(proxyAddress); } - /** - * Creates a {@link ProxyConfig} configuration for HAProxy protocol. The {@code sourceAddress} will - * be inferred from either the {@link ServiceRequestContext} or the local connection address. - * - * @param proxyAddress the proxy address - * @param refreshInterval the DNS refresh time - */ - public static ProxyConfig haproxy(InetSocketAddress proxyAddress, long refreshInterval) { - requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); - return new HAProxyConfig(proxyAddress, refreshInterval); - } - /** * Returns a {@code ProxyConfig} which signifies that a proxy is absent. */ @@ -377,34 +205,4 @@ public static ProxyConfig direct() { static String maskPassword(@Nullable String username, @Nullable String password) { return username != null ? "****" : null; } - - /** - * Reserves a task to periodically update DNS with a given scheduler. - * - * @param updateCallback The callback to update InetSocketAddress - * @param hostname The hostname - * @param port The port number - * @param refreshInterval The refresh Interval - * @param scheduler The scheduler - */ - protected static void reserveDNSUpdate(BiConsumer updateCallback, - String hostname, - int port, - long refreshInterval, - ScheduledExecutorService scheduler) { - scheduler.scheduleAtFixedRate(() -> { - try { - final String ipAddress = InetAddress.getByName(hostname) - .getHostAddress(); - final InetSocketAddress newInetSocketAddress = new InetSocketAddress(ipAddress, port); - final long lastUpdateTime = System.currentTimeMillis(); - updateCallback.accept(newInetSocketAddress, - lastUpdateTime); - } catch (UnknownHostException e) { - logger.warn("Failed to refresh {}'s ip address. " + - "Use the previous inet address instead.", - hostname); - } - }, 0, refreshInterval, TimeUnit.MILLISECONDS); - } } diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java index 72afa76c197..49adc940602 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java @@ -18,9 +18,6 @@ import java.net.InetSocketAddress; import java.util.Objects; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.function.BiConsumer; import com.google.common.base.MoreObjects; @@ -31,35 +28,14 @@ */ public final class Socks4ProxyConfig extends ProxyConfig { - private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); - - private InetSocketAddress proxyAddress; - - private long lastUpdateTime = System.currentTimeMillis(); + private final InetSocketAddress proxyAddress; @Nullable private final String username; Socks4ProxyConfig(InetSocketAddress proxyAddress, @Nullable String username) { - this(proxyAddress, username, -1); - } - - Socks4ProxyConfig(InetSocketAddress proxyAddress, @Nullable String username, long refreshInterval) { this.proxyAddress = proxyAddress; this.username = username; - - if (refreshInterval > 0) { - final BiConsumer callback = (newProxyAddress, updateTime) -> { - this.proxyAddress = newProxyAddress; - this.lastUpdateTime = updateTime; - }; - - ProxyConfig.reserveDNSUpdate(callback, - proxyAddress.getHostName(), - proxyAddress.getPort(), - refreshInterval, - scheduler); - } } @Override diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java index 5fa7f870e52..8e541b8c041 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java @@ -18,9 +18,6 @@ import java.net.InetSocketAddress; import java.util.Objects; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.function.BiConsumer; import com.google.common.base.MoreObjects; @@ -31,11 +28,7 @@ */ public final class Socks5ProxyConfig extends ProxyConfig { - private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); - - private InetSocketAddress proxyAddress; - - private long lastUpdateTime = System.currentTimeMillis(); + private final InetSocketAddress proxyAddress; @Nullable private final String username; @@ -45,27 +38,9 @@ public final class Socks5ProxyConfig extends ProxyConfig { Socks5ProxyConfig(InetSocketAddress proxyAddress, @Nullable String username, @Nullable String password) { - this(proxyAddress, username, password, -1); - } - - Socks5ProxyConfig(InetSocketAddress proxyAddress, @Nullable String username, - @Nullable String password, long refreshInterval) { this.proxyAddress = proxyAddress; this.username = username; this.password = password; - - if (refreshInterval > 0) { - final BiConsumer callback = (newProxyAddress, updateTime) -> { - this.proxyAddress = newProxyAddress; - this.lastUpdateTime = updateTime; - }; - - ProxyConfig.reserveDNSUpdate(callback, - proxyAddress.getHostName(), - proxyAddress.getPort(), - refreshInterval, - scheduler); - } } @Override From f40cbe4c721da29419a15e25964c7de7a2362e4e Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Wed, 30 Oct 2024 22:35:19 +0900 Subject: [PATCH 03/23] Use RefreshingAddressResolver to refresh proxy address. --- .../armeria/client/HttpClientDelegate.java | 29 +++++++++++++++++-- .../client/proxy/ConnectProxyConfig.java | 10 ++++++- .../client/proxy/DirectProxyConfig.java | 5 ++++ .../armeria/client/proxy/HAProxyConfig.java | 9 +++++- .../armeria/client/proxy/ProxyConfig.java | 6 ++++ .../client/proxy/Socks4ProxyConfig.java | 10 ++++++- .../client/proxy/Socks5ProxyConfig.java | 10 ++++++- 7 files changed, 73 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java index f3d60aea270..5e10d3c4781 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java @@ -21,6 +21,9 @@ import java.net.InetSocketAddress; import java.util.function.BiConsumer; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.linecorp.armeria.client.HttpChannelPool.PoolKey; import com.linecorp.armeria.client.endpoint.EmptyEndpointGroupException; import com.linecorp.armeria.client.proxy.HAProxyConfig; @@ -50,6 +53,7 @@ final class HttpClientDelegate implements HttpClient { + private static final Logger logger = LoggerFactory.getLogger(HttpClientDelegate.class); private final HttpClientFactory factory; private final AddressResolverGroup addressResolverGroup; @@ -90,7 +94,7 @@ public HttpResponse execute(ClientRequestContext ctx, HttpRequest req) throws Ex final SessionProtocol protocol = ctx.sessionProtocol(); final ProxyConfig proxyConfig; try { - proxyConfig = getProxyConfig(protocol, endpoint); + proxyConfig = getProxyConfig(protocol, endpoint, ctx); } catch (Throwable t) { return earlyFailedResponse(t, ctx); } @@ -215,10 +219,31 @@ private void acquireConnectionAndExecute0(ClientRequestContext ctx, Endpoint end } } - private ProxyConfig getProxyConfig(SessionProtocol protocol, Endpoint endpoint) { + private ProxyConfig getProxyConfig(SessionProtocol protocol, Endpoint endpoint, ClientRequestContext ctx) { final ProxyConfig proxyConfig = factory.proxyConfigSelector().select(protocol, endpoint); requireNonNull(proxyConfig, "proxyConfig"); + final Future resolveFuture = addressResolverGroup.getResolver( + ctx.eventLoop().withoutContext()).resolve(proxyConfig.proxyAddress()); + + resolveFuture.addListener(future -> { + if (future.isSuccess()) { + final InetSocketAddress resolvedAddress = (InetSocketAddress) future.get(); + if (resolvedAddress != null && !resolvedAddress.isUnresolved()) { + proxyConfig.refreshDns(resolvedAddress); + } else { + logger.warn("Resolved address is invalid or unresolved: {}. " + + "Using previous address instead.", resolvedAddress); + } + } else { + final Throwable cause = future.cause(); + logger.warn("Failed to refresh {}'s ip address. " + + "Use the previous inet address instead. reason: {}", + proxyConfig.proxyAddress(), + cause != null ? cause.getMessage() : "Unknown Error"); + } + }); + // special behavior for haproxy when sourceAddress is null if (proxyConfig.proxyType() == ProxyType.HAPROXY && ((HAProxyConfig) proxyConfig).sourceAddress() == null) { diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java index 085609d2d41..8f56a155bb6 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java @@ -16,6 +16,8 @@ package com.linecorp.armeria.client.proxy; +import static java.util.Objects.requireNonNull; + import java.net.InetSocketAddress; import java.util.Objects; @@ -29,7 +31,7 @@ */ public final class ConnectProxyConfig extends ProxyConfig { - private final InetSocketAddress proxyAddress; + private InetSocketAddress proxyAddress; @Nullable private final String username; @@ -90,6 +92,12 @@ public ProxyType proxyType() { return ProxyType.CONNECT; } + @Override + public void refreshDns(InetSocketAddress socketAddress) { + requireNonNull(socketAddress, "socketAddress"); + this.proxyAddress = socketAddress; + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java index 3a08c219e24..9dfbeceab27 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java @@ -40,6 +40,11 @@ public InetSocketAddress proxyAddress() { return null; } + @Override + public void refreshDns(InetSocketAddress socketAddress) { + // Do nothing. + } + @Override public String toString() { return "DirectProxyConfig{proxyType=DIRECT}"; diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java index f300e524f80..8866598d46f 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java @@ -17,6 +17,7 @@ package com.linecorp.armeria.client.proxy; import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; import java.net.InetSocketAddress; import java.util.Objects; @@ -31,7 +32,7 @@ */ public final class HAProxyConfig extends ProxyConfig { - private final InetSocketAddress proxyAddress; + private InetSocketAddress proxyAddress; @Nullable private final InetSocketAddress sourceAddress; @@ -67,6 +68,12 @@ public InetSocketAddress sourceAddress() { return sourceAddress; } + @Override + public void refreshDns(InetSocketAddress socketAddress) { + requireNonNull(socketAddress, "socketAddress"); + this.proxyAddress = socketAddress; + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java index 736124b8c91..67fb3522b1f 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java @@ -195,6 +195,12 @@ public static ProxyConfig direct() { */ public abstract ProxyType proxyType(); + /** + * Refreshes the proxyAddress which ProxyConfig has, to respect DNS TTL. + * @param socketAddress the inet socket address + */ + public abstract void refreshDns(InetSocketAddress socketAddress); + /** * Returns the proxy address which is {@code null} only for {@link ProxyType#DIRECT}. */ diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java index 49adc940602..6771e962b49 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java @@ -16,6 +16,8 @@ package com.linecorp.armeria.client.proxy; +import static java.util.Objects.requireNonNull; + import java.net.InetSocketAddress; import java.util.Objects; @@ -28,7 +30,7 @@ */ public final class Socks4ProxyConfig extends ProxyConfig { - private final InetSocketAddress proxyAddress; + private InetSocketAddress proxyAddress; @Nullable private final String username; @@ -56,6 +58,12 @@ public ProxyType proxyType() { return ProxyType.SOCKS4; } + @Override + public void refreshDns(InetSocketAddress socketAddress) { + requireNonNull(socketAddress, "socketAddress"); + this.proxyAddress = socketAddress; + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java index 8e541b8c041..6f6db0e09b9 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java @@ -16,6 +16,8 @@ package com.linecorp.armeria.client.proxy; +import static java.util.Objects.requireNonNull; + import java.net.InetSocketAddress; import java.util.Objects; @@ -28,7 +30,7 @@ */ public final class Socks5ProxyConfig extends ProxyConfig { - private final InetSocketAddress proxyAddress; + private InetSocketAddress proxyAddress; @Nullable private final String username; @@ -69,6 +71,12 @@ public ProxyType proxyType() { return ProxyType.SOCKS5; } + @Override + public void refreshDns(InetSocketAddress socketAddress) { + requireNonNull(socketAddress, "socketAddress"); + this.proxyAddress = socketAddress; + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { From 649469edfa91074fbe5f4e3068021d93acc81777 Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Thu, 31 Oct 2024 05:39:23 +0900 Subject: [PATCH 04/23] Handle null of proxyconfig. --- .../armeria/client/HttpClientDelegate.java | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java index 5e10d3c4781..ae2435d1b3b 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java @@ -223,26 +223,28 @@ private ProxyConfig getProxyConfig(SessionProtocol protocol, Endpoint endpoint, final ProxyConfig proxyConfig = factory.proxyConfigSelector().select(protocol, endpoint); requireNonNull(proxyConfig, "proxyConfig"); - final Future resolveFuture = addressResolverGroup.getResolver( - ctx.eventLoop().withoutContext()).resolve(proxyConfig.proxyAddress()); + if (proxyConfig.proxyAddress() != null) { + final Future resolveFuture = addressResolverGroup.getResolver( + ctx.eventLoop().withoutContext()).resolve(proxyConfig.proxyAddress()); - resolveFuture.addListener(future -> { - if (future.isSuccess()) { - final InetSocketAddress resolvedAddress = (InetSocketAddress) future.get(); - if (resolvedAddress != null && !resolvedAddress.isUnresolved()) { - proxyConfig.refreshDns(resolvedAddress); + resolveFuture.addListener(future -> { + if (future.isSuccess()) { + final InetSocketAddress resolvedAddress = (InetSocketAddress) future.get(); + if (resolvedAddress != null && !resolvedAddress.isUnresolved()) { + proxyConfig.refreshDns(resolvedAddress); + } else { + logger.warn("Resolved address is invalid or unresolved: {}. " + + "Using previous address instead.", resolvedAddress); + } } else { - logger.warn("Resolved address is invalid or unresolved: {}. " + - "Using previous address instead.", resolvedAddress); + final Throwable cause = future.cause(); + logger.warn("Failed to refresh {}'s ip address. " + + "Use the previous inet address instead. reason: {}", + proxyConfig.proxyAddress(), + cause != null ? cause.getMessage() : "Unknown Error"); } - } else { - final Throwable cause = future.cause(); - logger.warn("Failed to refresh {}'s ip address. " + - "Use the previous inet address instead. reason: {}", - proxyConfig.proxyAddress(), - cause != null ? cause.getMessage() : "Unknown Error"); - } - }); + }); + } // special behavior for haproxy when sourceAddress is null if (proxyConfig.proxyType() == ProxyType.HAPROXY && From ab4327765b2cde799818e52d46926809233ca246 Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Thu, 31 Oct 2024 05:51:08 +0900 Subject: [PATCH 05/23] Add a copmment. --- .../java/com/linecorp/armeria/client/HttpClientDelegate.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java index ae2435d1b3b..6c4109d3fa6 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java @@ -223,6 +223,7 @@ private ProxyConfig getProxyConfig(SessionProtocol protocol, Endpoint endpoint, final ProxyConfig proxyConfig = factory.proxyConfigSelector().select(protocol, endpoint); requireNonNull(proxyConfig, "proxyConfig"); + // DirectProxyConfig does not have proxyAddress as its field. if (proxyConfig.proxyAddress() != null) { final Future resolveFuture = addressResolverGroup.getResolver( ctx.eventLoop().withoutContext()).resolve(proxyConfig.proxyAddress()); From a6e725ce5f38f06d147de6f92d8c3ad31efbef47 Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Sat, 2 Nov 2024 23:04:08 +0900 Subject: [PATCH 06/23] Add withNewProxyAddress() to create after refresh DNS. --- .../linecorp/armeria/client/proxy/ConnectProxyConfig.java | 6 ++++++ .../linecorp/armeria/client/proxy/DirectProxyConfig.java | 5 +++++ .../com/linecorp/armeria/client/proxy/HAProxyConfig.java | 7 +++++++ .../com/linecorp/armeria/client/proxy/ProxyConfig.java | 2 ++ .../linecorp/armeria/client/proxy/Socks4ProxyConfig.java | 5 +++++ .../linecorp/armeria/client/proxy/Socks5ProxyConfig.java | 5 +++++ 6 files changed, 30 insertions(+) diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java index 8f56a155bb6..25ecdefba25 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java @@ -98,6 +98,12 @@ public void refreshDns(InetSocketAddress socketAddress) { this.proxyAddress = socketAddress; } + @Override + public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { + return new ConnectProxyConfig(newProxyAddress, this.username, + this.password, this.headers, this.useTls); + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java index 9dfbeceab27..8383edcfe68 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java @@ -45,6 +45,11 @@ public void refreshDns(InetSocketAddress socketAddress) { // Do nothing. } + @Override + public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { + return new DirectProxyConfig(); + } + @Override public String toString() { return "DirectProxyConfig{proxyType=DIRECT}"; diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java index 8866598d46f..cb8453774ab 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java @@ -74,6 +74,13 @@ public void refreshDns(InetSocketAddress socketAddress) { this.proxyAddress = socketAddress; } + @Override + public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { + return this.sourceAddress == null ? + new HAProxyConfig(proxyAddress) : + new HAProxyConfig(proxyAddress, this.sourceAddress); + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java index 67fb3522b1f..b146d214a86 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java @@ -207,6 +207,8 @@ public static ProxyConfig direct() { @Nullable public abstract InetSocketAddress proxyAddress(); + public abstract ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress); + @Nullable static String maskPassword(@Nullable String username, @Nullable String password) { return username != null ? "****" : null; diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java index 6771e962b49..090913ecbcb 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java @@ -64,6 +64,11 @@ public void refreshDns(InetSocketAddress socketAddress) { this.proxyAddress = socketAddress; } + @Override + public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { + return new Socks4ProxyConfig(newProxyAddress, this.username); + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java index 6f6db0e09b9..c291d9d65ca 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java @@ -77,6 +77,11 @@ public void refreshDns(InetSocketAddress socketAddress) { this.proxyAddress = socketAddress; } + @Override + public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { + return new Socks5ProxyConfig(newProxyAddress, this.username, this.password); + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { From e0819c4ca191d56d775257a6b33473c7c1ef210f Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Sun, 3 Nov 2024 14:13:07 +0900 Subject: [PATCH 07/23] Resolve Proxy DNS to respect DNS TTL asynchronously. --- .../armeria/client/HttpClientDelegate.java | 72 +++++++++++++------ .../armeria/client/proxy/HAProxyConfig.java | 5 +- .../armeria/client/proxy/ProxyConfig.java | 4 ++ 3 files changed, 55 insertions(+), 26 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java index 6c4109d3fa6..3f9406015e0 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java @@ -92,24 +92,37 @@ public HttpResponse execute(ClientRequestContext ctx, HttpRequest req) throws Ex } final SessionProtocol protocol = ctx.sessionProtocol(); - final ProxyConfig proxyConfig; + + final Endpoint endpointWithPort = endpoint.withDefaultPort(ctx.sessionProtocol()); + final EventLoop eventLoop = ctx.eventLoop().withoutContext(); + // TODO(ikhoon) Use ctx.exchangeType() to create an optimized HttpResponse for non-streaming response. + final DecodedHttpResponse res = new DecodedHttpResponse(eventLoop); + updateCancellationTask(ctx, req, res); + try { - proxyConfig = getProxyConfig(protocol, endpoint, ctx); + resolveProxyConfig(protocol, endpoint, ctx, (proxyConfig, thrown) -> { + if (thrown != null) { + earlyFailed(thrown, ctx); + res.close(thrown); + } else { + execute0(ctx, endpointWithPort, req, res, proxyConfig); + } + }); } catch (Throwable t) { return earlyFailedResponse(t, ctx); } + return res; + } + private void execute0(ClientRequestContext ctx, Endpoint endpointWithPort, HttpRequest req, + DecodedHttpResponse res, ProxyConfig proxyConfig) { final Throwable cancellationCause = ctx.cancellationCause(); if (cancellationCause != null) { - return earlyFailedResponse(cancellationCause, ctx); + final Throwable t = earlyFailed(cancellationCause, ctx); + res.close(t); + return; } - final Endpoint endpointWithPort = endpoint.withDefaultPort(ctx.sessionProtocol()); - final EventLoop eventLoop = ctx.eventLoop().withoutContext(); - // TODO(ikhoon) Use ctx.exchangeType() to create an optimized HttpResponse for non-streaming response. - final DecodedHttpResponse res = new DecodedHttpResponse(eventLoop); - updateCancellationTask(ctx, req, res); - final ClientConnectionTimingsBuilder timingsBuilder = ClientConnectionTimings.builder(); if (endpointWithPort.hasIpAddr() || @@ -129,8 +142,6 @@ public HttpResponse execute(ClientRequestContext ctx, HttpRequest req) throws Ex } }); } - - return res; } private static void updateCancellationTask(ClientRequestContext ctx, HttpRequest req, @@ -219,34 +230,47 @@ private void acquireConnectionAndExecute0(ClientRequestContext ctx, Endpoint end } } - private ProxyConfig getProxyConfig(SessionProtocol protocol, Endpoint endpoint, ClientRequestContext ctx) { + private void resolveProxyConfig(SessionProtocol protocol, Endpoint endpoint, ClientRequestContext ctx, + BiConsumer<@Nullable ProxyConfig, @Nullable Throwable> onComplete) { final ProxyConfig proxyConfig = factory.proxyConfigSelector().select(protocol, endpoint); requireNonNull(proxyConfig, "proxyConfig"); + final ServiceRequestContext serviceCtx = ServiceRequestContext.currentOrNull(); + final ProxiedAddresses capturedProxiedAddresses = serviceCtx == null ? null + : serviceCtx.proxiedAddresses(); + // DirectProxyConfig does not have proxyAddress as its field. if (proxyConfig.proxyAddress() != null) { - final Future resolveFuture = addressResolverGroup.getResolver( - ctx.eventLoop().withoutContext()).resolve(proxyConfig.proxyAddress()); + final Future resolveFuture = addressResolverGroup + .getResolver(ctx.eventLoop().withoutContext()) + .resolve(proxyConfig.proxyAddress()); resolveFuture.addListener(future -> { if (future.isSuccess()) { - final InetSocketAddress resolvedAddress = (InetSocketAddress) future.get(); + final InetSocketAddress resolvedAddress = (InetSocketAddress) future.getNow(); if (resolvedAddress != null && !resolvedAddress.isUnresolved()) { - proxyConfig.refreshDns(resolvedAddress); + final ProxyConfig newProxyConfig = proxyConfig.withNewProxyAddress(resolvedAddress); + onComplete.accept(maybeHAProxy(newProxyConfig, capturedProxiedAddresses), null); } else { logger.warn("Resolved address is invalid or unresolved: {}. " + - "Using previous address instead.", resolvedAddress); + "Using the previous address instead.", resolvedAddress); + onComplete.accept(maybeHAProxy(proxyConfig, capturedProxiedAddresses), null); } } else { final Throwable cause = future.cause(); logger.warn("Failed to refresh {}'s ip address. " + - "Use the previous inet address instead. reason: {}", + "Using the previous address instead. reason: {}", proxyConfig.proxyAddress(), cause != null ? cause.getMessage() : "Unknown Error"); + onComplete.accept(maybeHAProxy(proxyConfig, capturedProxiedAddresses), null); } }); + } else { + onComplete.accept(maybeHAProxy(proxyConfig, capturedProxiedAddresses), null); } + } + private ProxyConfig maybeHAProxy(ProxyConfig proxyConfig, @Nullable ProxiedAddresses proxiedAddresses) { // special behavior for haproxy when sourceAddress is null if (proxyConfig.proxyType() == ProxyType.HAPROXY && ((HAProxyConfig) proxyConfig).sourceAddress() == null) { @@ -254,13 +278,10 @@ private ProxyConfig getProxyConfig(SessionProtocol protocol, Endpoint endpoint, assert proxyAddress != null; // use proxy information in context if available - final ServiceRequestContext serviceCtx = ServiceRequestContext.currentOrNull(); - if (serviceCtx != null) { - final ProxiedAddresses proxiedAddresses = serviceCtx.proxiedAddresses(); + if (proxiedAddresses != null) { return ProxyConfig.haproxy(proxyAddress, proxiedAddresses.sourceAddress()); } } - return proxyConfig; } @@ -275,9 +296,14 @@ private static void logSession(ClientRequestContext ctx, @Nullable PooledChannel } } - private static HttpResponse earlyFailedResponse(Throwable t, ClientRequestContext ctx) { + private static Throwable earlyFailed(Throwable t, ClientRequestContext ctx) { final UnprocessedRequestException cause = UnprocessedRequestException.of(t); ctx.cancel(cause); + return cause; + } + + private static HttpResponse earlyFailedResponse(Throwable t, ClientRequestContext ctx) { + final Throwable cause = earlyFailed(t, ctx); return HttpResponse.ofFailure(cause); } diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java index cb8453774ab..cc6128f17ff 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java @@ -76,9 +76,8 @@ public void refreshDns(InetSocketAddress socketAddress) { @Override public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { - return this.sourceAddress == null ? - new HAProxyConfig(proxyAddress) : - new HAProxyConfig(proxyAddress, this.sourceAddress); + return this.sourceAddress == null ? new HAProxyConfig(proxyAddress) + : new HAProxyConfig(proxyAddress, this.sourceAddress); } @Override diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java index b146d214a86..38fda7c1ee1 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java @@ -207,6 +207,10 @@ public static ProxyConfig direct() { @Nullable public abstract InetSocketAddress proxyAddress(); + /** + * Returns a new proxy address instance that respects DNS TTL. + * @param newProxyAddress the inet socket address + */ public abstract ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress); @Nullable From 0b3cc2b3aa1ed9bf457a5a1dc13c1f876a8e03a2 Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Sun, 3 Nov 2024 14:15:01 +0900 Subject: [PATCH 08/23] Remove useless method from ProxyConfig. --- .../linecorp/armeria/client/proxy/ConnectProxyConfig.java | 6 ------ .../linecorp/armeria/client/proxy/DirectProxyConfig.java | 5 ----- .../com/linecorp/armeria/client/proxy/HAProxyConfig.java | 6 ------ .../java/com/linecorp/armeria/client/proxy/ProxyConfig.java | 6 ------ .../linecorp/armeria/client/proxy/Socks4ProxyConfig.java | 6 ------ .../linecorp/armeria/client/proxy/Socks5ProxyConfig.java | 6 ------ 6 files changed, 35 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java index 25ecdefba25..b3a035ad48a 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java @@ -92,12 +92,6 @@ public ProxyType proxyType() { return ProxyType.CONNECT; } - @Override - public void refreshDns(InetSocketAddress socketAddress) { - requireNonNull(socketAddress, "socketAddress"); - this.proxyAddress = socketAddress; - } - @Override public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { return new ConnectProxyConfig(newProxyAddress, this.username, diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java index 8383edcfe68..a00a869ebf0 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java @@ -40,11 +40,6 @@ public InetSocketAddress proxyAddress() { return null; } - @Override - public void refreshDns(InetSocketAddress socketAddress) { - // Do nothing. - } - @Override public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { return new DirectProxyConfig(); diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java index cc6128f17ff..75218674d3e 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java @@ -68,12 +68,6 @@ public InetSocketAddress sourceAddress() { return sourceAddress; } - @Override - public void refreshDns(InetSocketAddress socketAddress) { - requireNonNull(socketAddress, "socketAddress"); - this.proxyAddress = socketAddress; - } - @Override public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { return this.sourceAddress == null ? new HAProxyConfig(proxyAddress) diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java index 38fda7c1ee1..2253b42ba41 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java @@ -195,12 +195,6 @@ public static ProxyConfig direct() { */ public abstract ProxyType proxyType(); - /** - * Refreshes the proxyAddress which ProxyConfig has, to respect DNS TTL. - * @param socketAddress the inet socket address - */ - public abstract void refreshDns(InetSocketAddress socketAddress); - /** * Returns the proxy address which is {@code null} only for {@link ProxyType#DIRECT}. */ diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java index 090913ecbcb..d44b1331d88 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java @@ -58,12 +58,6 @@ public ProxyType proxyType() { return ProxyType.SOCKS4; } - @Override - public void refreshDns(InetSocketAddress socketAddress) { - requireNonNull(socketAddress, "socketAddress"); - this.proxyAddress = socketAddress; - } - @Override public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { return new Socks4ProxyConfig(newProxyAddress, this.username); diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java index c291d9d65ca..5c15dbc757e 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java @@ -71,12 +71,6 @@ public ProxyType proxyType() { return ProxyType.SOCKS5; } - @Override - public void refreshDns(InetSocketAddress socketAddress) { - requireNonNull(socketAddress, "socketAddress"); - this.proxyAddress = socketAddress; - } - @Override public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { return new Socks5ProxyConfig(newProxyAddress, this.username, this.password); From 54ec6f399eabd1273e8af1595922261c7709fe06 Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Sun, 3 Nov 2024 14:20:19 +0900 Subject: [PATCH 09/23] Fixes lint error. --- .../com/linecorp/armeria/client/proxy/ConnectProxyConfig.java | 2 -- .../java/com/linecorp/armeria/client/proxy/HAProxyConfig.java | 1 - .../com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java | 2 -- .../com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java | 2 -- 4 files changed, 7 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java index b3a035ad48a..d012920f114 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java @@ -16,8 +16,6 @@ package com.linecorp.armeria.client.proxy; -import static java.util.Objects.requireNonNull; - import java.net.InetSocketAddress; import java.util.Objects; diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java index 75218674d3e..f804b1c1209 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java @@ -17,7 +17,6 @@ package com.linecorp.armeria.client.proxy; import static com.google.common.base.Preconditions.checkArgument; -import static java.util.Objects.requireNonNull; import java.net.InetSocketAddress; import java.util.Objects; diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java index d44b1331d88..07e9b90e474 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java @@ -16,8 +16,6 @@ package com.linecorp.armeria.client.proxy; -import static java.util.Objects.requireNonNull; - import java.net.InetSocketAddress; import java.util.Objects; diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java index 5c15dbc757e..cc50d9a814a 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java @@ -16,8 +16,6 @@ package com.linecorp.armeria.client.proxy; -import static java.util.Objects.requireNonNull; - import java.net.InetSocketAddress; import java.util.Objects; From a67ee3faf4dc3f7062727253b6538c11cf1d80ca Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Sun, 3 Nov 2024 14:21:56 +0900 Subject: [PATCH 10/23] Revert previous commit. --- .../com/linecorp/armeria/client/proxy/ConnectProxyConfig.java | 2 +- .../java/com/linecorp/armeria/client/proxy/HAProxyConfig.java | 2 +- .../com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java | 2 +- .../com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java index d012920f114..8b7c27c61e1 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java @@ -29,7 +29,7 @@ */ public final class ConnectProxyConfig extends ProxyConfig { - private InetSocketAddress proxyAddress; + private final InetSocketAddress proxyAddress; @Nullable private final String username; diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java index f804b1c1209..57a556580f0 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java @@ -31,7 +31,7 @@ */ public final class HAProxyConfig extends ProxyConfig { - private InetSocketAddress proxyAddress; + private final InetSocketAddress proxyAddress; @Nullable private final InetSocketAddress sourceAddress; diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java index 07e9b90e474..19c89f0ac5b 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java @@ -28,7 +28,7 @@ */ public final class Socks4ProxyConfig extends ProxyConfig { - private InetSocketAddress proxyAddress; + private final InetSocketAddress proxyAddress; @Nullable private final String username; diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java index cc50d9a814a..f691205aa84 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java @@ -28,7 +28,7 @@ */ public final class Socks5ProxyConfig extends ProxyConfig { - private InetSocketAddress proxyAddress; + private final InetSocketAddress proxyAddress; @Nullable private final String username; From 241b4ff4c967d67b4978643d152610ed0766a100 Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Mon, 4 Nov 2024 21:32:12 +0900 Subject: [PATCH 11/23] Remove useless log codes. --- .../armeria/client/HttpClientDelegate.java | 17 +++-------------- .../armeria/client/proxy/DirectProxyConfig.java | 4 +++- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java index 3f9406015e0..84ce3f4b323 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java @@ -53,7 +53,6 @@ final class HttpClientDelegate implements HttpClient { - private static final Logger logger = LoggerFactory.getLogger(HttpClientDelegate.class); private final HttpClientFactory factory; private final AddressResolverGroup addressResolverGroup; @@ -248,21 +247,11 @@ private void resolveProxyConfig(SessionProtocol protocol, Endpoint endpoint, Cli resolveFuture.addListener(future -> { if (future.isSuccess()) { final InetSocketAddress resolvedAddress = (InetSocketAddress) future.getNow(); - if (resolvedAddress != null && !resolvedAddress.isUnresolved()) { - final ProxyConfig newProxyConfig = proxyConfig.withNewProxyAddress(resolvedAddress); - onComplete.accept(maybeHAProxy(newProxyConfig, capturedProxiedAddresses), null); - } else { - logger.warn("Resolved address is invalid or unresolved: {}. " + - "Using the previous address instead.", resolvedAddress); - onComplete.accept(maybeHAProxy(proxyConfig, capturedProxiedAddresses), null); - } + final ProxyConfig newProxyConfig = proxyConfig.withNewProxyAddress(resolvedAddress); + onComplete.accept(maybeHAProxy(newProxyConfig, capturedProxiedAddresses), null); } else { final Throwable cause = future.cause(); - logger.warn("Failed to refresh {}'s ip address. " + - "Using the previous address instead. reason: {}", - proxyConfig.proxyAddress(), - cause != null ? cause.getMessage() : "Unknown Error"); - onComplete.accept(maybeHAProxy(proxyConfig, capturedProxiedAddresses), null); + onComplete.accept(maybeHAProxy(proxyConfig, capturedProxiedAddresses), cause); } }); } else { diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java index a00a869ebf0..288a7254974 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java @@ -20,6 +20,8 @@ import com.linecorp.armeria.common.annotation.Nullable; +import sun.reflect.generics.reflectiveObjects.NotImplementedException; + /** * Represents a direct connection without a proxy. */ @@ -42,7 +44,7 @@ public InetSocketAddress proxyAddress() { @Override public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { - return new DirectProxyConfig(); + throw new UnsupportedOperationException("The method withNewProxyAddress(...) is not for DirectProxyConfig."); } @Override From fce4fd25b39260d71cbf48ed43b4cfdccbf3df02 Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Mon, 4 Nov 2024 21:36:03 +0900 Subject: [PATCH 12/23] Remove unused import. --- .../java/com/linecorp/armeria/client/HttpClientDelegate.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java index 84ce3f4b323..8da6bc0fa9e 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java @@ -21,9 +21,6 @@ import java.net.InetSocketAddress; import java.util.function.BiConsumer; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import com.linecorp.armeria.client.HttpChannelPool.PoolKey; import com.linecorp.armeria.client.endpoint.EmptyEndpointGroupException; import com.linecorp.armeria.client.proxy.HAProxyConfig; From 7d85a69715d7bca4e2632a3171444a23b8b0578e Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Mon, 4 Nov 2024 22:43:35 +0900 Subject: [PATCH 13/23] Fixes lint error. --- .../com/linecorp/armeria/client/proxy/DirectProxyConfig.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java index 288a7254974..42f38fe9068 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java @@ -20,8 +20,6 @@ import com.linecorp.armeria.common.annotation.Nullable; -import sun.reflect.generics.reflectiveObjects.NotImplementedException; - /** * Represents a direct connection without a proxy. */ @@ -44,7 +42,8 @@ public InetSocketAddress proxyAddress() { @Override public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { - throw new UnsupportedOperationException("The method withNewProxyAddress(...) is not for DirectProxyConfig."); + throw new UnsupportedOperationException( + "The method withNewProxyAddress(...) is not for DirectProxyConfig."); } @Override From 67f49978daf35a5ea295b266ad1596a0f2aa54bc Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Tue, 5 Nov 2024 00:26:08 +0900 Subject: [PATCH 14/23] Add integration test for DNS refreshing. --- .../armeria/client/proxy/ProxyConfig.java | 10 --- .../client/DNSResolverFacadeUtils.java | 66 ++++++++++++++++++ .../proxy/ProxyClientIntegrationTest.java | 69 +++++++++++++++++++ 3 files changed, 135 insertions(+), 10 deletions(-) create mode 100644 core/src/test/java/com/linecorp/armeria/client/DNSResolverFacadeUtils.java diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java index 2253b42ba41..14e70972cff 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java @@ -40,7 +40,6 @@ public abstract class ProxyConfig { */ public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new Socks4ProxyConfig(proxyAddress, null); } @@ -52,7 +51,6 @@ public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress) { */ public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress, String username) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new Socks4ProxyConfig(proxyAddress, requireNonNull(username, "username")); } @@ -63,7 +61,6 @@ public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress, String us */ public static Socks5ProxyConfig socks5(InetSocketAddress proxyAddress) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new Socks5ProxyConfig(proxyAddress, null, null); } @@ -77,7 +74,6 @@ public static Socks5ProxyConfig socks5(InetSocketAddress proxyAddress) { public static Socks5ProxyConfig socks5( InetSocketAddress proxyAddress, String username, String password) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new Socks5ProxyConfig(proxyAddress, requireNonNull(username, "username"), requireNonNull(password, "password")); } @@ -89,7 +85,6 @@ public static Socks5ProxyConfig socks5( */ public static ConnectProxyConfig connect(InetSocketAddress proxyAddress) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new ConnectProxyConfig(proxyAddress, null, null, HttpHeaders.of(), false); } @@ -101,7 +96,6 @@ public static ConnectProxyConfig connect(InetSocketAddress proxyAddress) { */ public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, boolean useTls) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new ConnectProxyConfig(proxyAddress, null, null, HttpHeaders.of(), useTls); } @@ -129,7 +123,6 @@ public static ConnectProxyConfig connect( public static ConnectProxyConfig connect( InetSocketAddress proxyAddress, HttpHeaders headers, boolean useTls) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new ConnectProxyConfig(proxyAddress, null, null, headers, useTls); } @@ -146,7 +139,6 @@ public static ConnectProxyConfig connect( public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, String username, String password, HttpHeaders headers, boolean useTls) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); requireNonNull(username, "username"); requireNonNull(password, "password"); requireNonNull(headers, "headers"); @@ -162,7 +154,6 @@ public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, String public static HAProxyConfig haproxy( InetSocketAddress proxyAddress, InetSocketAddress sourceAddress) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); requireNonNull(sourceAddress, "sourceAddress"); checkArgument(!sourceAddress.isUnresolved(), "sourceAddress must be resolved"); return new HAProxyConfig(proxyAddress, sourceAddress); @@ -176,7 +167,6 @@ public static HAProxyConfig haproxy( */ public static ProxyConfig haproxy(InetSocketAddress proxyAddress) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new HAProxyConfig(proxyAddress); } diff --git a/core/src/test/java/com/linecorp/armeria/client/DNSResolverFacadeUtils.java b/core/src/test/java/com/linecorp/armeria/client/DNSResolverFacadeUtils.java new file mode 100644 index 00000000000..c0a0eaf2d0e --- /dev/null +++ b/core/src/test/java/com/linecorp/armeria/client/DNSResolverFacadeUtils.java @@ -0,0 +1,66 @@ +/* + * Copyright 2024 LINE Corporation + * + * LINE Corporation 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: + * + * https://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 com.linecorp.armeria.client; + +import static com.google.common.collect.ImmutableList.toImmutableList; + +import java.net.InetSocketAddress; +import java.util.function.Function; +import java.util.stream.Stream; + +import com.linecorp.armeria.client.endpoint.dns.TestDnsServer; +import com.linecorp.armeria.client.retry.Backoff; +import com.linecorp.armeria.common.prometheus.PrometheusMeterRegistries; + +import io.netty.channel.EventLoopGroup; +import io.netty.resolver.AddressResolverGroup; +import io.netty.resolver.ResolvedAddressTypes; +import io.netty.resolver.dns.DnsServerAddressStreamProvider; +import io.netty.resolver.dns.DnsServerAddresses; + +public final class DNSResolverFacadeUtils { + + private DNSResolverFacadeUtils() { } + + public static Function> getAddressResolverGroupForTest( + TestDnsServer dnsServer) { + return eventLoopGroup -> { + final DnsResolverGroupBuilder builder = builder(dnsServer); + builder.autoRefreshBackoff(Backoff.fixed(0L)); + return builder.build(eventLoopGroup); + }; + } + + private static DnsResolverGroupBuilder builder(TestDnsServer... servers) { + return builder(true, servers); + } + + private static DnsResolverGroupBuilder builder(boolean withCacheOption, TestDnsServer... servers) { + final DnsServerAddressStreamProvider dnsServerAddressStreamProvider = + hostname -> DnsServerAddresses.sequential( + Stream.of(servers).map(TestDnsServer::addr).collect(toImmutableList())).stream(); + final DnsResolverGroupBuilder builder = new DnsResolverGroupBuilder() + .serverAddressStreamProvider(dnsServerAddressStreamProvider) + .meterRegistry(PrometheusMeterRegistries.newRegistry()) + .resolvedAddressTypes(ResolvedAddressTypes.IPV4_ONLY) + .traceEnabled(false); + if (withCacheOption) { + builder.dnsCache(DnsCache.builder().build()); + } + return builder; + } +} diff --git a/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyClientIntegrationTest.java b/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyClientIntegrationTest.java index 932c15197a1..3378dc9e440 100644 --- a/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyClientIntegrationTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyClientIntegrationTest.java @@ -15,6 +15,9 @@ */ package com.linecorp.armeria.client.proxy; +import static com.linecorp.armeria.client.endpoint.dns.TestDnsServer.newAddressRecord; +import static io.netty.handler.codec.dns.DnsRecordType.A; +import static io.netty.handler.codec.dns.DnsSection.ANSWER; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.awaitility.Awaitility.await; @@ -51,12 +54,15 @@ import org.junit.jupiter.params.provider.MethodSource; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.linecorp.armeria.client.ClientFactory; +import com.linecorp.armeria.client.DNSResolverFacadeUtils; import com.linecorp.armeria.client.Endpoint; import com.linecorp.armeria.client.SessionProtocolNegotiationException; import com.linecorp.armeria.client.UnprocessedRequestException; import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.client.endpoint.dns.TestDnsServer; import com.linecorp.armeria.client.logging.LoggingClient; import com.linecorp.armeria.common.AggregatedHttpResponse; import com.linecorp.armeria.common.Flags; @@ -75,6 +81,8 @@ import io.netty.channel.Channel; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; +import io.netty.handler.codec.dns.DefaultDnsQuestion; +import io.netty.handler.codec.dns.DefaultDnsResponse; import io.netty.handler.codec.http.DefaultFullHttpResponse; import io.netty.handler.codec.http.DefaultHttpHeaders; import io.netty.handler.codec.http.EmptyHttpHeaders; @@ -95,6 +103,7 @@ import io.netty.handler.ssl.SslContext; import io.netty.handler.ssl.SslContextBuilder; import io.netty.handler.traffic.ChannelTrafficShapingHandler; +import io.netty.resolver.dns.DnsErrorCauseException; import io.netty.util.ReferenceCountUtil; class ProxyClientIntegrationTest { @@ -790,6 +799,66 @@ public void connectFailed(SessionProtocol protocol, Endpoint endpoint, } } + @Test + void testProxyConfigShouldBeResolved() throws Exception { + try (TestDnsServer server = new TestDnsServer(ImmutableMap.of( + new DefaultDnsQuestion("a.com.", A), + new DefaultDnsResponse(0).addRecord(ANSWER, newAddressRecord("a.com.", "127.0.0.1")))) + ) { + final InetSocketAddress proxySocketAddress = new InetSocketAddress( + "a.com", httpProxyServer.address().getPort()); + try (ClientFactory clientFactory = + ClientFactory.builder() + .addressResolverGroupFactory( + DNSResolverFacadeUtils.getAddressResolverGroupForTest(server)) + .proxyConfig(ProxyConfig.connect(proxySocketAddress)) + .useHttp2Preface(true) + .build()) { + + final WebClient webClient = WebClient.builder(SessionProtocol.H1C, backendServer.httpEndpoint()) + .factory(clientFactory) + .decorator(LoggingClient.newDecorator()) + .build(); + final CompletableFuture responseFuture = + webClient.get(PROXY_PATH).aggregate(); + final AggregatedHttpResponse response = responseFuture.join(); + assertThat(response.status()).isEqualTo(HttpStatus.OK); + assertThat(response.contentUtf8()).isEqualTo(SUCCESS_RESPONSE); + assertThat(numSuccessfulProxyRequests).isEqualTo(1); + } + } + } + + @Test + void testProxyConfigShouldBeFailedToResolved() throws Exception { + try (TestDnsServer server = new TestDnsServer(ImmutableMap.of( + new DefaultDnsQuestion("b.com.", A), + new DefaultDnsResponse(0).addRecord(ANSWER, newAddressRecord("b.com.", "127.0.0.1")))) + ) { + final InetSocketAddress proxySocketAddress = new InetSocketAddress( + "a.com", httpProxyServer.address().getPort()); + try (ClientFactory clientFactory = + ClientFactory.builder() + .addressResolverGroupFactory( + DNSResolverFacadeUtils.getAddressResolverGroupForTest(server)) + .proxyConfig(ProxyConfig.connect(proxySocketAddress)) + .useHttp2Preface(true) + .build()) { + + final WebClient webClient = WebClient.builder(SessionProtocol.H1C, backendServer.httpEndpoint()) + .factory(clientFactory) + .decorator(LoggingClient.newDecorator()) + .build(); + final CompletableFuture responseFuture = + webClient.get(PROXY_PATH).aggregate(); + assertThatThrownBy(responseFuture::join).isInstanceOf(CompletionException.class) + .hasCauseInstanceOf(UnprocessedRequestException.class) + .hasRootCauseInstanceOf(DnsErrorCauseException.class) + .hasRootCauseMessage("Query failed with NXDOMAIN"); + } + } + } + private static final class SleepHandler extends ChannelInboundHandlerAdapter { @Override From e7753515ba9abe24cd66c5385b7e4271251b3823 Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Tue, 5 Nov 2024 08:06:14 +0900 Subject: [PATCH 15/23] Remove deprecated test case. --- .../armeria/client/proxy/ProxyConfigTest.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyConfigTest.java b/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyConfigTest.java index 1c0b8c68bc7..146911555cd 100644 --- a/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyConfigTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyConfigTest.java @@ -44,19 +44,6 @@ void testEqualityAndHashCode(ProxyConfig config) { assertThat(equalProxyConfigs.get(0).hashCode()).isEqualTo(config.hashCode()); } - @Test - void testUnresolvedProxyAddress() { - final InetSocketAddress unresolved = InetSocketAddress.createUnresolved("unresolved", 0); - final InetSocketAddress resolved = new InetSocketAddress("127.0.0.1", 80); - assertThatThrownBy(() -> ProxyConfig.socks4(unresolved)).isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> ProxyConfig.socks5(unresolved)).isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> ProxyConfig.connect(unresolved)).isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> ProxyConfig.haproxy(unresolved, resolved)) - .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> ProxyConfig.haproxy(resolved, unresolved)) - .isInstanceOf(IllegalArgumentException.class); - } - @Test void testNullProxyAddress() { assertThatThrownBy(() -> ProxyConfig.socks4(null)).isInstanceOf(NullPointerException.class); From 18b24ea6ecf5e29f51421ba66296507352e3ca76 Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Mon, 11 Nov 2024 23:43:11 +0900 Subject: [PATCH 16/23] Apply reviews. --- .../armeria/client/HttpClientDelegate.java | 24 +++++++++-------- .../client/proxy/ConnectProxyConfig.java | 2 +- .../client/proxy/DirectProxyConfig.java | 4 +-- .../armeria/client/proxy/HAProxyConfig.java | 26 ++++++++++++++++--- .../armeria/client/proxy/ProxyConfig.java | 2 +- .../client/proxy/Socks4ProxyConfig.java | 2 +- .../client/proxy/Socks5ProxyConfig.java | 2 +- .../armeria/client/proxy/ProxyConfigTest.java | 19 ++++++++++++++ 8 files changed, 60 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java index 8da6bc0fa9e..b0c4b4eb7ef 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java @@ -98,8 +98,7 @@ public HttpResponse execute(ClientRequestContext ctx, HttpRequest req) throws Ex try { resolveProxyConfig(protocol, endpoint, ctx, (proxyConfig, thrown) -> { if (thrown != null) { - earlyFailed(thrown, ctx); - res.close(thrown); + earlyFailedResponse(thrown, ctx, res); } else { execute0(ctx, endpointWithPort, req, res, proxyConfig); } @@ -114,8 +113,7 @@ private void execute0(ClientRequestContext ctx, Endpoint endpointWithPort, HttpR DecodedHttpResponse res, ProxyConfig proxyConfig) { final Throwable cancellationCause = ctx.cancellationCause(); if (cancellationCause != null) { - final Throwable t = earlyFailed(cancellationCause, ctx); - res.close(t); + earlyFailedResponse(cancellationCause, ctx, res); return; } @@ -244,11 +242,11 @@ private void resolveProxyConfig(SessionProtocol protocol, Endpoint endpoint, Cli resolveFuture.addListener(future -> { if (future.isSuccess()) { final InetSocketAddress resolvedAddress = (InetSocketAddress) future.getNow(); - final ProxyConfig newProxyConfig = proxyConfig.withNewProxyAddress(resolvedAddress); + final ProxyConfig newProxyConfig = proxyConfig.withProxyAddress(resolvedAddress); onComplete.accept(maybeHAProxy(newProxyConfig, capturedProxiedAddresses), null); } else { final Throwable cause = future.cause(); - onComplete.accept(maybeHAProxy(proxyConfig, capturedProxiedAddresses), cause); + onComplete.accept(null, cause); } }); } else { @@ -282,15 +280,19 @@ private static void logSession(ClientRequestContext ctx, @Nullable PooledChannel } } - private static Throwable earlyFailed(Throwable t, ClientRequestContext ctx) { + private static HttpResponse earlyFailedResponse(Throwable t, ClientRequestContext ctx) { final UnprocessedRequestException cause = UnprocessedRequestException.of(t); ctx.cancel(cause); - return cause; + return HttpResponse.ofFailure(cause); } - private static HttpResponse earlyFailedResponse(Throwable t, ClientRequestContext ctx) { - final Throwable cause = earlyFailed(t, ctx); - return HttpResponse.ofFailure(cause); + private static HttpResponse earlyFailedResponse(Throwable t, + ClientRequestContext ctx, + DecodedHttpResponse response) { + final UnprocessedRequestException cause = UnprocessedRequestException.of(t); + ctx.cancel(cause); + response.close(cause); + return response; } private static void doExecute(PooledChannel pooledChannel, ClientRequestContext ctx, diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java index 8b7c27c61e1..7cb08d5dda2 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java @@ -91,7 +91,7 @@ public ProxyType proxyType() { } @Override - public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { + public ProxyConfig withProxyAddress(InetSocketAddress newProxyAddress) { return new ConnectProxyConfig(newProxyAddress, this.username, this.password, this.headers, this.useTls); } diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java index 42f38fe9068..7eb33e83ee0 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java @@ -41,9 +41,9 @@ public InetSocketAddress proxyAddress() { } @Override - public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { + public ProxyConfig withProxyAddress(InetSocketAddress newProxyAddress) { throw new UnsupportedOperationException( - "The method withNewProxyAddress(...) is not for DirectProxyConfig."); + "A proxy address can't be set to DirectProxyConfig."); } @Override diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java index 57a556580f0..f31ad6449aa 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java @@ -17,10 +17,15 @@ package com.linecorp.armeria.client.proxy; import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.hash; +import static java.util.Objects.requireNonNull; import java.net.InetSocketAddress; import java.util.Objects; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.google.common.base.MoreObjects; import com.linecorp.armeria.common.annotation.Nullable; @@ -31,6 +36,8 @@ */ public final class HAProxyConfig extends ProxyConfig { + private static final Logger logger = LoggerFactory.getLogger(HAProxyConfig.class); + private final InetSocketAddress proxyAddress; @Nullable @@ -42,8 +49,18 @@ public final class HAProxyConfig extends ProxyConfig { } HAProxyConfig(InetSocketAddress proxyAddress, InetSocketAddress sourceAddress) { - checkArgument(sourceAddress.getAddress().getClass() == proxyAddress.getAddress().getClass(), - "sourceAddress and proxyAddress should be the same type"); + if (sourceAddress.getAddress() != null && + proxyAddress.getAddress() != null) { + checkArgument(sourceAddress.getAddress().getClass() == proxyAddress.getAddress().getClass(), + "sourceAddress and proxyAddress should be the same type"); + } else { + logger.warn("Either the source or proxy address could not be resolved. " + + "The proxy address may be resolved later. " + + "proxyAddress: {}, sourceAddress: {}", + proxyAddress.getAddress(), + sourceAddress.getAddress()); + } + this.proxyAddress = proxyAddress; this.sourceAddress = sourceAddress; } @@ -68,7 +85,8 @@ public InetSocketAddress sourceAddress() { } @Override - public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { + public ProxyConfig withProxyAddress(InetSocketAddress newProxyAddress) { + requireNonNull(newProxyAddress, "newProxyAddress"); return this.sourceAddress == null ? new HAProxyConfig(proxyAddress) : new HAProxyConfig(proxyAddress, this.sourceAddress); } @@ -88,7 +106,7 @@ public boolean equals(@Nullable Object o) { @Override public int hashCode() { - return Objects.hash(proxyAddress, sourceAddress); + return hash(proxyAddress, sourceAddress); } @Override diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java index 14e70972cff..e26c2590b01 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java @@ -195,7 +195,7 @@ public static ProxyConfig direct() { * Returns a new proxy address instance that respects DNS TTL. * @param newProxyAddress the inet socket address */ - public abstract ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress); + public abstract ProxyConfig withProxyAddress(InetSocketAddress newProxyAddress); @Nullable static String maskPassword(@Nullable String username, @Nullable String password) { diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java index 19c89f0ac5b..d8f76ef6fa5 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java @@ -57,7 +57,7 @@ public ProxyType proxyType() { } @Override - public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { + public ProxyConfig withProxyAddress(InetSocketAddress newProxyAddress) { return new Socks4ProxyConfig(newProxyAddress, this.username); } diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java index f691205aa84..b100ff90439 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java @@ -70,7 +70,7 @@ public ProxyType proxyType() { } @Override - public ProxyConfig withNewProxyAddress(InetSocketAddress newProxyAddress) { + public ProxyConfig withProxyAddress(InetSocketAddress newProxyAddress) { return new Socks5ProxyConfig(newProxyAddress, this.username, this.password); } diff --git a/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyConfigTest.java b/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyConfigTest.java index 146911555cd..295cf96c6b3 100644 --- a/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyConfigTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyConfigTest.java @@ -44,6 +44,25 @@ void testEqualityAndHashCode(ProxyConfig config) { assertThat(equalProxyConfigs.get(0).hashCode()).isEqualTo(config.hashCode()); } + @Test + void testUnresolvedProxyAddress() { + final InetSocketAddress unresolved = InetSocketAddress.createUnresolved("unresolved", 0); + final InetSocketAddress resolved = new InetSocketAddress("127.0.0.1", 80); + + assertThat(ProxyConfig.socks4(unresolved)).isInstanceOf(Socks4ProxyConfig.class); + assertThat(ProxyConfig.socks5(unresolved)).isInstanceOf(Socks5ProxyConfig.class); + assertThat(ProxyConfig.connect(unresolved)).isInstanceOf(ConnectProxyConfig.class); + + // for the HAProxy. + assertThat(ProxyConfig.haproxy(unresolved, resolved)).isInstanceOf(HAProxyConfig.class); + assertThat(ProxyConfig.haproxy(unresolved)).isInstanceOf(HAProxyConfig.class); + assertThat(ProxyConfig.haproxy(resolved)).isInstanceOf(HAProxyConfig.class); + assertThatThrownBy( + () -> ProxyConfig.haproxy(resolved, unresolved)).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy( + () -> ProxyConfig.haproxy(unresolved, unresolved)).isInstanceOf(IllegalArgumentException.class); + } + @Test void testNullProxyAddress() { assertThatThrownBy(() -> ProxyConfig.socks4(null)).isInstanceOf(NullPointerException.class); From 979e6fdef3cac69893637855b9f9de2c8294b9dc Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Sun, 17 Nov 2024 11:05:55 +0900 Subject: [PATCH 17/23] Add a private method to refresh proxy DNS. --- .../com/linecorp/armeria/client/HttpClientDelegate.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java index b0c4b4eb7ef..26e063db542 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java @@ -237,7 +237,7 @@ private void resolveProxyConfig(SessionProtocol protocol, Endpoint endpoint, Cli if (proxyConfig.proxyAddress() != null) { final Future resolveFuture = addressResolverGroup .getResolver(ctx.eventLoop().withoutContext()) - .resolve(proxyConfig.proxyAddress()); + .resolve(createUnresolvedAddressForRefreshing(proxyConfig.proxyAddress())); resolveFuture.addListener(future -> { if (future.isSuccess()) { @@ -302,4 +302,8 @@ private static void doExecute(PooledChannel pooledChannel, ClientRequestContext res.init(session.inboundTrafficController()); session.invoke(pooledChannel, ctx, req, res); } + + private static InetSocketAddress createUnresolvedAddressForRefreshing(InetSocketAddress previousAddress) { + return new InetSocketAddress(previousAddress.getHostName(), previousAddress.getPort()); + } } From 3dbc847ffb7ab80ee2415b05d312de59ea9db00f Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Wed, 20 Nov 2024 21:55:34 +0900 Subject: [PATCH 18/23] Skip refreshing ProxyConfig DNS when it is configured with a direct IP. --- .../armeria/client/HttpClientDelegate.java | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java index 26e063db542..8c258fd017d 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java @@ -232,23 +232,28 @@ private void resolveProxyConfig(SessionProtocol protocol, Endpoint endpoint, Cli final ServiceRequestContext serviceCtx = ServiceRequestContext.currentOrNull(); final ProxiedAddresses capturedProxiedAddresses = serviceCtx == null ? null : serviceCtx.proxiedAddresses(); - // DirectProxyConfig does not have proxyAddress as its field. if (proxyConfig.proxyAddress() != null) { - final Future resolveFuture = addressResolverGroup - .getResolver(ctx.eventLoop().withoutContext()) - .resolve(createUnresolvedAddressForRefreshing(proxyConfig.proxyAddress())); - - resolveFuture.addListener(future -> { - if (future.isSuccess()) { - final InetSocketAddress resolvedAddress = (InetSocketAddress) future.getNow(); - final ProxyConfig newProxyConfig = proxyConfig.withProxyAddress(resolvedAddress); - onComplete.accept(maybeHAProxy(newProxyConfig, capturedProxiedAddresses), null); - } else { - final Throwable cause = future.cause(); - onComplete.accept(null, cause); - } - }); + final InetSocketAddress currentAddr = proxyConfig.proxyAddress(); + if (!currentAddr.isUnresolved() && (currentAddr.getHostName() == currentAddr.getHostString())) { + // If InetSocket is created from IP Address, we don't need to refresh DNS. + onComplete.accept(maybeHAProxy(proxyConfig, capturedProxiedAddresses), null); + } else { + final Future resolveFuture = addressResolverGroup + .getResolver(ctx.eventLoop().withoutContext()) + .resolve(createUnresolvedAddressForRefreshing(proxyConfig.proxyAddress())); + + resolveFuture.addListener(future -> { + if (future.isSuccess()) { + final InetSocketAddress resolvedAddress = (InetSocketAddress) future.getNow(); + final ProxyConfig newProxyConfig = proxyConfig.withProxyAddress(resolvedAddress); + onComplete.accept(maybeHAProxy(newProxyConfig, capturedProxiedAddresses), null); + } else { + final Throwable cause = future.cause(); + onComplete.accept(null, cause); + } + }); + } } else { onComplete.accept(maybeHAProxy(proxyConfig, capturedProxiedAddresses), null); } @@ -304,6 +309,6 @@ private static void doExecute(PooledChannel pooledChannel, ClientRequestContext } private static InetSocketAddress createUnresolvedAddressForRefreshing(InetSocketAddress previousAddress) { - return new InetSocketAddress(previousAddress.getHostName(), previousAddress.getPort()); + return InetSocketAddress.createUnresolved(previousAddress.getHostString(), previousAddress.getPort()); } } From fcfed4573fec4cd6d92360b8cb4bc085882fa3d4 Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Thu, 21 Nov 2024 18:59:36 +0900 Subject: [PATCH 19/23] clean up --- .../armeria/client/HttpClientDelegate.java | 71 +++++++++---------- .../internal/common/util/IpAddrUtil.java | 17 +++++ .../internal/common/util/IpAddrUtilTest.java | 48 +++++++++++++ 3 files changed, 100 insertions(+), 36 deletions(-) create mode 100644 core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java index 8c258fd017d..7c90a5a4adf 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java @@ -15,6 +15,7 @@ */ package com.linecorp.armeria.client; +import static com.linecorp.armeria.internal.common.util.IpAddrUtil.isCreatedWithIpAddressOnly; import static java.util.Objects.requireNonNull; import java.net.InetAddress; @@ -226,50 +227,48 @@ private void acquireConnectionAndExecute0(ClientRequestContext ctx, Endpoint end private void resolveProxyConfig(SessionProtocol protocol, Endpoint endpoint, ClientRequestContext ctx, BiConsumer<@Nullable ProxyConfig, @Nullable Throwable> onComplete) { - final ProxyConfig proxyConfig = factory.proxyConfigSelector().select(protocol, endpoint); - requireNonNull(proxyConfig, "proxyConfig"); + final ProxyConfig unresolvedProxyConfig = factory.proxyConfigSelector().select(protocol, endpoint); + requireNonNull(unresolvedProxyConfig, "unresolvedProxyConfig"); + final ProxyConfig proxyConfig = maybeSetHAProxySourceAddress(unresolvedProxyConfig); - final ServiceRequestContext serviceCtx = ServiceRequestContext.currentOrNull(); - final ProxiedAddresses capturedProxiedAddresses = serviceCtx == null ? null - : serviceCtx.proxiedAddresses(); - // DirectProxyConfig does not have proxyAddress as its field. - if (proxyConfig.proxyAddress() != null) { - final InetSocketAddress currentAddr = proxyConfig.proxyAddress(); - if (!currentAddr.isUnresolved() && (currentAddr.getHostName() == currentAddr.getHostString())) { - // If InetSocket is created from IP Address, we don't need to refresh DNS. - onComplete.accept(maybeHAProxy(proxyConfig, capturedProxiedAddresses), null); - } else { - final Future resolveFuture = addressResolverGroup - .getResolver(ctx.eventLoop().withoutContext()) - .resolve(createUnresolvedAddressForRefreshing(proxyConfig.proxyAddress())); + final InetSocketAddress proxyAddress = proxyConfig.proxyAddress(); + final boolean needsDnsResolution = proxyAddress != null && !isCreatedWithIpAddressOnly(proxyAddress); + if (needsDnsResolution) { + final Future resolveFuture = addressResolverGroup + .getResolver(ctx.eventLoop().withoutContext()) + .resolve(createUnresolvedAddressForRefreshing(proxyAddress)); - resolveFuture.addListener(future -> { - if (future.isSuccess()) { - final InetSocketAddress resolvedAddress = (InetSocketAddress) future.getNow(); - final ProxyConfig newProxyConfig = proxyConfig.withProxyAddress(resolvedAddress); - onComplete.accept(maybeHAProxy(newProxyConfig, capturedProxiedAddresses), null); - } else { - final Throwable cause = future.cause(); - onComplete.accept(null, cause); - } - }); - } + resolveFuture.addListener(future -> { + if (future.isSuccess()) { + final InetSocketAddress resolvedAddress = (InetSocketAddress) future.getNow(); + final ProxyConfig newProxyConfig = proxyConfig.withProxyAddress(resolvedAddress); + onComplete.accept(newProxyConfig, null); + } else { + final Throwable cause = future.cause(); + onComplete.accept(null, cause); + } + }); } else { - onComplete.accept(maybeHAProxy(proxyConfig, capturedProxiedAddresses), null); + onComplete.accept(proxyConfig, null); } } - private ProxyConfig maybeHAProxy(ProxyConfig proxyConfig, @Nullable ProxiedAddresses proxiedAddresses) { - // special behavior for haproxy when sourceAddress is null - if (proxyConfig.proxyType() == ProxyType.HAPROXY && - ((HAProxyConfig) proxyConfig).sourceAddress() == null) { + private static ProxyConfig maybeSetHAProxySourceAddress(ProxyConfig proxyConfig) { + if (proxyConfig.proxyType() != ProxyType.HAPROXY) { + return proxyConfig; + } + if (((HAProxyConfig) proxyConfig).sourceAddress() != null) { + return proxyConfig; + } + + final ServiceRequestContext sctx = ServiceRequestContext.currentOrNull(); + final ProxiedAddresses serviceProxiedAddresses = sctx == null ? null : sctx.proxiedAddresses(); + if (serviceProxiedAddresses != null) { + // A special behavior for haproxy when sourceAddress is null. + // Use proxy information in the service context if available. final InetSocketAddress proxyAddress = proxyConfig.proxyAddress(); assert proxyAddress != null; - - // use proxy information in context if available - if (proxiedAddresses != null) { - return ProxyConfig.haproxy(proxyAddress, proxiedAddresses.sourceAddress()); - } + return ProxyConfig.haproxy(proxyAddress, serviceProxiedAddresses.sourceAddress()); } return proxyConfig; } diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/util/IpAddrUtil.java b/core/src/main/java/com/linecorp/armeria/internal/common/util/IpAddrUtil.java index 3fd74fdfe5c..ea445a85938 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/util/IpAddrUtil.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/util/IpAddrUtil.java @@ -15,6 +15,9 @@ */ package com.linecorp.armeria.internal.common.util; +import java.net.InetAddress; +import java.net.InetSocketAddress; + import com.linecorp.armeria.common.annotation.Nullable; import io.netty.util.NetUtil; @@ -36,5 +39,19 @@ public static String normalize(@Nullable String ipAddr) { return NetUtil.bytesToIpAddress(array); } + public static boolean isCreatedWithIpAddressOnly(InetSocketAddress socketAddress) { + if (socketAddress.isUnresolved()) { + return false; + } + + final InetAddress inetAddress = socketAddress.getAddress(); + if (inetAddress.isAnyLocalAddress()) { + return false; // Wildcard address + } + + // If hostname and host address are the same, it was created with an IP address + return socketAddress.getHostString().equals(inetAddress.getHostAddress()); + } + private IpAddrUtil() {} } diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java new file mode 100644 index 00000000000..e92edb9cf39 --- /dev/null +++ b/core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java @@ -0,0 +1,48 @@ +/* + * Copyright 2024 LINE Corporation + * + * LINE Corporation 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: + * + * https://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 com.linecorp.armeria.internal.common.util; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.UnknownHostException; + +import org.junit.jupiter.api.Test; + +class IpAddrUtilTest { + + @Test + void testIsCreatedWithIpAddressOnly() throws UnknownHostException { + InetSocketAddress inetSocketAddress = new InetSocketAddress("foo.com", 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isFalse(); + + inetSocketAddress = InetSocketAddress.createUnresolved("foo.com", 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isFalse(); + + inetSocketAddress = new InetSocketAddress("1.2.3.4", 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isTrue(); + + InetAddress inetAddress = InetAddress.getByName("1.2.3.4"); + inetSocketAddress = new InetSocketAddress(inetAddress, 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isTrue(); + + inetAddress = InetAddress.getByAddress("foo.com", new byte[] { 1, 2, 3, 4 }); + inetSocketAddress = new InetSocketAddress(inetAddress, 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isFalse(); + } +} From 09efa86cb96503ad55ce800feab67442f36c4ba3 Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Thu, 21 Nov 2024 19:06:37 +0900 Subject: [PATCH 20/23] add more tests --- .../linecorp/armeria/internal/common/util/IpAddrUtilTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java index e92edb9cf39..1c4b01b1266 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java @@ -44,5 +44,9 @@ void testIsCreatedWithIpAddressOnly() throws UnknownHostException { inetAddress = InetAddress.getByAddress("foo.com", new byte[] { 1, 2, 3, 4 }); inetSocketAddress = new InetSocketAddress(inetAddress, 8080); assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isFalse(); + + inetAddress = InetAddress.getByName("foo.com"); + inetSocketAddress = new InetSocketAddress(inetAddress, 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isFalse(); } } From 25ff1af6ca87cd60e2b2286a159ac3b286a48777 Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Thu, 21 Nov 2024 21:55:47 +0900 Subject: [PATCH 21/23] Fixes broken test codes and add new test cases. --- .../com/linecorp/armeria/client/HttpClientDelegate.java | 1 + .../com/linecorp/armeria/client/proxy/HAProxyConfig.java | 7 +++---- .../linecorp/armeria/internal/common/util/IpAddrUtil.java | 4 ---- .../armeria/client/proxy/ProxyClientIntegrationTest.java | 2 +- .../armeria/internal/common/util/IpAddrUtilTest.java | 8 ++++++++ 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java index 7c90a5a4adf..24e25d36934 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java @@ -234,6 +234,7 @@ private void resolveProxyConfig(SessionProtocol protocol, Endpoint endpoint, Cli final InetSocketAddress proxyAddress = proxyConfig.proxyAddress(); final boolean needsDnsResolution = proxyAddress != null && !isCreatedWithIpAddressOnly(proxyAddress); if (needsDnsResolution) { + assert proxyAddress != null; final Future resolveFuture = addressResolverGroup .getResolver(ctx.eventLoop().withoutContext()) .resolve(createUnresolvedAddressForRefreshing(proxyAddress)); diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java index f31ad6449aa..b62f9e11323 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java @@ -17,7 +17,6 @@ package com.linecorp.armeria.client.proxy; import static com.google.common.base.Preconditions.checkArgument; -import static java.util.Objects.hash; import static java.util.Objects.requireNonNull; import java.net.InetSocketAddress; @@ -87,8 +86,8 @@ public InetSocketAddress sourceAddress() { @Override public ProxyConfig withProxyAddress(InetSocketAddress newProxyAddress) { requireNonNull(newProxyAddress, "newProxyAddress"); - return this.sourceAddress == null ? new HAProxyConfig(proxyAddress) - : new HAProxyConfig(proxyAddress, this.sourceAddress); + return this.sourceAddress == null ? new HAProxyConfig(newProxyAddress) + : new HAProxyConfig(newProxyAddress, this.sourceAddress); } @Override @@ -106,7 +105,7 @@ public boolean equals(@Nullable Object o) { @Override public int hashCode() { - return hash(proxyAddress, sourceAddress); + return Objects.hash(proxyAddress, sourceAddress); } @Override diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/util/IpAddrUtil.java b/core/src/main/java/com/linecorp/armeria/internal/common/util/IpAddrUtil.java index ea445a85938..b9b5a063d4b 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/util/IpAddrUtil.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/util/IpAddrUtil.java @@ -45,10 +45,6 @@ public static boolean isCreatedWithIpAddressOnly(InetSocketAddress socketAddress } final InetAddress inetAddress = socketAddress.getAddress(); - if (inetAddress.isAnyLocalAddress()) { - return false; // Wildcard address - } - // If hostname and host address are the same, it was created with an IP address return socketAddress.getHostString().equals(inetAddress.getHostAddress()); } diff --git a/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyClientIntegrationTest.java b/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyClientIntegrationTest.java index 3378dc9e440..097d5746900 100644 --- a/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyClientIntegrationTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyClientIntegrationTest.java @@ -405,7 +405,7 @@ public void connectFailed(SessionProtocol protocol, Endpoint endpoint, assertThatThrownBy(responseFuture::join).isInstanceOf(CompletionException.class) .hasCauseInstanceOf(UnprocessedRequestException.class) .hasRootCauseInstanceOf(NullPointerException.class) - .hasRootCauseMessage("proxyConfig"); + .hasRootCauseMessage("unresolvedProxyConfig"); } } diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java index 1c4b01b1266..8f670ab67e1 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java @@ -41,6 +41,14 @@ void testIsCreatedWithIpAddressOnly() throws UnknownHostException { inetSocketAddress = new InetSocketAddress(inetAddress, 8080); assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isTrue(); + inetAddress = InetAddress.getByName("0.0.0.0"); + inetSocketAddress = new InetSocketAddress(inetAddress, 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isTrue(); + + inetAddress = InetAddress.getByName("::"); + inetSocketAddress = new InetSocketAddress(inetAddress, 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isTrue(); + inetAddress = InetAddress.getByAddress("foo.com", new byte[] { 1, 2, 3, 4 }); inetSocketAddress = new InetSocketAddress(inetAddress, 8080); assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isFalse(); From 43212b624516f6873e52f3f1e7d14466ac799460 Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Fri, 22 Nov 2024 10:02:53 +0900 Subject: [PATCH 22/23] Remove useless logging. --- .../armeria/client/proxy/HAProxyConfig.java | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java index b62f9e11323..cc6a4db7400 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java @@ -48,18 +48,8 @@ public final class HAProxyConfig extends ProxyConfig { } HAProxyConfig(InetSocketAddress proxyAddress, InetSocketAddress sourceAddress) { - if (sourceAddress.getAddress() != null && - proxyAddress.getAddress() != null) { - checkArgument(sourceAddress.getAddress().getClass() == proxyAddress.getAddress().getClass(), - "sourceAddress and proxyAddress should be the same type"); - } else { - logger.warn("Either the source or proxy address could not be resolved. " + - "The proxy address may be resolved later. " + - "proxyAddress: {}, sourceAddress: {}", - proxyAddress.getAddress(), - sourceAddress.getAddress()); - } - + checkArgument(sourceAddress.getAddress().getClass() == proxyAddress.getAddress().getClass(), + "sourceAddress and proxyAddress should be the same type"); this.proxyAddress = proxyAddress; this.sourceAddress = sourceAddress; } From 195033fff2e6babc77a863262d0104e6ff28cc3b Mon Sep 17 00:00:00 2001 From: chickenchickenlove Date: Fri, 22 Nov 2024 19:26:03 +0900 Subject: [PATCH 23/23] Add null condition. --- .../com/linecorp/armeria/client/proxy/HAProxyConfig.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java index cc6a4db7400..79b7bccbab3 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java @@ -48,8 +48,11 @@ public final class HAProxyConfig extends ProxyConfig { } HAProxyConfig(InetSocketAddress proxyAddress, InetSocketAddress sourceAddress) { - checkArgument(sourceAddress.getAddress().getClass() == proxyAddress.getAddress().getClass(), - "sourceAddress and proxyAddress should be the same type"); + // If proxyAddress is unresolved, getAddress() will return null. + if (proxyAddress.getAddress() != null) { + checkArgument(sourceAddress.getAddress().getClass() == proxyAddress.getAddress().getClass(), + "sourceAddress and proxyAddress should be the same type"); + } this.proxyAddress = proxyAddress; this.sourceAddress = sourceAddress; }