diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt index e67d4c638..fa85d0c37 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt @@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.service.okhttp import okhttp3.Dns import java.net.InetAddress import java.net.UnknownHostException +import java.util.Locale import java.util.concurrent.CompletableFuture import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.ExecutionException @@ -85,25 +86,29 @@ class AmethystDns( private val dirty = AtomicBoolean(false) override fun lookup(hostname: String): List { - cache[hostname]?.let { entry -> + // DNS hostnames are case-insensitive; canonicalize so "Example.com" and "example.com" + // share a cache entry. OkHttp normally hands us lowercase, but custom callers may not. + val key = hostname.lowercase(Locale.ROOT) + + cache[key]?.let { entry -> val now = System.currentTimeMillis() if (entry.expiresAtMillis > now) { - return entry.unwrap(hostname) + return entry.unwrap(key) } // Soft-expired positive entry: serve stale, refresh in background. Negative // entries fall through to the sync path so transient failures recover quickly. if (entry.addresses.isNotEmpty()) { - triggerBackgroundRefresh(hostname) + triggerBackgroundRefresh(key) return entry.addresses } } val newFuture = CompletableFuture>() - val existing = inflight.putIfAbsent(hostname, newFuture) + val existing = inflight.putIfAbsent(key, newFuture) return if (existing == null) { - resolveAsLeader(hostname, newFuture) + resolveAsLeader(key, newFuture) } else { - awaitFollower(hostname, existing) + awaitFollower(key, existing) } } @@ -212,7 +217,8 @@ class AmethystDns( /** Drop a single host's cached entry. Call when a connection to it fails. */ fun invalidate(hostname: String) { - if (cache.remove(hostname) != null) dirty.set(true) + val key = hostname.lowercase(Locale.ROOT) + if (cache.remove(key) != null) dirty.set(true) } /** @@ -249,7 +255,8 @@ class AmethystDns( runCatching { InetAddress.getByName(literal) }.getOrNull() } if (addresses.isNotEmpty()) { - cache.putIfAbsent(record.hostname, Entry(addresses, record.expiresAtMillis)) + val key = record.hostname.lowercase(Locale.ROOT) + cache.putIfAbsent(key, Entry(addresses, record.expiresAtMillis)) } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt new file mode 100644 index 000000000..9feb9884e --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.okhttp + +import okhttp3.Call +import okhttp3.EventListener +import java.io.IOException + +/** + * Drops a host's [AmethystDns] entry whenever an OkHttp call to it fails outright. We hook + * `callFailed` (final-stage signal after OkHttp has tried every address from the DNS lookup) + * rather than per-attempt `connectFailed`: a multi-A-record host with one bad IP fires the + * latter while OkHttp recovers via the next address, and we don't want to invalidate the cache + * just because the first IP was unreachable. + * + * Used by the relay client. The media path uses [MediaCallEventListener], which folds the same + * invalidation into its `finish` method alongside its existing timing logging. + */ +class DnsInvalidatingEventListener private constructor() : EventListener() { + override fun callFailed( + call: Call, + ioe: IOException, + ) { + AmethystDns.shared.invalidate(call.request().url.host) + } + + object Factory : EventListener.Factory { + private val INSTANCE = DnsInvalidatingEventListener() + + override fun create(call: Call): EventListener = INSTANCE + } +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt index 18875d661..4397112a8 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt @@ -122,6 +122,16 @@ class MediaCallEventListener( call: Call, error: IOException?, ) { + val host = call.request().url.host + + // The whole call failed (after OkHttp exhausted every address from the DNS lookup). + // Drop the cached entry so the next attempt re-resolves instead of trying the same + // dead IPs for up to 24h. Per-attempt connectFailed isn't enough — a multi-A-record + // host can have one bad IP and OkHttp will recover by trying the next one. + if (error != null) { + AmethystDns.shared.invalidate(host) + } + val totalMs = (System.nanoTime() - callStartNanos) / 1_000_000 val isSlow = totalMs >= SLOW_CALL_THRESHOLD_MS val wasQueued = queuedAtStart > 0 @@ -129,7 +139,6 @@ class MediaCallEventListener( if (error == null && !isSlow && !wasQueued && !isDebug) return val ttfbMs = if (responseHeadersNanos > 0) (responseHeadersNanos - callStartNanos) / 1_000_000 else -1L - val host = call.request().url.host val reuseTag = if (connectionReused) "reused" else "new" val msg = diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt index fb63b3416..b5dc62b03 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt @@ -56,6 +56,7 @@ class OkHttpClientFactoryForRelays( .Builder() .dispatcher(myDispatcher) .dns(AmethystDns.shared) + .eventListenerFactory(DnsInvalidatingEventListener.Factory) .followRedirects(true) .followSslRedirects(true) .addInterceptor(DefaultContentTypeInterceptor(userAgent)) diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt index fcdcbfb73..45a86b6a6 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsTest.kt @@ -392,6 +392,45 @@ class AmethystDnsTest { assertEquals("In-memory entry wins", listOf(ip("1.1.1.1")), dns.lookup("relay.example")) } + @Test + fun `lookup is case-insensitive`() { + val upstream = CountingDns(mapOf("example.com" to listOf(ip("1.2.3.4")))) + val dns = AmethystDns(delegate = upstream) + + dns.lookup("Example.COM") + dns.lookup("example.com") + dns.lookup("ExAmPlE.cOm") + + assertEquals("Mixed-case lookups should share one cache entry", 1, upstream.calls("example.com")) + } + + @Test + fun `invalidate is case-insensitive`() { + val upstream = CountingDns(mapOf("example.com" to listOf(ip("1.2.3.4")))) + val dns = AmethystDns(delegate = upstream) + + dns.lookup("example.com") + dns.invalidate("EXAMPLE.com") + dns.lookup("example.com") + + assertEquals(2, upstream.calls("example.com")) + } + + @Test + fun `restore lowercases hostnames so subsequent lookups hit`() { + val upstream = CountingDns(mapOf("example.com" to listOf(ip("9.9.9.9")))) + val dns = AmethystDns(delegate = upstream) + + dns.restore( + listOf( + DnsCacheRecord("Example.COM", listOf("9.9.9.9"), System.currentTimeMillis() + 60_000), + ), + ) + + assertEquals(listOf(ip("9.9.9.9")), dns.lookup("example.com")) + assertEquals("Lookup should hit restored entry without upstream", 0, upstream.calls("example.com")) + } + @Test fun `dirty flag tracks positive writes`() { val upstream = CountingDns(mapOf("a.example" to listOf(ip("1.2.3.4"))))