Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions okhttp-dnsoverhttps/api/okhttp-dnsoverhttps.api
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
public final class okhttp3/dnsoverhttps/DnsOverHttps : okhttp3/Dns {
public static final field Companion Lokhttp3/dnsoverhttps/DnsOverHttps$Companion;
public static final field MAX_RESPONSE_SIZE I
public final fun cache ()Lokhttp3/DnsCache;
public final fun client ()Lokhttp3/OkHttpClient;
public final fun includeIPv6 ()Z
public final fun includeServiceMetadata ()Z
Expand All @@ -17,6 +18,7 @@ public final class okhttp3/dnsoverhttps/DnsOverHttps$Builder {
public final fun bootstrapDnsHosts (Ljava/util/List;)Lokhttp3/dnsoverhttps/DnsOverHttps$Builder;
public final fun bootstrapDnsHosts ([Ljava/net/InetAddress;)Lokhttp3/dnsoverhttps/DnsOverHttps$Builder;
public final fun build ()Lokhttp3/dnsoverhttps/DnsOverHttps;
public final fun cache (Lokhttp3/DnsCache;)Lokhttp3/dnsoverhttps/DnsOverHttps$Builder;
public final fun client (Lokhttp3/OkHttpClient;)Lokhttp3/dnsoverhttps/DnsOverHttps$Builder;
public final fun includeIPv6 (Z)Lokhttp3/dnsoverhttps/DnsOverHttps$Builder;
public final fun includeServiceMetadata (Z)Lokhttp3/dnsoverhttps/DnsOverHttps$Builder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package okhttp3.dnsoverhttps
import java.net.InetAddress
import java.net.UnknownHostException
import okhttp3.Dns
import okhttp3.DnsCache
import okhttp3.HttpUrl
import okhttp3.MediaType
import okhttp3.MediaType.Companion.toMediaType
Expand All @@ -42,20 +43,23 @@ class DnsOverHttps internal constructor(
private val taskRunner: TaskRunner,
@get:JvmName("client") val client: OkHttpClient,
@get:JvmName("url") val url: HttpUrl,
@get:JvmName("cache") val cache: DnsCache,
@get:JvmName("includeIPv6") val includeIPv6: Boolean,
@get:JvmName("includeServiceMetadata") val includeServiceMetadata: Boolean,
@get:JvmName("post") val post: Boolean,
@get:JvmName("resolvePrivateAddresses") val resolvePrivateAddresses: Boolean,
@get:JvmName("resolvePublicAddresses") val resolvePublicAddresses: Boolean,
) : Dns {
private val queryFactory =
DnsOverHttpsQuery.Factory(
taskRunner = taskRunner,
resolvePrivateAddresses = resolvePrivateAddresses,
resolvePublicAddresses = resolvePublicAddresses,
client = client,
dnsUrl = url,
post = post,
cache.`-delegate`.wrap(
DnsOverHttpsQuery.Factory(
taskRunner = taskRunner,
resolvePrivateAddresses = resolvePrivateAddresses,
resolvePublicAddresses = resolvePublicAddresses,
client = client,
dnsUrl = url,
post = post,
),
)

override fun newCall(request: Dns.Request): Dns.Call =
Expand All @@ -74,6 +78,7 @@ class DnsOverHttps internal constructor(
taskRunner = taskRunner,
client = client,
url = url,
cache = cache,
includeIPv6 = includeIPv6,
includeServiceMetadata = false,
post = post,
Expand All @@ -91,6 +96,7 @@ class DnsOverHttps internal constructor(
internal val taskRunner = TaskRunner.INSTANCE
internal var client: OkHttpClient? = null
internal var url: HttpUrl? = null
internal var cache: DnsCache = DnsCache()
internal var includeIPv6 = true
internal var includeServiceMetadata = true
internal var post = false
Expand All @@ -105,6 +111,7 @@ class DnsOverHttps internal constructor(
taskRunner = taskRunner,
client = client.newBuilder().dns(buildBootstrapClient(this)).build(),
url = checkNotNull(url) { "url not set" },
cache = cache,
includeIPv6 = includeIPv6,
includeServiceMetadata = includeServiceMetadata,
post = post,
Expand All @@ -123,6 +130,11 @@ class DnsOverHttps internal constructor(
this.url = url
}

fun cache(cache: DnsCache) =
apply {
this.cache = cache
}

/**
* True to request [`HTTPS` DNS records](https://datatracker.ietf.org/doc/rfc9460/), which are
* necessary for [Encrypted Client Hello (ECH)](https://datatracker.ietf.org/doc/rfc9849/).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import okhttp3.CallEvent.CacheHit
import okhttp3.CallEvent.CacheMiss
import okhttp3.Dispatcher
import okhttp3.Dns
import okhttp3.DnsCache
import okhttp3.EventRecorder
import okhttp3.FakeDns
import okhttp3.FakeDns.Request.DnsOverHttpsRequest
Expand Down Expand Up @@ -156,6 +157,28 @@ class DnsOverHttpsTest(
.isEqualTo(queryRequest("lysine.dev", TYPE_A))
}

@Test
fun getWithCache() {
val dnsCache = DnsCache()
dns = buildLocalhost(bootstrapClient, dnsCache = dnsCache)

// Put a sample record in for the first query.
server["lysine.dev"] = listOf(InetAddress.getByName("10.20.30.40"))
val result0 = dns.invoke(entryPoint, "lysine.dev")
assertThat(result0).isEqualTo(listOf(address("10.20.30.40")))
val (httpsRequest, dnsRequest) = server.takeRequest() as DnsOverHttpsRequest
assertThat(httpsRequest.method).isEqualTo("GET")
assertThat(dnsRequest)
.isEqualTo(queryRequest("lysine.dev", TYPE_A))

// Put a different record in for the second query. We'll get the first record because that's
// what is in the cache.
server["lysine.dev"] = listOf(InetAddress.getByName("55.66.77.88"))
val result1 = dns.invoke(entryPoint, "lysine.dev")
assertThat(result1).isEqualTo(listOf(address("10.20.30.40"))) // Stale! Cache worked!
assertThat(server.pollRequest()).isNull() // No request.
}

@Test
fun getIpv6() {
server["lysine.dev"] =
Expand Down Expand Up @@ -293,10 +316,14 @@ class DnsOverHttpsTest(
// 4. successful stale cached GET response
// 5. unsuccessful response
@Test
fun usesCache() {
fun usesHttpCache() {
val cache = Cache(cacheFs, "cache".toPath(), (100 * 1024).toLong())
val cachedClient = bootstrapClient.newBuilder().cache(cache).build()
val cachedDns = buildLocalhost(cachedClient)
val cachedDns =
buildLocalhost(
bootstrapClient = cachedClient,
dnsCache = DnsCache(maxEntryCount = 0),
)

server.extraHeaders =
headersOf(
Expand Down Expand Up @@ -332,10 +359,15 @@ class DnsOverHttpsTest(
}

@Test
fun usesCacheEvenForPost() {
fun usesHttpCacheEvenForPost() {
val cache = Cache(cacheFs, "cache".toPath(), (100 * 1024).toLong())
val cachedClient = bootstrapClient.newBuilder().cache(cache).build()
val cachedDns = buildLocalhost(cachedClient, post = true)
val cachedDns =
buildLocalhost(
bootstrapClient = cachedClient,
post = true,
dnsCache = DnsCache(maxEntryCount = 0),
)
server.extraHeaders =
headersOf(
"cache-control",
Expand Down Expand Up @@ -370,10 +402,14 @@ class DnsOverHttpsTest(
}

@Test
fun usesCacheOnlyIfFresh() {
fun usesHttpCacheOnlyIfFresh() {
val cache = Cache(File("./target/DnsOverHttpsTest.cache"), 100 * 1024L)
val cachedClient = bootstrapClient.newBuilder().cache(cache).build()
val cachedDns = buildLocalhost(cachedClient)
val cachedDns =
buildLocalhost(
bootstrapClient = cachedClient,
dnsCache = DnsCache(maxEntryCount = 0),
)
server.extraHeaders =
headersOf(
"cache-control",
Expand Down Expand Up @@ -835,6 +871,7 @@ class DnsOverHttpsTest(

private fun buildLocalhost(
bootstrapClient: OkHttpClient,
dnsCache: DnsCache = DnsCache(),
includeIPv6: Boolean = false,
includeServiceMetadata: Boolean = false,
post: Boolean = false,
Expand All @@ -845,6 +882,7 @@ class DnsOverHttpsTest(
return DnsOverHttps
.Builder()
.client(bootstrapClient)
.cache(dnsCache)
.includeIPv6(includeIPv6)
.includeServiceMetadata(includeServiceMetadata)
.resolvePrivateAddresses(resolvePrivateAddresses)
Expand Down
4 changes: 2 additions & 2 deletions okhttp/api/android/okhttp.api
Original file line number Diff line number Diff line change
Expand Up @@ -1383,8 +1383,8 @@ public abstract class okhttp3/WebSocketListener {

public final class okhttp3/android/AndroidDns : okhttp3/Dns {
public fun <init> ()V
public fun <init> (Landroid/net/DnsResolver;Landroid/net/Network;ZLjava/util/concurrent/Executor;)V
public synthetic fun <init> (Landroid/net/DnsResolver;Landroid/net/Network;ZLjava/util/concurrent/Executor;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (Landroid/net/DnsResolver;Landroid/net/Network;Lokhttp3/DnsCache;ZLjava/util/concurrent/Executor;)V
public synthetic fun <init> (Landroid/net/DnsResolver;Landroid/net/Network;Lokhttp3/DnsCache;ZLjava/util/concurrent/Executor;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun lookup (Ljava/lang/String;)Ljava/util/List;
public fun newCall (Lokhttp3/Dns$Request;)Lokhttp3/Dns$Call;
}
Expand Down
4 changes: 3 additions & 1 deletion okhttp/src/androidMain/kotlin/okhttp3/android/AndroidDns.kt
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import java.net.InetAddress
import java.net.UnknownHostException
import java.util.concurrent.Executor
import okhttp3.Dns
import okhttp3.DnsCache
import okhttp3.internal.OkHttpInternalApi
import okhttp3.internal.SuppressSignatureCheck
import okhttp3.internal.concurrent.TaskRunner
Expand All @@ -53,6 +54,7 @@ import okio.Buffer
class AndroidDns(
private val dnsResolver: DnsResolver = DnsResolver.getInstance(),
private val network: Network? = null,
dnsCache: DnsCache = DnsCache(),
/**
* True to also query the `HTTPS` record for service metadata. Keep this on: it enables privacy
* features such as Encrypted Client Hello (ECH) for the HTTPS call. Set it to false only when
Expand All @@ -66,7 +68,7 @@ class AndroidDns(

/** Drives [StateMachineDnsCall] using the system resolver and [DnsResolver]. */
private val queryFactory =
DnsQuery.Factory { question ->
dnsCache.`-delegate`.wrap { question ->

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we do this, we are also responsible for evicting on network changes. So it's if it's the default, we need to ship that wiring also.

This is fine until some oblivious well intentioned grunt makes it the default in an unrelated PR.

AndroidQuery(question)
}

Expand Down
Loading