diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 32232ee16..9ca90fa2c 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -190,6 +190,8 @@ dependencies { implementation(libs.conscrypt) implementation(libs.dnsjava) implementation(libs.guava) + implementation(libs.ktor.client.core) + implementation(libs.ktor.client.okhttp) implementation(libs.mikepenz.aboutLibraries.m3) implementation(libs.okhttp.base) implementation(libs.okhttp.brotli) diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientBuilderTest.kt similarity index 77% rename from app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientTest.kt rename to app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientBuilderTest.kt index f08125b1f..1e8cfe836 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientBuilderTest.kt @@ -7,7 +7,9 @@ package at.bitfire.davdroid.network import android.security.NetworkSecurityPolicy import dagger.hilt.android.testing.HiltAndroidRule import dagger.hilt.android.testing.HiltAndroidTest -import okhttp3.OkHttpClient +import io.ktor.client.request.get +import io.ktor.client.statement.bodyAsText +import kotlinx.coroutines.test.runTest import okhttp3.Request import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer @@ -20,25 +22,23 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import javax.inject.Inject +import javax.inject.Provider @HiltAndroidTest -class HttpClientTest { +class HttpClientBuilderTest { @get:Rule var hiltRule = HiltAndroidRule(this) @Inject - lateinit var httpClientBuilder: HttpClientBuilder + lateinit var httpClientBuilder: Provider - lateinit var httpClient: OkHttpClient lateinit var server: MockWebServer @Before fun setUp() { hiltRule.inject() - httpClient = httpClientBuilder.build() - server = MockWebServer() server.start(30000) } @@ -49,6 +49,18 @@ class HttpClientTest { } + @Test + fun testBuildKtor_CreatesWorkingClient() = runTest { + server.enqueue(MockResponse() + .setResponseCode(200) + .setBody("Some Content")) + + val client = httpClientBuilder.get().buildKtor() + val response = client.get(server.url("/").toString()) + assertEquals(200, response.status.value) + assertEquals("Some Content", response.bodyAsText()) + } + @Test fun testCookies() { Assume.assumeTrue(NetworkSecurityPolicy.getInstance().isCleartextTrafficPermitted) @@ -60,6 +72,8 @@ class HttpClientTest { .addHeader("Set-Cookie", "cookie1=1; path=/") .addHeader("Set-Cookie", "cookie2=2") .setBody("Cookie set")) + + val httpClient = httpClientBuilder.get().build() httpClient.newCall(Request.Builder() .get().url(url) .build()).execute() diff --git a/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClientBuilder.kt b/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClientBuilder.kt index 6ec95ac5a..f4558f718 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClientBuilder.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClientBuilder.kt @@ -19,6 +19,8 @@ import at.bitfire.davdroid.settings.SettingsManager import at.bitfire.davdroid.ui.ForegroundTracker import com.google.common.net.HttpHeaders import dagger.hilt.android.qualifiers.ApplicationContext +import io.ktor.client.HttpClient +import io.ktor.client.engine.okhttp.OkHttp import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.withContext import net.openid.appauth.AuthState @@ -181,17 +183,17 @@ class HttpClientBuilder @Inject constructor( } - // actual builder + // okhttp builder /** - * Builds the [OkHttpClient]. + * Builds an [OkHttpClient] with the configured settings. * - * Must be called only once because multiple calls indicate this wrong usage pattern: + * [build] or [buildKtor] must be called only once because multiple calls indicate this wrong usage pattern: * * ``` * val builder = HttpClientBuilder(/*injected*/) - * val client1 = builder.configure().builder() - * val client2 = builder.configureOtherwise().builder() + * val client1 = builder.configure().build() + * val client2 = builder.configureOtherwise().build() * ``` * * However in this case the configuration of `client1` is still in `builder` and would be reused for `client2`, @@ -199,42 +201,39 @@ class HttpClientBuilder @Inject constructor( * * @throws IllegalStateException on second and later calls */ + @Deprecated("Use buildKtor instead", replaceWith = ReplaceWith("buildKtor()")) fun build(): OkHttpClient { if (alreadyBuilt) throw IllegalStateException("build() must only be called once; use Provider") - val okBuilder = OkHttpClient.Builder() - // Set timeouts. According to [AbstractThreadedSyncAdapter], when there is no network - // traffic within a minute, a sync will be cancelled. - .connectTimeout(15, TimeUnit.SECONDS) - .writeTimeout(30, TimeUnit.SECONDS) - .readTimeout(120, TimeUnit.SECONDS) - .pingInterval(45, TimeUnit.SECONDS) // avoid cancellation because of missing traffic; only works for HTTP/2 + val builder = OkHttpClient.Builder() + configureOkHttp(builder) - // don't allow redirects by default because it would break PROPFIND handling - .followRedirects(followRedirects) + alreadyBuilt = true + return builder.build() + } - // add User-Agent to every request - .addInterceptor(UserAgentInterceptor) + private fun configureOkHttp(builder: OkHttpClient.Builder) { + buildTimeouts(builder) - // connection-private cookie store - .cookieJar(cookieStore) + // don't allow redirects by default because it would break PROPFIND handling + builder.followRedirects(followRedirects) - // allow cleartext and TLS 1.2+ - .connectionSpecs(listOf( - ConnectionSpec.Companion.CLEARTEXT, - ConnectionSpec.Companion.MODERN_TLS - )) + // add User-Agent to every request + builder.addInterceptor(UserAgentInterceptor) - // offer Brotli and gzip compression (can be disabled per request with `Accept-Encoding: identity`) - .addInterceptor(BrotliInterceptor) + // connection-private cookie store + builder.cookieJar(cookieStore) + + // offer Brotli and gzip compression (can be disabled per request with `Accept-Encoding: identity`) + builder.addInterceptor(BrotliInterceptor) // app-wide custom proxy support - buildProxy(okBuilder) + buildProxy(builder) // add connection security (including client certificates) and authentication - buildConnectionSecurity(okBuilder) - buildAuthentication(okBuilder) + buildConnectionSecurity(builder) + buildAuthentication(builder) // add network logging, if requested if (logger.isLoggable(Level.FINEST)) { @@ -244,11 +243,8 @@ class HttpClientBuilder @Inject constructor( loggingInterceptor.redactHeader(HttpHeaders.SET_COOKIE) loggingInterceptor.redactHeader(HttpHeaders.SET_COOKIE2) loggingInterceptor.level = loggerInterceptorLevel - okBuilder.addNetworkInterceptor(loggingInterceptor) + builder.addNetworkInterceptor(loggingInterceptor) } - - alreadyBuilt = true - return okBuilder.build() } private fun buildAuthentication(okBuilder: OkHttpClient.Builder) { @@ -258,6 +254,12 @@ class HttpClientBuilder @Inject constructor( } private fun buildConnectionSecurity(okBuilder: OkHttpClient.Builder) { + // allow cleartext and TLS 1.2+ + okBuilder.connectionSpecs(listOf( + ConnectionSpec.CLEARTEXT, + ConnectionSpec.MODERN_TLS + )) + // client certificate val clientKeyManager: KeyManager? = certificateAlias?.let { alias -> try { @@ -346,4 +348,55 @@ class HttpClientBuilder @Inject constructor( } } + /** + * Set timeouts for the connection. + * + * **Note:** According to [android.content.AbstractThreadedSyncAdapter], when there is no network + * traffic within a minute, a sync will be cancelled. + */ + private fun buildTimeouts(builder: OkHttpClient.Builder) { + builder.connectTimeout(15, TimeUnit.SECONDS) + .writeTimeout(30, TimeUnit.SECONDS) + .readTimeout(120, TimeUnit.SECONDS) + .pingInterval(45, TimeUnit.SECONDS) // avoid cancellation because of missing traffic; only works for HTTP/2 + } + + + // Ktor builder + + /** + * Builds a Ktor [HttpClient] with the configured settings. + * + * [buildKtor] or [build] must be called only once because multiple calls indicate this wrong usage pattern: + * + * ``` + * val builder = HttpClientBuilder(/*injected*/) + * val client1 = builder.configure().buildKtor() + * val client2 = builder.configureOtherwise().buildKtor() + * ``` + * + * However in this case the configuration of `client1` is still in `builder` and would be reused for `client2`, + * which is usually not desired. + */ + fun buildKtor(): HttpClient { + if (alreadyBuilt) + throw IllegalStateException("build() must only be called once; use Provider") + + val client = HttpClient(OkHttp) { + // Ktor-level configuration here + + engine { + // okhttp engine configuration here + + config { + // OkHttpClient.Builder configuration here + configureOkHttp(this) + } + } + } + + alreadyBuilt = true + return client + } + } \ No newline at end of file diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 303674d1e..04a5bfb9f 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -32,6 +32,7 @@ hilt = "2.57.2" kotlin = "2.2.21" kotlinx-coroutines = "1.10.2" ksp = "2.3.2" +ktor = "3.3.2" mikepenz-aboutLibraries = "13.1.0" mockk = "1.14.5" okhttp = "5.3.0" @@ -93,6 +94,8 @@ junit = { module = "junit:junit", version = "4.13.2" } kotlin-stdlib = { module = "org.jetbrains.kotlin:kotlin-stdlib", version.ref = "kotlin" } kotlinx-coroutines = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "kotlinx-coroutines" } kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "kotlinx-coroutines" } +ktor-client-core = { module = "io.ktor:ktor-client-core", version.ref = "ktor" } +ktor-client-okhttp = { module = "io.ktor:ktor-client-okhttp", version.ref = "ktor" } mikepenz-aboutLibraries-m3 = { module = "com.mikepenz:aboutlibraries-compose-m3", version.ref = "mikepenz-aboutLibraries" } mockk = { module = "io.mockk:mockk", version.ref = "mockk" } mockk-android = { module = "io.mockk:mockk-android", version.ref = "mockk" }