From 6595d590f5cc456d5ba13bb72f9733d793664dd1 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sat, 6 Apr 2024 10:15:25 +0100 Subject: [PATCH 1/7] StrictMode warning --- .../main/kotlin/okhttp3/internal/platform/AndroidPlatform.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/okhttp/src/main/kotlin/okhttp3/internal/platform/AndroidPlatform.kt b/okhttp/src/main/kotlin/okhttp3/internal/platform/AndroidPlatform.kt index 8a6f67a8670f..c5db83a37dff 100644 --- a/okhttp/src/main/kotlin/okhttp3/internal/platform/AndroidPlatform.kt +++ b/okhttp/src/main/kotlin/okhttp3/internal/platform/AndroidPlatform.kt @@ -16,6 +16,7 @@ package okhttp3.internal.platform import android.os.Build +import android.os.StrictMode import android.security.NetworkSecurityPolicy import java.io.IOException import java.lang.reflect.InvocationTargetException @@ -42,6 +43,10 @@ import okhttp3.internal.tls.TrustRootIndex /** Android 5 to 9 (API 21 to 28). */ @SuppressSignatureCheck class AndroidPlatform : Platform() { + init { + StrictMode.noteSlowCall("okhttp3.internal.platform.Pl") + } + private val socketAdapters = listOfNotNull( StandardAndroidSocketAdapter.buildIfSupported(), From e1147942be7339a550ddd004319e0b5f63d7a684 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sat, 6 Apr 2024 10:28:24 +0100 Subject: [PATCH 2/7] Fix test --- .../okhttp3/android/OkHttpStrictModeTest.kt | 33 +++++++++++++++++++ .../internal/platform/AndroidPlatform.kt | 13 +++++--- 2 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 okhttp-android/src/test/kotlin/okhttp3/android/OkHttpStrictModeTest.kt diff --git a/okhttp-android/src/test/kotlin/okhttp3/android/OkHttpStrictModeTest.kt b/okhttp-android/src/test/kotlin/okhttp3/android/OkHttpStrictModeTest.kt new file mode 100644 index 000000000000..7dcaaf8f6f4d --- /dev/null +++ b/okhttp-android/src/test/kotlin/okhttp3/android/OkHttpStrictModeTest.kt @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2022 Square, Inc. + * + * 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.android + +import java.net.UnknownHostException +import javafx.application.Platform +import okhttp3.ConnectionSpec +import okhttp3.OkHttpClient +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class OkHttpStrictModeTest { + @Test + fun testStrictMode() { + val clientBuilder = OkHttpClient() + } +} diff --git a/okhttp/src/main/kotlin/okhttp3/internal/platform/AndroidPlatform.kt b/okhttp/src/main/kotlin/okhttp3/internal/platform/AndroidPlatform.kt index c5db83a37dff..1d4e007530ba 100644 --- a/okhttp/src/main/kotlin/okhttp3/internal/platform/AndroidPlatform.kt +++ b/okhttp/src/main/kotlin/okhttp3/internal/platform/AndroidPlatform.kt @@ -25,6 +25,7 @@ import java.net.InetSocketAddress import java.net.Socket import java.security.cert.TrustAnchor import java.security.cert.X509Certificate +import javax.net.ssl.SSLContext import javax.net.ssl.SSLSocket import javax.net.ssl.SSLSocketFactory import javax.net.ssl.X509TrustManager @@ -43,10 +44,6 @@ import okhttp3.internal.tls.TrustRootIndex /** Android 5 to 9 (API 21 to 28). */ @SuppressSignatureCheck class AndroidPlatform : Platform() { - init { - StrictMode.noteSlowCall("okhttp3.internal.platform.Pl") - } - private val socketAdapters = listOfNotNull( StandardAndroidSocketAdapter.buildIfSupported(), @@ -75,6 +72,12 @@ class AndroidPlatform : Platform() { } } + override fun newSSLContext(): SSLContext { + StrictMode.noteSlowCall("newSSLContext") + + return super.newSSLContext() + } + override fun trustManager(sslSocketFactory: SSLSocketFactory): X509TrustManager? = socketAdapters.find { it.matchesSocketFactory(sslSocketFactory) } ?.trustManager(sslSocketFactory) @@ -105,6 +108,8 @@ class AndroidPlatform : Platform() { override fun buildTrustRootIndex(trustManager: X509TrustManager): TrustRootIndex = try { + StrictMode.noteSlowCall("buildTrustRootIndex") + // From org.conscrypt.TrustManagerImpl, we want the method with this signature: // private TrustAnchor findTrustAnchorByIssuerAndSignature(X509Certificate lastCert); val method = From 467aa8213e77e5301ba76a39ddbf3a8e952ba59d Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sat, 6 Apr 2024 11:56:56 +0100 Subject: [PATCH 3/7] Add strict mode checks on slow calls --- .../okhttp/android/test/StrictModeTest.kt | 66 +++++++++++++++++++ .../internal/platform/Android10Platform.kt | 15 +++++ 2 files changed, 81 insertions(+) create mode 100644 android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt diff --git a/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt b/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt new file mode 100644 index 000000000000..c674e385cf4a --- /dev/null +++ b/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt @@ -0,0 +1,66 @@ +package okhttp.android.test + +import android.os.StrictMode +import android.os.StrictMode.ThreadPolicy +import assertk.assertThat +import assertk.assertions.hasMessage +import okhttp3.HttpUrl.Companion.toHttpUrl +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.internal.platform.Platform +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import org.junit.jupiter.api.fail +import org.junit.jupiter.api.parallel.Isolated +import org.opentest4j.AssertionFailedError + +@Isolated +class StrictModeTest { + @AfterEach + fun cleanup() { + StrictMode.setThreadPolicy( + ThreadPolicy.Builder() + .permitAll() + .build(), + ) + } + + @Test + fun testInit() { + Platform.resetForTests() + + applyStrictMode() + + val e = + assertThrows { + // Not currently safe + // See https://github.com/square/okhttp/pull/8248 + OkHttpClient() + } + assertThat(e).hasMessage("Slow call on main") + } + + @Test + fun testNewCall() { + Platform.resetForTests() + + val client = OkHttpClient() + + applyStrictMode() + + // Safe on main + client.newCall(Request("https://google.com/robots.txt".toHttpUrl())) + } + + private fun applyStrictMode() { + StrictMode.setThreadPolicy( + ThreadPolicy.Builder() + .detectCustomSlowCalls() + .penaltyListener({ it.run() }) { + fail("Slow call on main") + } + .build(), + ) + } +} diff --git a/okhttp/src/main/kotlin/okhttp3/internal/platform/Android10Platform.kt b/okhttp/src/main/kotlin/okhttp3/internal/platform/Android10Platform.kt index 02df7e3fdfde..ea31d548fcec 100644 --- a/okhttp/src/main/kotlin/okhttp3/internal/platform/Android10Platform.kt +++ b/okhttp/src/main/kotlin/okhttp3/internal/platform/Android10Platform.kt @@ -17,8 +17,10 @@ package okhttp3.internal.platform import android.annotation.SuppressLint import android.os.Build +import android.os.StrictMode import android.security.NetworkSecurityPolicy import android.util.CloseGuard +import javax.net.ssl.SSLContext import javax.net.ssl.SSLSocket import javax.net.ssl.SSLSocketFactory import javax.net.ssl.X509TrustManager @@ -31,6 +33,7 @@ import okhttp3.internal.platform.android.BouncyCastleSocketAdapter import okhttp3.internal.platform.android.ConscryptSocketAdapter import okhttp3.internal.platform.android.DeferredSocketAdapter import okhttp3.internal.tls.CertificateChainCleaner +import okhttp3.internal.tls.TrustRootIndex /** Android 10+ (API 29+). */ @SuppressSignatureCheck @@ -48,6 +51,18 @@ class Android10Platform : Platform() { socketAdapters.find { it.matchesSocketFactory(sslSocketFactory) } ?.trustManager(sslSocketFactory) + override fun newSSLContext(): SSLContext { + StrictMode.noteSlowCall("newSSLContext") + + return super.newSSLContext() + } + + override fun buildTrustRootIndex(trustManager: X509TrustManager): TrustRootIndex { + StrictMode.noteSlowCall("buildTrustRootIndex") + + return super.buildTrustRootIndex(trustManager) + } + override fun configureTlsExtensions( sslSocket: SSLSocket, hostname: String?, From 2446e0141d251a14f12bf8503626274e533f2904 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sat, 6 Apr 2024 11:57:31 +0100 Subject: [PATCH 4/7] Add strict mode checks on slow calls --- .../okhttp3/android/OkHttpStrictModeTest.kt | 33 ------------------- 1 file changed, 33 deletions(-) delete mode 100644 okhttp-android/src/test/kotlin/okhttp3/android/OkHttpStrictModeTest.kt diff --git a/okhttp-android/src/test/kotlin/okhttp3/android/OkHttpStrictModeTest.kt b/okhttp-android/src/test/kotlin/okhttp3/android/OkHttpStrictModeTest.kt deleted file mode 100644 index 7dcaaf8f6f4d..000000000000 --- a/okhttp-android/src/test/kotlin/okhttp3/android/OkHttpStrictModeTest.kt +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright (c) 2022 Square, Inc. - * - * 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.android - -import java.net.UnknownHostException -import javafx.application.Platform -import okhttp3.ConnectionSpec -import okhttp3.OkHttpClient -import org.junit.Test -import org.junit.runner.RunWith -import org.robolectric.RobolectricTestRunner - -@RunWith(RobolectricTestRunner::class) -class OkHttpStrictModeTest { - @Test - fun testStrictMode() { - val clientBuilder = OkHttpClient() - } -} From f4805b00179502d911640408fae2ab9c191a3688 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sat, 6 Apr 2024 12:42:50 +0100 Subject: [PATCH 5/7] dont fail --- .../okhttp/android/test/StrictModeTest.kt | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt b/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt index c674e385cf4a..c8e92031c9ee 100644 --- a/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt +++ b/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt @@ -2,8 +2,12 @@ package okhttp.android.test import android.os.StrictMode import android.os.StrictMode.ThreadPolicy +import android.os.strictmode.Violation import assertk.assertThat import assertk.assertions.hasMessage +import assertk.assertions.hasSize +import assertk.assertions.isEmpty +import assertk.assertions.isEqualTo import okhttp3.HttpUrl.Companion.toHttpUrl import okhttp3.OkHttpClient import okhttp3.Request @@ -17,6 +21,8 @@ import org.opentest4j.AssertionFailedError @Isolated class StrictModeTest { + private val violations = mutableListOf() + @AfterEach fun cleanup() { StrictMode.setThreadPolicy( @@ -32,13 +38,12 @@ class StrictModeTest { applyStrictMode() - val e = - assertThrows { - // Not currently safe - // See https://github.com/square/okhttp/pull/8248 - OkHttpClient() - } - assertThat(e).hasMessage("Slow call on main") + // Not currently safe + // See https://github.com/square/okhttp/pull/8248 + OkHttpClient() + + assertThat(violations).hasSize(1) + assertThat(violations[0].message).isEqualTo("newSSLContext") } @Test @@ -51,6 +56,8 @@ class StrictModeTest { // Safe on main client.newCall(Request("https://google.com/robots.txt".toHttpUrl())) + + assertThat(violations).isEmpty() } private fun applyStrictMode() { @@ -58,7 +65,7 @@ class StrictModeTest { ThreadPolicy.Builder() .detectCustomSlowCalls() .penaltyListener({ it.run() }) { - fail("Slow call on main") + violations.add(it) } .build(), ) From 3389def953e98eb36d32528610640befdb253d2c Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sat, 6 Apr 2024 15:21:21 +0100 Subject: [PATCH 6/7] Update StrictModeTest.kt --- .../src/androidTest/java/okhttp/android/test/StrictModeTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt b/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt index c8e92031c9ee..46b63745abca 100644 --- a/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt +++ b/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt @@ -4,7 +4,6 @@ import android.os.StrictMode import android.os.StrictMode.ThreadPolicy import android.os.strictmode.Violation import assertk.assertThat -import assertk.assertions.hasMessage import assertk.assertions.hasSize import assertk.assertions.isEmpty import assertk.assertions.isEqualTo From 6f39d9a81248331eb3d7f0cc99408deb4afa93a4 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sat, 6 Apr 2024 16:05:36 +0100 Subject: [PATCH 7/7] Cleanup --- .../src/androidTest/java/okhttp/android/test/StrictModeTest.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt b/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt index 46b63745abca..6df00bf4e23b 100644 --- a/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt +++ b/android-test/src/androidTest/java/okhttp/android/test/StrictModeTest.kt @@ -13,10 +13,7 @@ import okhttp3.Request import okhttp3.internal.platform.Platform import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Test -import org.junit.jupiter.api.assertThrows -import org.junit.jupiter.api.fail import org.junit.jupiter.api.parallel.Isolated -import org.opentest4j.AssertionFailedError @Isolated class StrictModeTest {