From 94382b32d635ff38e2c4f82eaaf1ec9af3d81e15 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Thu, 16 Jul 2026 16:28:31 -0400 Subject: [PATCH 1/6] New tests for HTTPS records --- .../api/okhttp-dnsoverhttps.api | 1 + .../okhttp3/dnsoverhttps/-DnsMessage.kt | 42 +- .../okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt | 67 ++- .../okhttp3/dnsoverhttps/DnsOverHttps.kt | 56 +-- .../DnsMessageReaderRecordedValuesTest.kt | 11 - .../dnsoverhttps/DnsOverHttpsServer.kt | 7 +- .../okhttp3/dnsoverhttps/DnsOverHttpsTest.kt | 406 ++++++++++++++---- .../java/okhttp3/dnsoverhttps/DnsTesting.kt | 104 +++++ 8 files changed, 511 insertions(+), 183 deletions(-) create mode 100644 okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsTesting.kt diff --git a/okhttp-dnsoverhttps/api/okhttp-dnsoverhttps.api b/okhttp-dnsoverhttps/api/okhttp-dnsoverhttps.api index 6b827e94363c..534cebe09d44 100644 --- a/okhttp-dnsoverhttps/api/okhttp-dnsoverhttps.api +++ b/okhttp-dnsoverhttps/api/okhttp-dnsoverhttps.api @@ -18,6 +18,7 @@ public final class okhttp3/dnsoverhttps/DnsOverHttps$Builder { public final fun bootstrapDnsHosts ([Ljava/net/InetAddress;)Lokhttp3/dnsoverhttps/DnsOverHttps$Builder; public final fun build ()Lokhttp3/dnsoverhttps/DnsOverHttps; public final fun client (Lokhttp3/OkHttpClient;)Lokhttp3/dnsoverhttps/DnsOverHttps$Builder; + public final fun includeHttps (Z)Lokhttp3/dnsoverhttps/DnsOverHttps$Builder; public final fun includeIPv6 (Z)Lokhttp3/dnsoverhttps/DnsOverHttps$Builder; public final fun post (Z)Lokhttp3/dnsoverhttps/DnsOverHttps$Builder; public final fun resolvePrivateAddresses (Z)Lokhttp3/dnsoverhttps/DnsOverHttps$Builder; diff --git a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsMessage.kt b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsMessage.kt index 11a36272efa9..b318d450afaa 100644 --- a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsMessage.kt +++ b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsMessage.kt @@ -17,9 +17,16 @@ package okhttp3.dnsoverhttps +import java.io.IOException import java.net.InetAddress +import java.net.ProtocolException +import java.net.UnknownHostException +import okhttp3.Protocol import okhttp3.RequestBody +import okhttp3.Response import okhttp3.dnsoverhttps.DnsOverHttps.Companion.DNS_MESSAGE +import okhttp3.dnsoverhttps.DnsOverHttps.Companion.MAX_RESPONSE_SIZE +import okhttp3.internal.platform.Platform import okio.Buffer import okio.BufferedSink import okio.ByteString @@ -109,8 +116,8 @@ internal sealed interface ResourceRecord { data class Https( override val name: String, override val timeToLive: Int, - val priority: Int, - val targetName: String, + val priority: Int = 1, + val targetName: String = "", val alpnIds: List? = null, var port: Int = 443, val ipAddressHints: List = listOf(), @@ -169,3 +176,34 @@ internal class QueryRequestBody( sink.emitCompleteSegments() } } + +@Throws(IOException::class) +internal fun decodeResponse(response: Response): List { + if ( + response.cacheResponse == null && + response.protocol !== Protocol.HTTP_2 && + response.protocol !== Protocol.QUIC + ) { + Platform.get().log("Unexpected protocol: ${response.protocol}", Platform.WARN) + } + + response.use { + if (!response.isSuccessful) { + throw IOException("response: ${response.code} ${response.message}") + } + + val body = response.body + if (body.contentLength() > MAX_RESPONSE_SIZE) { + throw ProtocolException( + "response size exceeds limit ($MAX_RESPONSE_SIZE bytes): ${body.contentLength()} bytes", + ) + } + + val dnsResponse = DnsMessageReader(body.source()).read() + when (dnsResponse.responseCode) { + RESPONSE_CODE_SUCCESS -> return dnsResponse.answers + RESPONSE_CODE_SERVER_FAILURE -> throw UnknownHostException("DNS server failure") + else -> throw UnknownHostException() + } + } +} diff --git a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt index e10a384a295e..f78da861c6c2 100644 --- a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt +++ b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt @@ -36,43 +36,22 @@ import okhttp3.internal.testAndSet * Implements [Dns2.Call] by making multiple HTTPS calls. */ internal class DnsOverHttpsCall( - private val dnsOverHttps: DnsOverHttps, override val request: Dns2.Request, + private val calls: List, ) : Dns2.Call, Callback { private val state = AtomicReference(State.Idle) override fun enqueue(callback: Dns2.Callback) { - val calls = - buildList { - if (dnsOverHttps.includeHttps) { - add(dnsOverHttps.createCall(request.hostname, TYPE_HTTPS)) - } - - add(dnsOverHttps.createCall(request.hostname, TYPE_A)) - - if (dnsOverHttps.includeIPv6) { - add(dnsOverHttps.createCall(request.hostname, TYPE_AAAA)) - } - } - val running = State.Running(callback, calls) val previous = state.testAndSet(running) { it is State.Idle } - when (previous) { - is State.Idle -> { - for (call in calls) { - call.enqueue(this) - } - } - - is State.Running, State.Complete -> { - throw IllegalStateException("already enqueued") - } + check(previous is State.Idle) { + "already enqueued" + } - State.Canceled -> { - callback.onFailure(this, IOException("canceled")) - } + for (call in calls) { + call.enqueue(this) } } @@ -125,7 +104,7 @@ internal class DnsOverHttpsCall( ) { val resourceRecords = try { - dnsOverHttps.decodeResponse(response) + decodeResponse(response) } catch (e: IOException) { return onFailure(call, e) } @@ -165,9 +144,10 @@ internal class DnsOverHttpsCall( ?: return // Already complete or canceled; nothing to do. val newRunningCalls = previous.runningCalls - call + val lastRunningCall = newRunningCalls.isEmpty() val next = when { - newRunningCalls.isEmpty() -> { + lastRunningCall -> { State.Complete } @@ -182,22 +162,33 @@ internal class DnsOverHttpsCall( if (!state.compareAndSet(previous, next)) continue // Lost a race; retry. - previous.callback.onRecords( - call = this, - last = newRunningCalls.isEmpty(), - records = dns2Records, - ) + val emitDelayedFailures = lastRunningCall && previous.delayedFailures.isNotEmpty() + val lastEvent = lastRunningCall && !emitDelayedFailures + if (dns2Records.isNotEmpty() || lastEvent) { + previous.callback.onRecords( + call = this, + last = lastEvent, + records = dns2Records, + ) + } + if (emitDelayedFailures) { + previous.callback.onFailure( + call = this, + exceptions = previous.delayedFailures, + ) + } return } } override fun cancel() { - val previous = state.testAndSet(State.Canceled) { it is State.Running || it == State.Idle } - (previous as? State.Running)?.callback?.onFailure(this, IOException("canceled")) + for (call in calls) { + call.cancel() + } } - override fun isCanceled() = state.get() is State.Canceled + override fun isCanceled() = calls.all { it.isCanceled() } private sealed interface State { object Idle : State @@ -209,8 +200,6 @@ internal class DnsOverHttpsCall( ) : State object Complete : State - - object Canceled : State } } diff --git a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt index 22222a30f4d2..c7e2156c8e05 100644 --- a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt +++ b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt @@ -17,7 +17,6 @@ package okhttp3.dnsoverhttps import java.io.IOException import java.net.InetAddress -import java.net.ProtocolException import java.net.UnknownHostException import java.util.concurrent.CountDownLatch import okhttp3.Call @@ -28,10 +27,8 @@ import okhttp3.HttpUrl import okhttp3.MediaType import okhttp3.MediaType.Companion.toMediaType import okhttp3.OkHttpClient -import okhttp3.Protocol import okhttp3.Request import okhttp3.Response -import okhttp3.internal.platform.Platform import okhttp3.internal.publicsuffix.PublicSuffixDatabase /** @@ -54,7 +51,22 @@ class DnsOverHttps internal constructor( @get:JvmName("resolvePublicAddresses") val resolvePublicAddresses: Boolean, ) : Dns, Dns2 { - override fun newCall(request: Dns2.Request): Dns2.Call = DnsOverHttpsCall(this, request) + override fun newCall(request: Dns2.Request): Dns2.Call = + DnsOverHttpsCall( + request = request, + calls = + buildList { + if (includeHttps) { + add(createCall(request.hostname, TYPE_HTTPS)) + } + + if (includeIPv6) { + add(createCall(request.hostname, TYPE_AAAA)) + } + + add(createCall(request.hostname, TYPE_A)) + }, + ) @Throws(UnknownHostException::class) override fun lookup(hostname: String): List { @@ -175,37 +187,6 @@ class DnsOverHttps internal constructor( throw unknownHostException } - @Throws(IOException::class) - internal fun decodeResponse(response: Response): List { - if ( - response.cacheResponse == null && - response.protocol !== Protocol.HTTP_2 && - response.protocol !== Protocol.QUIC - ) { - Platform.get().log("Incorrect protocol: ${response.protocol}", Platform.WARN) - } - - response.use { - if (!response.isSuccessful) { - throw IOException("response: ${response.code} ${response.message}") - } - - val body = response.body - if (body.contentLength() > MAX_RESPONSE_SIZE) { - throw ProtocolException( - "response size exceeds limit ($MAX_RESPONSE_SIZE bytes): ${body.contentLength()} bytes", - ) - } - - val dnsResponse = DnsMessageReader(body.source()).read() - when (dnsResponse.responseCode) { - RESPONSE_CODE_SUCCESS -> return dnsResponse.answers - RESPONSE_CODE_SERVER_FAILURE -> throw UnknownHostException("DNS server failure") - else -> throw UnknownHostException() - } - } - } - internal fun createCall( hostname: String, type: Int, @@ -278,6 +259,11 @@ class DnsOverHttps internal constructor( * * This is false by default, but that default is subject to change in 2026. */ + fun includeHttps(includeHttps: Boolean) = + apply { + this.includeHttps = includeHttps + } + fun includeIPv6(includeIPv6: Boolean) = apply { this.includeIPv6 = includeIPv6 diff --git a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsMessageReaderRecordedValuesTest.kt b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsMessageReaderRecordedValuesTest.kt index f057584370e3..6e5687f220ef 100644 --- a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsMessageReaderRecordedValuesTest.kt +++ b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsMessageReaderRecordedValuesTest.kt @@ -207,8 +207,6 @@ class DnsMessageReaderRecordedValuesTest { ResourceRecord.Https( name = "ylilauta.org", timeToLive = 300, - priority = 1, - targetName = "", alpnIds = listOf("h2"), ), ) @@ -227,8 +225,6 @@ class DnsMessageReaderRecordedValuesTest { ResourceRecord.Https( name = "airnewzealand.co.nz", timeToLive = 600, - priority = 1, - targetName = "", alpnIds = listOf("h2", "http/1.1"), ), ) @@ -247,8 +243,6 @@ class DnsMessageReaderRecordedValuesTest { ResourceRecord.Https( name = "fbcdn.net", timeToLive = 924, - priority = 1, - targetName = "", alpnIds = listOf("h2", "h3", "http/1.1"), ), ResourceRecord.Https( @@ -349,7 +343,6 @@ class DnsMessageReaderRecordedValuesTest { name = "prosody.im", timeToLive = 300, priority = 1, - targetName = "", alpnIds = listOf("h2", "http/1.1"), ipAddressHints = listOf(InetAddress.getByName("2a00:1098:3a0:0:0:0:0:1")), ), @@ -357,7 +350,6 @@ class DnsMessageReaderRecordedValuesTest { name = "prosody.im", timeToLive = 300, priority = 2, - targetName = "", alpnIds = listOf("http/1.1"), ipAddressHints = listOf(InetAddress.getByName("176.126.242.74")), ), @@ -378,8 +370,6 @@ class DnsMessageReaderRecordedValuesTest { ResourceRecord.Https( name = "mtgal.com", timeToLive = 604800, - priority = 1, - targetName = "", alpnIds = listOf("h3", "http/1.1"), echConfigList = ( @@ -405,7 +395,6 @@ class DnsMessageReaderRecordedValuesTest { ResourceRecord.Https( name = "planaltonet.net.br", timeToLive = 300, - priority = 1, targetName = "dnsr1.planaltonet.net.br", alpnIds = listOf("h2", "h3", "http/1.1"), ipAddressHints = diff --git a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsServer.kt b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsServer.kt index de2aa53e02df..cedb7a6d695f 100644 --- a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsServer.kt +++ b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsServer.kt @@ -20,6 +20,7 @@ import java.net.Inet6Address import java.net.InetAddress import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.LinkedBlockingDeque +import java.util.concurrent.atomic.AtomicInteger import mockwebserver3.Dispatcher import mockwebserver3.MockResponse import mockwebserver3.RecordedRequest @@ -35,7 +36,8 @@ internal class DnsOverHttpsServer : Dispatcher() { var extraHeaders: Headers = Headers.headersOf() val requests = LinkedBlockingDeque>() - var override: MockResponse? = null + val nextSequenceIndex = AtomicInteger(0) + val sequenceIndexToOverride = ConcurrentHashMap() operator fun set( hostname: String, @@ -66,7 +68,8 @@ internal class DnsOverHttpsServer : Dispatcher() { fun pollRequest(): Pair? = requests.poll() override fun dispatch(request: RecordedRequest): MockResponse { - val override = this.override + val sequenceIndex = nextSequenceIndex.getAndIncrement() + val override = sequenceIndexToOverride.remove(sequenceIndex) if (override != null) return override val requestBody = request.body diff --git a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt index 52b28f0c187c..c557adcc3078 100644 --- a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt +++ b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt @@ -17,19 +17,17 @@ package okhttp3.dnsoverhttps import app.cash.burst.Burst import assertk.assertThat -import assertk.assertions.contains import assertk.assertions.containsExactly import assertk.assertions.containsExactlyInAnyOrder import assertk.assertions.hasMessage import assertk.assertions.isEqualTo import assertk.assertions.isInstanceOf import assertk.assertions.isNull +import assertk.assertions.isTrue import java.io.EOFException import java.io.File import java.net.InetAddress import java.net.UnknownHostException -import java.util.concurrent.CompletableFuture -import java.util.concurrent.ExecutionException import kotlin.reflect.KClass import kotlin.test.assertFailsWith import mockwebserver3.MockResponse @@ -39,23 +37,33 @@ import okhttp3.Cache import okhttp3.CallEvent import okhttp3.CallEvent.CacheHit import okhttp3.CallEvent.CacheMiss +import okhttp3.Dispatcher import okhttp3.Dns2 import okhttp3.EventRecorder import okhttp3.Headers.Companion.headersOf +import okhttp3.Interceptor import okhttp3.OkHttpClient import okhttp3.Protocol import okhttp3.testing.PlatformRule import okio.Buffer import okio.ByteString.Companion.decodeHex +import okio.ByteString.Companion.encodeUtf8 import okio.IOException import okio.Path.Companion.toPath import okio.fakefilesystem.FakeFileSystem -import org.junit.jupiter.api.Assertions.fail +import org.junit.jupiter.api.Assumptions.assumeTrue import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Tag import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension +/** + * This test takes advantage of the fact that the new DNS API always requests records in this order: + * + * * [TYPE_HTTPS] + * * [TYPE_AAAA] + * * [TYPE_A] + */ @Tag("Slowish") @Burst class DnsOverHttpsTest( @@ -74,25 +82,38 @@ class DnsOverHttpsTest( } private lateinit var dns: DnsOverHttps + private val dispatcher = + Dispatcher() + .apply { + // Force calls to run sequentially for determinism. + this.maxRequestsPerHost = 1 + } private val cacheFs = FakeFileSystem() private val eventRecorder = EventRecorder() private val bootstrapClient = OkHttpClient .Builder() .protocols(listOf(Protocol.HTTP_2, Protocol.HTTP_1_1)) + .dispatcher(dispatcher) .eventListener(eventRecorder.eventListener) + .addInterceptor(Interceptor { chain -> interceptor.intercept(chain) }) .build() + private var interceptor = + Interceptor { chain -> + chain.proceed(chain.request()) + } + @BeforeEach fun setUp() { server.protocols = bootstrapClient.protocols - dns = buildLocalhost(bootstrapClient, false) + dns = buildLocalhost(bootstrapClient) } @Test fun getOne() { dnsOverHttpsServer["lysine.dev"] = listOf(InetAddress.getByName("10.20.30.40")) - val result = dns.invoke("lysine.dev") + val result = dns.invoke(entryPoint, "lysine.dev") assertThat(result).isEqualTo(listOf(address("10.20.30.40"))) val (httpsRequest, dnsRequest) = dnsOverHttpsServer.takeRequest() assertThat(httpsRequest.method).isEqualTo("GET") @@ -107,15 +128,19 @@ class DnsOverHttpsTest( InetAddress.getByName("10.20.30.40"), InetAddress.getByName("1:2::3:4"), ) - dns = buildLocalhost(bootstrapClient, true) - val result = dns("lysine.dev") - assertThat(result.size).isEqualTo(2) - assertThat(result).contains(address("10.20.30.40")) - assertThat(result).contains(address("1:2::3:4")) + dns = buildLocalhost(bootstrapClient, includeIPv6 = true) + val result = dns(entryPoint, "lysine.dev") + assertThat(result).containsExactlyInAnyOrder( + address("1:2::3:4"), + address("10.20.30.40"), + ) + val (httpsRequest1, dnsRequest1) = dnsOverHttpsServer.takeRequest() assertThat(httpsRequest1.method).isEqualTo("GET") + val (httpsRequest2, dnsRequest2) = dnsOverHttpsServer.takeRequest() assertThat(httpsRequest2.method).isEqualTo("GET") + assertThat(listOf(dnsRequest1, dnsRequest2)) .containsExactlyInAnyOrder( queryRequest("lysine.dev", TYPE_A), @@ -126,7 +151,7 @@ class DnsOverHttpsTest( @Test fun failure() { assertFailsWith { - dns("lysine.dev") + dns(entryPoint, "lysine.dev") } val (httpsRequest, dnsRequest) = dnsOverHttpsServer.takeRequest() assertThat(httpsRequest.method).isEqualTo("GET") @@ -137,22 +162,21 @@ class DnsOverHttpsTest( @Test fun failOnExcessiveResponse() { val array = CharArray(128 * 1024 + 2) { '0' } - dnsOverHttpsServer.override = overrideResponse(String(array)) - try { - dns("lysine.dev") - fail() - } catch (e: IOException) { - val rootCause = e.cause ?: e - assertThat(rootCause).hasMessage("response size exceeds limit (65536 bytes): 65537 bytes") - } + dnsOverHttpsServer.sequenceIndexToOverride[0] = overrideResponse(String(array)) + val e = + assertFailsWith { + dns(entryPoint, "lysine.dev") + } + val rootCause = e.cause ?: e + assertThat(rootCause).hasMessage("response size exceeds limit (65536 bytes): 65537 bytes") } @Test fun failOnBadResponse() { - dnsOverHttpsServer.override = overrideResponse("00") + dnsOverHttpsServer.sequenceIndexToOverride[0] = overrideResponse("00") val e = assertFailsWith { - dns("lysine.dev") + dns(entryPoint, "lysine.dev") } val rootCause = e.cause ?: e assertThat(rootCause).isInstanceOf() @@ -168,7 +192,7 @@ class DnsOverHttpsTest( fun usesCache() { val cache = Cache(cacheFs, "cache".toPath(), (100 * 1024).toLong()) val cachedClient = bootstrapClient.newBuilder().cache(cache).build() - val cachedDns = buildLocalhost(cachedClient, false) + val cachedDns = buildLocalhost(cachedClient) dnsOverHttpsServer.extraHeaders = headersOf( @@ -178,7 +202,7 @@ class DnsOverHttpsTest( dnsOverHttpsServer["lysine.dev"] = listOf(InetAddress.getByName("10.20.30.40")) dnsOverHttpsServer["alternate.lysine.dev"] = listOf(InetAddress.getByName("55.66.77.88")) - val result1 = cachedDns("lysine.dev") + val result1 = cachedDns(entryPoint, "lysine.dev") assertThat(result1).containsExactly(address("10.20.30.40")) val (httpsRequest1, dnsRequest1) = dnsOverHttpsServer.takeRequest() assertThat(httpsRequest1.method).isEqualTo("GET") @@ -187,13 +211,13 @@ class DnsOverHttpsTest( assertThat(cacheEvents()).containsExactly(CacheMiss::class) - val result2 = cachedDns("lysine.dev") + val result2 = cachedDns(entryPoint, "lysine.dev") assertThat(dnsOverHttpsServer.pollRequest()).isNull() assertThat(result2).isEqualTo(listOf(address("10.20.30.40"))) assertThat(cacheEvents()).containsExactly(CacheHit::class) - val result3 = cachedDns("alternate.lysine.dev") + val result3 = cachedDns(entryPoint, "alternate.lysine.dev") assertThat(result3).containsExactly(address("55.66.77.88")) val (httpsRequest2, dnsRequest2) = dnsOverHttpsServer.takeRequest() assertThat(httpsRequest2.method).isEqualTo("GET") @@ -207,7 +231,7 @@ class DnsOverHttpsTest( fun usesCacheEvenForPost() { val cache = Cache(cacheFs, "cache".toPath(), (100 * 1024).toLong()) val cachedClient = bootstrapClient.newBuilder().cache(cache).build() - val cachedDns = buildLocalhost(cachedClient, false, post = true) + val cachedDns = buildLocalhost(cachedClient, post = true) dnsOverHttpsServer.extraHeaders = headersOf( "cache-control", @@ -216,7 +240,7 @@ class DnsOverHttpsTest( dnsOverHttpsServer["lysine.dev"] = listOf(InetAddress.getByName("10.20.30.40")) dnsOverHttpsServer["alternate.lysine.dev"] = listOf(InetAddress.getByName("55.66.77.88")) - val result1 = cachedDns("lysine.dev") + val result1 = cachedDns(entryPoint, "lysine.dev") assertThat(result1).containsExactly(address("10.20.30.40")) val (httpsRequest1, _) = dnsOverHttpsServer.takeRequest() assertThat(httpsRequest1.method).isEqualTo("POST") @@ -225,13 +249,13 @@ class DnsOverHttpsTest( assertThat(cacheEvents()).containsExactly(CacheMiss::class) - val result2 = cachedDns("lysine.dev") + val result2 = cachedDns(entryPoint, "lysine.dev") assertThat(dnsOverHttpsServer.pollRequest()).isNull() assertThat(result2).isEqualTo(listOf(address("10.20.30.40"))) assertThat(cacheEvents()).containsExactly(CacheHit::class) - val result3 = cachedDns("alternate.lysine.dev") + val result3 = cachedDns(entryPoint, "alternate.lysine.dev") assertThat(result3).containsExactly(address("55.66.77.88")) val (httpsRequest2, _) = dnsOverHttpsServer.takeRequest() assertThat(httpsRequest2.method).isEqualTo("POST") @@ -245,14 +269,14 @@ class DnsOverHttpsTest( fun usesCacheOnlyIfFresh() { val cache = Cache(File("./target/DnsOverHttpsTest.cache"), 100 * 1024L) val cachedClient = bootstrapClient.newBuilder().cache(cache).build() - val cachedDns = buildLocalhost(cachedClient, false) + val cachedDns = buildLocalhost(cachedClient) dnsOverHttpsServer.extraHeaders = headersOf( "cache-control", "no-store", ) dnsOverHttpsServer["lysine.dev"] = listOf(InetAddress.getByName("10.20.30.40")) - val result1 = cachedDns("lysine.dev") + val result1 = cachedDns(entryPoint, "lysine.dev") assertThat(result1).containsExactly(address("10.20.30.40")) val (httpsRequest1, dnsRequest1) = dnsOverHttpsServer.takeRequest() assertThat(httpsRequest1.method).isEqualTo("GET") @@ -261,7 +285,7 @@ class DnsOverHttpsTest( assertThat(cacheEvents()).containsExactly(CacheMiss::class) - val result2 = cachedDns("lysine.dev") + val result2 = cachedDns(entryPoint, "lysine.dev") assertThat(result2).isEqualTo(listOf(address("10.20.30.40"))) val (httpsRequest2, dnsRequest2) = dnsOverHttpsServer.takeRequest() assertThat(httpsRequest2.method).isEqualTo("GET") @@ -271,6 +295,255 @@ class DnsOverHttpsTest( assertThat(cacheEvents()).containsExactly(CacheMiss::class) } + @Test + fun completeHttpsRecordsReturned() { + assumeTrue(entryPoint == EntryPoint.NewCall) + + dns = buildLocalhost(bootstrapClient, includeIPv6 = true, includeHttps = true) + dnsOverHttpsServer["lysine.dev"] = + listOf( + ResourceRecord.IpAddress( + name = "lysine.dev", + timeToLive = 5, + address = InetAddress.getByName("10.20.30.40"), + ), + ResourceRecord.IpAddress( + name = "lysine.dev", + timeToLive = 5, + address = InetAddress.getByName("1:2::3:4"), + ), + ResourceRecord.Https( + name = "lysine.dev", + timeToLive = 5, + alpnIds = listOf(Protocol.HTTP_2.toString()), + port = 8843, + ipAddressHints = + listOf( + InetAddress.getByName("55.66.77.88"), + InetAddress.getByName("aa:bb::cc:dd"), + ), + echConfigList = "this is an encrypted client hello".encodeUtf8(), + ), + ) + + val call = dns.newCall(Dns2.Request("lysine.dev")) + val dnsEvents = call.execute() + + assertThat(dnsEvents.take()).isEqualTo( + DnsEvent.Records( + last = false, + records = + listOf( + Dns2.Record.ServiceMetadata( + hostname = "lysine.dev", + alpnIds = listOf(Protocol.HTTP_2), + port = 8843, + ipAddressHints = + listOf( + InetAddress.getByName("55.66.77.88"), + InetAddress.getByName("aa:bb::cc:dd"), + ), + echConfigList = "this is an encrypted client hello".encodeUtf8(), + ), + ), + ), + ) + assertThat(dnsEvents.take()).isEqualTo( + DnsEvent.Records( + last = false, + records = + listOf( + Dns2.Record.IpAddress( + hostname = "lysine.dev", + address = InetAddress.getByName("1:2::3:4"), + ), + ), + ), + ) + assertThat(dnsEvents.take()).isEqualTo( + DnsEvent.Records( + last = true, + records = + listOf( + Dns2.Record.IpAddress( + hostname = "lysine.dev", + address = InetAddress.getByName("10.20.30.40"), + ), + ), + ), + ) + } + + /** An HTTPS error is received before the IPv4 results, but the error is delivered last. */ + @Test + fun httpsFailureIsDeliveredAfterIpv6AndIpv4Records() { + assumeTrue(entryPoint == EntryPoint.NewCall) + + dns = buildLocalhost(bootstrapClient, includeIPv6 = true, includeHttps = true) + + // Fail the HTTPS call, which should have index 0. + dnsOverHttpsServer.sequenceIndexToOverride[0] = overrideResponse("") + dnsOverHttpsServer["lysine.dev"] = + listOf( + ResourceRecord.IpAddress( + name = "lysine.dev", + timeToLive = 5, + address = InetAddress.getByName("11:22::33:44"), + ), + ResourceRecord.IpAddress( + name = "lysine.dev", + timeToLive = 5, + address = InetAddress.getByName("10.20.30.40"), + ), + ) + + val call = dns.newCall(Dns2.Request("lysine.dev")) + val dnsEvents = call.execute() + + assertThat(dnsEvents.take()).isEqualTo( + DnsEvent.Records( + last = false, + records = + listOf( + Dns2.Record.IpAddress( + hostname = "lysine.dev", + address = InetAddress.getByName("11:22::33:44"), + ), + ), + ), + ) + assertThat(dnsEvents.take()).isEqualTo( + DnsEvent.Records( + last = false, + records = + listOf( + Dns2.Record.IpAddress( + hostname = "lysine.dev", + address = InetAddress.getByName("10.20.30.40"), + ), + ), + ), + ) + assertThat(dnsEvents.take()).isInstanceOf() + } + + /** An IPv6 error is received before the IPv4 results, but the error is delivered last. */ + @Test + fun ipv6FailureIsDeliveredAfterIpv4Records() { + assumeTrue(entryPoint == EntryPoint.NewCall) + + dns = buildLocalhost(bootstrapClient, includeIPv6 = true, includeHttps = true) + + // Fail the IPv6 call, which should have index 1. + dnsOverHttpsServer.sequenceIndexToOverride[1] = overrideResponse("") + dnsOverHttpsServer["lysine.dev"] = + listOf( + ResourceRecord.IpAddress( + name = "lysine.dev", + timeToLive = 5, + address = InetAddress.getByName("10.20.30.40"), + ), + ) + + val call = dns.newCall(Dns2.Request("lysine.dev")) + val dnsEvents = call.execute() + + assertThat(dnsEvents.take()).isEqualTo( + DnsEvent.Records( + last = false, + records = + listOf( + Dns2.Record.IpAddress( + hostname = "lysine.dev", + address = InetAddress.getByName("10.20.30.40"), + ), + ), + ), + ) + assertThat(dnsEvents.take()).isInstanceOf() + } + + /** There's no IPv6 event because there's no IPv6 results. */ + @Test + fun emptyResultsAreSkipped() { + assumeTrue(entryPoint == EntryPoint.NewCall) + + dns = buildLocalhost(bootstrapClient, includeIPv6 = true, includeHttps = true) + dnsOverHttpsServer["lysine.dev"] = + listOf( + ResourceRecord.IpAddress( + name = "lysine.dev", + timeToLive = 5, + address = InetAddress.getByName("10.20.30.40"), + ), + ) + + val call = dns.newCall(Dns2.Request("lysine.dev")) + val dnsEvents = call.execute() + + assertThat(dnsEvents.take()).isEqualTo( + DnsEvent.Records( + last = true, + records = + listOf( + Dns2.Record.IpAddress( + hostname = "lysine.dev", + address = InetAddress.getByName("10.20.30.40"), + ), + ), + ), + ) + } + + @Test + fun lastEventIsDeliveredEventIfItIsEmpty() { + assumeTrue(entryPoint == EntryPoint.NewCall) + + dns = buildLocalhost(bootstrapClient, includeIPv6 = true, includeHttps = true) + + val call = dns.newCall(Dns2.Request("lysine.dev")) + val dnsEvents = call.execute() + + assertThat(dnsEvents.take()).isEqualTo( + DnsEvent.Records( + last = true, + records = listOf(), + ), + ) + } + + @Test + fun callIsCanceledBeforeItIsStarted() { + assumeTrue(entryPoint == EntryPoint.NewCall) + + dns = buildLocalhost(bootstrapClient, includeIPv6 = true, includeHttps = true) + + val call = dns.newCall(Dns2.Request("lysine.dev")) + call.cancel() + assertThat(call.isCanceled()).isTrue() + val dnsEvents = call.execute() + + assertThat(dnsEvents.take()).isInstanceOf() + } + + @Test + fun callIsCanceledBeforeItReachesTheNetwork() { + assumeTrue(entryPoint == EntryPoint.NewCall) + + dns = buildLocalhost(bootstrapClient, includeIPv6 = true, includeHttps = true) + val call = dns.newCall(Dns2.Request("lysine.dev")) + + interceptor = + Interceptor { chain -> + call.cancel() + assertThat(call.isCanceled()).isTrue() + chain.proceed(chain.request()) + } + + val dnsEvents = call.execute() + assertThat(dnsEvents.take()).isInstanceOf() + } + private fun cacheEvents(): List> = eventRecorder .recordedEventTypes() @@ -279,7 +552,8 @@ class DnsOverHttpsTest( private fun buildLocalhost( bootstrapClient: OkHttpClient, - includeIPv6: Boolean, + includeIPv6: Boolean = false, + includeHttps: Boolean = false, post: Boolean = false, ): DnsOverHttps { val url = server.url("/lookup?ct") @@ -287,18 +561,19 @@ class DnsOverHttpsTest( .Builder() .client(bootstrapClient) .includeIPv6(includeIPv6) + .includeHttps(includeHttps) .resolvePrivateAddresses(true) .url(url) .post(post) .build() } - private fun overrideResponse(body: String): MockResponse = + private fun overrideResponse(hexBody: String = ""): MockResponse = MockResponse .Builder() - .body(Buffer().write(body.decodeHex())) + .body(Buffer().write(hexBody.decodeHex())) .addHeader("content-type", "application/dns-message") - .addHeader("content-length", body.length / 2) + .addHeader("content-length", hexBody.length / 2) .build() private fun queryRequest( @@ -318,63 +593,6 @@ class DnsOverHttpsTest( ), ) - /** - * Use Burst to call either the blocking API or the non-blocking API, both with a signature that - * resembles the [InetAddress.getAllByName] API. - */ - private operator fun DnsOverHttps.invoke(hostname: String): List = - when (entryPoint) { - EntryPoint.Lookup -> { - lookup(hostname) - } - - EntryPoint.NewCall -> { - val future = CompletableFuture>() - - val call = newCall(Dns2.Request(hostname)) - call.enqueue( - object : Dns2.Callback { - private val results = mutableListOf() - - override fun onRecords( - call: Dns2.Call, - last: Boolean, - records: List, - ) { - this.results += - records - .filterIsInstance() - .map { it.address } - if (last) { - when { - results.isNotEmpty() -> future.complete(this.results) - else -> future.completeExceptionally(UnknownHostException()) - } - } - } - - override fun onFailure( - call: Dns2.Call, - e: IOException, - ) { - future.completeExceptionally(e) - } - }, - ) - - try { - future.get() - } catch (e: ExecutionException) { - throw e.cause!! - } - } - } - - enum class EntryPoint { - Lookup, - NewCall, - } - companion object { private fun address(host: String) = InetAddress.getByName(host) } diff --git a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsTesting.kt b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsTesting.kt new file mode 100644 index 000000000000..f765be4d68ea --- /dev/null +++ b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsTesting.kt @@ -0,0 +1,104 @@ +/* + * Copyright (c) 2026 OkHttp Authors + * + * Licensed 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 + * + * http://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 okhttp3.dnsoverhttps + +import java.net.InetAddress +import java.net.UnknownHostException +import java.util.concurrent.BlockingDeque +import java.util.concurrent.LinkedBlockingDeque +import okhttp3.Dns2 +import okio.IOException + +sealed interface DnsEvent { + data class Records( + val last: Boolean, + val records: List, + ) : DnsEvent + + data class Failure( + val e: IOException, + ) : DnsEvent +} + +internal fun Dns2.Call.execute(): BlockingDeque { + val result = LinkedBlockingDeque() + + enqueue( + object : Dns2.Callback { + override fun onRecords( + call: Dns2.Call, + last: Boolean, + records: List, + ) { + result.put(DnsEvent.Records(last, records)) + } + + override fun onFailure( + call: Dns2.Call, + e: IOException, + ) { + result.put(DnsEvent.Failure(e)) + } + }, + ) + + return result +} + +/** + * Use Burst to call either the blocking API or the non-blocking API, both with a signature that + * resembles the [InetAddress.getAllByName] API. + */ +operator fun DnsOverHttps.invoke( + entryPoint: EntryPoint, + hostname: String, +): List = + when (entryPoint) { + EntryPoint.Lookup -> { + lookup(hostname) + } + + EntryPoint.NewCall -> { + buildList { + val dnsEvents = newCall(Dns2.Request(hostname)).execute() + while (true) { + when (val dnsEvent = dnsEvents.take()) { + is DnsEvent.Failure -> { + throw dnsEvent.e + } + + is DnsEvent.Records -> { + addAll( + dnsEvent.records + .filterIsInstance() + .map { it.address }, + ) + + if (dnsEvent.last) { + if (isEmpty()) throw UnknownHostException("no results") + break + } + } + } + } + } + } + } + +enum class EntryPoint { + Lookup, + NewCall, +} From f4de3f5cef99f6e1b929d7a32db1ced83eae9614 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Thu, 16 Jul 2026 17:02:48 -0400 Subject: [PATCH 2/6] Honor resolvePrivateAddresses and resolvePublicAddresses in DnsOverHttps --- .../okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt | 6 +- .../okhttp3/dnsoverhttps/DnsOverHttps.kt | 92 +++++++++++-------- .../okhttp3/dnsoverhttps/DnsOverHttpsTest.kt | 38 ++++++-- 3 files changed, 89 insertions(+), 47 deletions(-) diff --git a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt index f78da861c6c2..bd679acb40c7 100644 --- a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt +++ b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt @@ -38,6 +38,7 @@ import okhttp3.internal.testAndSet internal class DnsOverHttpsCall( override val request: Dns2.Request, private val calls: List, + private val canceledException: IOException?, ) : Dns2.Call, Callback { private val state = AtomicReference(State.Idle) @@ -69,7 +70,10 @@ internal class DnsOverHttpsCall( ?: return // Already complete or canceled; nothing to do. val newRunningCalls = previous.runningCalls - call - val allFailures = previous.delayedFailures + e + val allFailures = when { + canceledException != null -> listOf(canceledException) + else -> previous.delayedFailures + e + } val next = when { newRunningCalls.isEmpty() -> { diff --git a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt index c7e2156c8e05..09aec41482b2 100644 --- a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt +++ b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt @@ -51,49 +51,48 @@ class DnsOverHttps internal constructor( @get:JvmName("resolvePublicAddresses") val resolvePublicAddresses: Boolean, ) : Dns, Dns2 { - override fun newCall(request: Dns2.Request): Dns2.Call = - DnsOverHttpsCall( - request = request, - calls = - buildList { - if (includeHttps) { - add(createCall(request.hostname, TYPE_HTTPS)) - } + override fun newCall(request: Dns2.Request): Dns2.Call { + val calls = callsList(request.hostname) - if (includeIPv6) { - add(createCall(request.hostname, TYPE_AAAA)) - } + val canceledException = validate(request.hostname) + if (canceledException != null) { + for (call in calls) { + call.cancel() + } + } - add(createCall(request.hostname, TYPE_A)) - }, + return DnsOverHttpsCall( + request = request, + calls = calls, + canceledException = canceledException, ) + } - @Throws(UnknownHostException::class) - override fun lookup(hostname: String): List { - if (!resolvePrivateAddresses || !resolvePublicAddresses) { - val privateHost = isPrivateHost(hostname) - - if (privateHost && !resolvePrivateAddresses) { - throw UnknownHostException("private hosts not resolved") - } - - if (!privateHost && !resolvePublicAddresses) { - throw UnknownHostException("public hosts not resolved") - } + /** + * Returns an exception if [hostname] should not be resolved. + * + * We **return** this exception rather than throwing it because in the [Dns2.Callback] case we want + * `onFailure()` to be called on a dispatcher thread and not synchronously. + */ + private fun validate(hostname: String): UnknownHostException? { + // Don't load the public suffix list unless necessary. + if (resolvePrivateAddresses && resolvePublicAddresses) return null + + val privateHost = isPrivateHost(hostname) + + return when { + privateHost && !resolvePrivateAddresses -> UnknownHostException("private hosts not resolved") + !privateHost && !resolvePublicAddresses -> UnknownHostException("public hosts not resolved") + else -> null } - - return lookupHttps(hostname) } @Throws(UnknownHostException::class) - private fun lookupHttps(hostname: String): List { - val calls = - buildList { - add(createCall(hostname, TYPE_A)) - if (includeIPv6) { - add(createCall(hostname, TYPE_AAAA)) - } - } + override fun lookup(hostname: String): List { + val validationException = validate(hostname) + if (validationException != null) throw validationException + + val calls = callsList(hostname, inetAddressesOnly = true) val failures = ArrayList(3) val results = ArrayList(5) @@ -219,6 +218,23 @@ class DnsOverHttps internal constructor( }.build(), ) + private fun callsList( + hostname: String, + inetAddressesOnly: Boolean = false, + ): List { + return buildList { + if (includeHttps && !inetAddressesOnly) { + add(createCall(hostname, TYPE_HTTPS)) + } + + if (includeIPv6) { + add(createCall(hostname, TYPE_AAAA)) + } + + add(createCall(hostname, TYPE_A)) + } + } + class Builder { internal var client: OkHttpClient? = null internal var url: HttpUrl? = null @@ -289,7 +305,8 @@ class DnsOverHttps internal constructor( this.bootstrapDnsHosts = bootstrapDnsHosts } - fun bootstrapDnsHosts(vararg bootstrapDnsHosts: InetAddress): Builder = bootstrapDnsHosts(bootstrapDnsHosts.toList()) + fun bootstrapDnsHosts(vararg bootstrapDnsHosts: InetAddress): Builder = + bootstrapDnsHosts(bootstrapDnsHosts.toList()) fun systemDns(systemDns: Dns) = apply { @@ -311,6 +328,7 @@ class DnsOverHttps internal constructor( } } - internal fun isPrivateHost(host: String): Boolean = PublicSuffixDatabase.get().getEffectiveTldPlusOne(host) == null + internal fun isPrivateHost(host: String): Boolean = + PublicSuffixDatabase.get().getEffectiveTldPlusOne(host) == null } } diff --git a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt index c557adcc3078..b1ce8c74e998 100644 --- a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt +++ b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt @@ -18,7 +18,6 @@ package okhttp3.dnsoverhttps import app.cash.burst.Burst import assertk.assertThat import assertk.assertions.containsExactly -import assertk.assertions.containsExactlyInAnyOrder import assertk.assertions.hasMessage import assertk.assertions.isEqualTo import assertk.assertions.isInstanceOf @@ -130,22 +129,18 @@ class DnsOverHttpsTest( ) dns = buildLocalhost(bootstrapClient, includeIPv6 = true) val result = dns(entryPoint, "lysine.dev") - assertThat(result).containsExactlyInAnyOrder( + assertThat(result).containsExactly( address("1:2::3:4"), address("10.20.30.40"), ) val (httpsRequest1, dnsRequest1) = dnsOverHttpsServer.takeRequest() assertThat(httpsRequest1.method).isEqualTo("GET") + assertThat(dnsRequest1).isEqualTo(queryRequest("lysine.dev", TYPE_AAAA)) val (httpsRequest2, dnsRequest2) = dnsOverHttpsServer.takeRequest() assertThat(httpsRequest2.method).isEqualTo("GET") - - assertThat(listOf(dnsRequest1, dnsRequest2)) - .containsExactlyInAnyOrder( - queryRequest("lysine.dev", TYPE_A), - queryRequest("lysine.dev", TYPE_AAAA), - ) + assertThat(dnsRequest2).isEqualTo(queryRequest("lysine.dev", TYPE_A)) } @Test @@ -159,6 +154,28 @@ class DnsOverHttpsTest( .isEqualTo(queryRequest("lysine.dev", TYPE_A)) } + @Test + fun resolveForbiddenPrivateDomain() { + dns = buildLocalhost(bootstrapClient, resolvePrivateAddresses = false) + + val e = assertFailsWith { + dns(entryPoint, "dev") + } + assertThat(e).hasMessage("private hosts not resolved") + assertThat(dnsOverHttpsServer.pollRequest()).isNull() + } + + @Test + fun resolveForbiddenPublicDomain() { + dns = buildLocalhost(bootstrapClient, resolvePublicAddresses = false) + + val e = assertFailsWith { + dns(entryPoint, "lysine.dev") + } + assertThat(e).hasMessage("public hosts not resolved") + assertThat(dnsOverHttpsServer.pollRequest()).isNull() + } + @Test fun failOnExcessiveResponse() { val array = CharArray(128 * 1024 + 2) { '0' } @@ -555,6 +572,8 @@ class DnsOverHttpsTest( includeIPv6: Boolean = false, includeHttps: Boolean = false, post: Boolean = false, + resolvePrivateAddresses: Boolean = true, + resolvePublicAddresses: Boolean = true, ): DnsOverHttps { val url = server.url("/lookup?ct") return DnsOverHttps @@ -562,7 +581,8 @@ class DnsOverHttpsTest( .client(bootstrapClient) .includeIPv6(includeIPv6) .includeHttps(includeHttps) - .resolvePrivateAddresses(true) + .resolvePrivateAddresses(resolvePrivateAddresses) + .resolvePublicAddresses(resolvePublicAddresses) .url(url) .post(post) .build() From 84b308bfd0d02413ac0589f16993f3876a403da4 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Thu, 16 Jul 2026 17:16:51 -0400 Subject: [PATCH 3/6] Spotless --- .../okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt | 9 +++++---- .../kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt | 11 ++++------- .../java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt | 14 ++++++++------ 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt index bd679acb40c7..4a66bb07c00d 100644 --- a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt +++ b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt @@ -70,10 +70,11 @@ internal class DnsOverHttpsCall( ?: return // Already complete or canceled; nothing to do. val newRunningCalls = previous.runningCalls - call - val allFailures = when { - canceledException != null -> listOf(canceledException) - else -> previous.delayedFailures + e - } + val allFailures = + when { + canceledException != null -> listOf(canceledException) + else -> previous.delayedFailures + e + } val next = when { newRunningCalls.isEmpty() -> { diff --git a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt index 09aec41482b2..bbe34b39d243 100644 --- a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt +++ b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt @@ -221,8 +221,8 @@ class DnsOverHttps internal constructor( private fun callsList( hostname: String, inetAddressesOnly: Boolean = false, - ): List { - return buildList { + ): List = + buildList { if (includeHttps && !inetAddressesOnly) { add(createCall(hostname, TYPE_HTTPS)) } @@ -233,7 +233,6 @@ class DnsOverHttps internal constructor( add(createCall(hostname, TYPE_A)) } - } class Builder { internal var client: OkHttpClient? = null @@ -305,8 +304,7 @@ class DnsOverHttps internal constructor( this.bootstrapDnsHosts = bootstrapDnsHosts } - fun bootstrapDnsHosts(vararg bootstrapDnsHosts: InetAddress): Builder = - bootstrapDnsHosts(bootstrapDnsHosts.toList()) + fun bootstrapDnsHosts(vararg bootstrapDnsHosts: InetAddress): Builder = bootstrapDnsHosts(bootstrapDnsHosts.toList()) fun systemDns(systemDns: Dns) = apply { @@ -328,7 +326,6 @@ class DnsOverHttps internal constructor( } } - internal fun isPrivateHost(host: String): Boolean = - PublicSuffixDatabase.get().getEffectiveTldPlusOne(host) == null + internal fun isPrivateHost(host: String): Boolean = PublicSuffixDatabase.get().getEffectiveTldPlusOne(host) == null } } diff --git a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt index b1ce8c74e998..6eb76c7f9e7e 100644 --- a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt +++ b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt @@ -158,9 +158,10 @@ class DnsOverHttpsTest( fun resolveForbiddenPrivateDomain() { dns = buildLocalhost(bootstrapClient, resolvePrivateAddresses = false) - val e = assertFailsWith { - dns(entryPoint, "dev") - } + val e = + assertFailsWith { + dns(entryPoint, "dev") + } assertThat(e).hasMessage("private hosts not resolved") assertThat(dnsOverHttpsServer.pollRequest()).isNull() } @@ -169,9 +170,10 @@ class DnsOverHttpsTest( fun resolveForbiddenPublicDomain() { dns = buildLocalhost(bootstrapClient, resolvePublicAddresses = false) - val e = assertFailsWith { - dns(entryPoint, "lysine.dev") - } + val e = + assertFailsWith { + dns(entryPoint, "lysine.dev") + } assertThat(e).hasMessage("public hosts not resolved") assertThat(dnsOverHttpsServer.pollRequest()).isNull() } From 76c52096731ff0e84ce8bb295558f958ffd95ea2 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Fri, 17 Jul 2026 09:09:23 -0400 Subject: [PATCH 4/6] Test canceling after a partial success --- .../okhttp3/dnsoverhttps/DnsOverHttpsTest.kt | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt index 6eb76c7f9e7e..05489c551165 100644 --- a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt +++ b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt @@ -43,6 +43,7 @@ import okhttp3.Headers.Companion.headersOf import okhttp3.Interceptor import okhttp3.OkHttpClient import okhttp3.Protocol +import okhttp3.Response import okhttp3.testing.PlatformRule import okio.Buffer import okio.ByteString.Companion.decodeHex @@ -563,6 +564,50 @@ class DnsOverHttpsTest( assertThat(dnsEvents.take()).isInstanceOf() } + @Test + fun callIsCanceledAfterPartialSuccess() { + assumeTrue(entryPoint == EntryPoint.NewCall) + + dns = buildLocalhost(bootstrapClient, includeIPv6 = true) + dnsOverHttpsServer["lysine.dev"] = + listOf( + ResourceRecord.IpAddress( + name = "lysine.dev", + timeToLive = 5, + address = InetAddress.getByName("11:22::33:44"), + ), + ) + + val call = dns.newCall(Dns2.Request("lysine.dev")) + + interceptor = + object: Interceptor { + var requestCount = 0 + override fun intercept(chain: Interceptor.Chain): Response { + if (requestCount++ == 1) { + call.cancel() + assertThat(call.isCanceled()).isTrue() + } + return chain.proceed(chain.request()) + } + } + + val dnsEvents = call.execute() + assertThat(dnsEvents.take()).isEqualTo( + DnsEvent.Records( + last = false, + records = + listOf( + Dns2.Record.IpAddress( + hostname = "lysine.dev", + address = InetAddress.getByName("11:22::33:44"), + ), + ), + ), + ) + assertThat(dnsEvents.take()).isInstanceOf() + } + private fun cacheEvents(): List> = eventRecorder .recordedEventTypes() From 445455f586b38fcccf5982935482aff523dd3136 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Fri, 17 Jul 2026 09:15:59 -0400 Subject: [PATCH 5/6] Use a volatile to track whether it's canceled --- .../main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt index 4a66bb07c00d..0a4dfe214107 100644 --- a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt +++ b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/-DnsOverHttpsCall.kt @@ -41,6 +41,7 @@ internal class DnsOverHttpsCall( private val canceledException: IOException?, ) : Dns2.Call, Callback { + @Volatile private var canceled = false private val state = AtomicReference(State.Idle) override fun enqueue(callback: Dns2.Callback) { @@ -188,12 +189,15 @@ internal class DnsOverHttpsCall( } override fun cancel() { + if (canceled) return // Already canceled. + + canceled = true for (call in calls) { call.cancel() } } - override fun isCanceled() = calls.all { it.isCanceled() } + override fun isCanceled() = canceled private sealed interface State { object Idle : State From a8ba0e7c488ca5d76824f9a985e569f200ee0cc6 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Fri, 17 Jul 2026 09:46:13 -0400 Subject: [PATCH 6/6] Spotless --- .../src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt index 05489c551165..099dc1428e9b 100644 --- a/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt +++ b/okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.kt @@ -581,8 +581,9 @@ class DnsOverHttpsTest( val call = dns.newCall(Dns2.Request("lysine.dev")) interceptor = - object: Interceptor { + object : Interceptor { var requestCount = 0 + override fun intercept(chain: Interceptor.Chain): Response { if (requestCount++ == 1) { call.cancel()