Skip to content

Commit 7e7bf1a

Browse files
xinsong-cuixinsong-cui
committed
pr feedbacks
1 parent 22db413 commit 7e7bf1a

File tree

7 files changed

+48
-111
lines changed

7 files changed

+48
-111
lines changed

‎runtime/protocol/http-client-engines/http-client-engine-okhttp/api/http-client-engine-okhttp.api‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,11 @@ public final class aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpEngineConf
7070
public final fun getCipherSuites ()Ljava/util/List;
7171
public final fun getConnectionIdlePollingInterval-FghU774 ()Lkotlin/time/Duration;
7272
public final fun getHostnameVerifier ()Ljavax/net/ssl/HostnameVerifier;
73-
public final fun getKeyManagerProvider ()Laws/smithy/kotlin/runtime/http/engine/okhttp/TlsKeyManagersProvider;
73+
public final fun getKeyManager ()Ljavax/net/ssl/KeyManager;
7474
public final fun getMaxConcurrencyPerHost-pVg5ArA ()I
75-
public final fun getTrustManagerProvider ()Laws/smithy/kotlin/runtime/http/engine/okhttp/TlsTrustManagersProvider;
76-
public final fun setKeyManagerProvider (Laws/smithy/kotlin/runtime/http/engine/okhttp/TlsKeyManagersProvider;)V
77-
public final fun setTrustManagerProvider (Laws/smithy/kotlin/runtime/http/engine/okhttp/TlsTrustManagersProvider;)V
75+
public final fun getTrustManager ()Ljavax/net/ssl/X509TrustManager;
76+
public final fun setKeyManager (Ljavax/net/ssl/KeyManager;)V
77+
public final fun setTrustManager (Ljavax/net/ssl/X509TrustManager;)V
7878
public fun toBuilderApplicator ()Lkotlin/jvm/functions/Function1;
7979
}
8080

@@ -84,18 +84,18 @@ public final class aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpEngineConf
8484
public final fun getCipherSuites ()Ljava/util/List;
8585
public final fun getConnectionIdlePollingInterval-FghU774 ()Lkotlin/time/Duration;
8686
public final fun getHostnameVerifier ()Ljavax/net/ssl/HostnameVerifier;
87-
public final fun getKeyManagerProvider ()Laws/smithy/kotlin/runtime/http/engine/okhttp/TlsKeyManagersProvider;
87+
public final fun getKeyManager ()Ljavax/net/ssl/KeyManager;
8888
public final fun getMaxConcurrencyPerHost-0hXNFcg ()Lkotlin/UInt;
8989
public fun getTelemetryProvider ()Laws/smithy/kotlin/runtime/telemetry/TelemetryProvider;
90-
public final fun getTrustManagerProvider ()Laws/smithy/kotlin/runtime/http/engine/okhttp/TlsTrustManagersProvider;
90+
public final fun getTrustManager ()Ljavax/net/ssl/X509TrustManager;
9191
public final fun setCertificatePinner (Lokhttp3/CertificatePinner;)V
9292
public final fun setCipherSuites (Ljava/util/List;)V
9393
public final fun setConnectionIdlePollingInterval-BwNAW2A (Lkotlin/time/Duration;)V
9494
public final fun setHostnameVerifier (Ljavax/net/ssl/HostnameVerifier;)V
95-
public final fun setKeyManagerProvider (Laws/smithy/kotlin/runtime/http/engine/okhttp/TlsKeyManagersProvider;)V
95+
public final fun setKeyManager (Ljavax/net/ssl/KeyManager;)V
9696
public final fun setMaxConcurrencyPerHost-ExVfyTY (Lkotlin/UInt;)V
9797
public fun setTelemetryProvider (Laws/smithy/kotlin/runtime/telemetry/TelemetryProvider;)V
98-
public final fun setTrustManagerProvider (Laws/smithy/kotlin/runtime/http/engine/okhttp/TlsTrustManagersProvider;)V
98+
public final fun setTrustManager (Ljavax/net/ssl/X509TrustManager;)V
9999
}
100100

101101
public final class aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpEngineConfig$Companion {
@@ -151,11 +151,3 @@ public final class aws/smithy/kotlin/runtime/http/engine/okhttp/StreamingRequest
151151
public fun writeTo (Lokio/BufferedSink;)V
152152
}
153153

154-
public abstract interface class aws/smithy/kotlin/runtime/http/engine/okhttp/TlsKeyManagersProvider {
155-
public abstract fun keyManagers ()[Ljavax/net/ssl/KeyManager;
156-
}
157-
158-
public abstract interface class aws/smithy/kotlin/runtime/http/engine/okhttp/TlsTrustManagersProvider {
159-
public abstract fun trustManagers ()[Ljavax/net/ssl/TrustManager;
160-
}
161-

‎runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpEngine.kt‎

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import kotlinx.coroutines.job
2424
import okhttp3.*
2525
import okhttp3.coroutines.executeAsync
2626
import java.util.concurrent.TimeUnit
27+
import javax.net.ssl.KeyManager
2728
import javax.net.ssl.SSLContext
2829
import javax.net.ssl.X509TrustManager
2930
import kotlin.time.toJavaDuration
@@ -107,18 +108,13 @@ public fun OkHttpEngineConfig.buildClient(
107108

108109
connectionSpecs(listOf(tlsConnectionSpec(config.tlsContext, config.cipherSuites), ConnectionSpec.CLEARTEXT))
109110

110-
if (config.trustManagerProvider != null) {
111-
val (sslContext, trustManager) = createSslContext(config.trustManagerProvider, config.keyManagerProvider)
112-
sslSocketFactory(sslContext.socketFactory, trustManager)
111+
config.trustManager?.let {
112+
val sslContext = createSslContext(it, config.keyManager)
113+
sslSocketFactory(sslContext.socketFactory, trustManager!!)
113114
}
114115

115-
if (config.certificatePinner != null) {
116-
certificatePinner(config.certificatePinner)
117-
}
118-
119-
if (config.hostnameVerifier != null) {
120-
hostnameVerifier(config.hostnameVerifier)
121-
}
116+
config.certificatePinner?.let(::certificatePinner)
117+
config.hostnameVerifier?.let(::hostnameVerifier)
122118

123119
// Transient connection errors are handled by retry strategy (exceptions are wrapped and marked retryable
124120
// appropriately internally). We don't want inner retry logic that inflates the number of retries.
@@ -186,9 +182,7 @@ private fun tlsConnectionSpec(tlsContext: TlsContext, cipherSuites: List<String>
186182
.Builder(ConnectionSpec.MODERN_TLS)
187183
.tlsVersions(*okHttpTlsVersions)
188184
.apply {
189-
if (cipherSuites != null) {
190-
cipherSuites(*cipherSuites.toTypedArray())
191-
}
185+
cipherSuites?.toTypedArray()?.let(::cipherSuites)
192186
}
193187
.build()
194188
}
@@ -203,19 +197,12 @@ private fun toOkHttpTlsVersion(sdkTlsVersion: SdkTlsVersion): OkHttpTlsVersion =
203197
/**
204198
* Creates an SSL context with custom trust and key managers
205199
*/
206-
private fun createSslContext(trustManagersProvider: TlsTrustManagersProvider?, keyManagersProvider: TlsKeyManagersProvider?): Pair<SSLContext, X509TrustManager> {
207-
val trustManagers = trustManagersProvider?.trustManagers()
208-
val keyManagers = keyManagersProvider?.keyManagers()
209-
210-
if (trustManagersProvider != null) {
211-
check(!trustManagers.isNullOrEmpty()) { "Trust managers provider returned null or empty trust managers." }
212-
check(trustManagers[0] is X509TrustManager) { "Trust managers provider must return X509TrustManager." }
213-
}
200+
private fun createSslContext(trustManager: X509TrustManager, keyManager: KeyManager?): SSLContext {
201+
val keyManagers = keyManager?.let { arrayOf(it) }
202+
val trustManagers = arrayOf(trustManager)
214203

215204
val sslContext = SSLContext.getInstance("TLS")
216205
sslContext.init(keyManagers, trustManagers, null)
217206

218-
val trustManager = trustManagers!![0] as X509TrustManager
219-
220-
return sslContext to trustManager
207+
return sslContext
221208
}

‎runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpEngineConfig.kt‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import aws.smithy.kotlin.runtime.telemetry.Global
1111
import aws.smithy.kotlin.runtime.telemetry.TelemetryProvider
1212
import okhttp3.CertificatePinner
1313
import javax.net.ssl.HostnameVerifier
14+
import javax.net.ssl.KeyManager
15+
import javax.net.ssl.X509TrustManager
1416
import kotlin.time.Duration
1517

1618
/**
@@ -53,18 +55,18 @@ public class OkHttpEngineConfig private constructor(builder: Builder) : HttpClie
5355
public val maxConcurrencyPerHost: UInt = builder.maxConcurrencyPerHost ?: builder.maxConcurrency
5456

5557
/**
56-
* Provider for TLS trust managers used to validate server certificates during TLS handshake.
57-
* Trust managers determine whether to trust the certificate chain presented by a remote server.
58-
* When provided, these trust managers will be used instead of the default system trust store.
58+
* Trust manager used to validate server certificates during TLS handshake.
59+
* Determines whether to trust the certificate chain presented by a remote server.
60+
* When provided, this trust manager will be used instead of the default system trust store.
5961
*/
60-
public var trustManagerProvider: TlsTrustManagersProvider? = builder.trustManagerProvider
62+
public var trustManager: X509TrustManager? = builder.trustManager
6163

6264
/**
63-
* Provider for KeyManagers that supply client certificates for mutual TLS (mTLS) authentication.
64-
* When provided, the client will present certificates from these KeyManagers when the server
65+
* Key manager that supplies client certificates for mutual TLS (mTLS) authentication.
66+
* When provided, the client will present certificates from this key manager when the server
6567
* requests client authentication. Used for scenarios requiring client certificate authentication.
6668
*/
67-
public var keyManagerProvider: TlsKeyManagersProvider? = builder.keyManagerProvider
69+
public var keyManager: KeyManager? = builder.keyManager
6870

6971
/**
7072
* List of cipher suites to enable for TLS connections. If null, uses OkHttp defaults.
@@ -91,8 +93,8 @@ public class OkHttpEngineConfig private constructor(builder: Builder) : HttpClie
9193
if (this is Builder) {
9294
connectionIdlePollingInterval = this@OkHttpEngineConfig.connectionIdlePollingInterval
9395
maxConcurrencyPerHost = this@OkHttpEngineConfig.maxConcurrencyPerHost
94-
trustManagerProvider = this@OkHttpEngineConfig.trustManagerProvider
95-
keyManagerProvider = this@OkHttpEngineConfig.keyManagerProvider
96+
trustManager = this@OkHttpEngineConfig.trustManager
97+
keyManager = this@OkHttpEngineConfig.keyManager
9698
cipherSuites = this@OkHttpEngineConfig.cipherSuites
9799
certificatePinner = this@OkHttpEngineConfig.certificatePinner
98100
hostnameVerifier = this@OkHttpEngineConfig.hostnameVerifier
@@ -125,18 +127,18 @@ public class OkHttpEngineConfig private constructor(builder: Builder) : HttpClie
125127
public var maxConcurrencyPerHost: UInt? = null
126128

127129
/**
128-
* Provider for TLS trust managers used to validate server certificates during TLS handshake.
129-
* Trust managers determine whether to trust the certificate chain presented by a remote server.
130-
* When provided, these trust managers will be used instead of the default system trust store.
130+
* Trust manager used to validate server certificates during TLS handshake.
131+
* Determines whether to trust the certificate chain presented by a remote server.
132+
* When provided, this trust manager will be used instead of the default system trust store.
131133
*/
132-
public var trustManagerProvider: TlsTrustManagersProvider? = null
134+
public var trustManager: X509TrustManager? = null
133135

134136
/**
135-
* Provider for KeyManagers that supply client certificates for mutual TLS (mTLS) authentication.
136-
* When provided, the client will present certificates from these KeyManagers when the server
137+
* Key manager that supplies client certificates for mutual TLS (mTLS) authentication.
138+
* When provided, the client will present certificates from this key manager when the server
137139
* requests client authentication. Used for scenarios requiring client certificate authentication.
138140
*/
139-
public var keyManagerProvider: TlsKeyManagersProvider? = null
141+
public var keyManager: KeyManager? = null
140142

141143
/**
142144
* List of cipher suites to enable for TLS connections. If null, uses OkHttp defaults.

‎runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/TlsKeyManagersProvider.kt‎

Lines changed: 0 additions & 20 deletions
This file was deleted.

‎runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/TlsTrustManagersProvider.kt‎

Lines changed: 0 additions & 20 deletions
This file was deleted.

‎runtime/protocol/http-client-engines/test-suite/jvm/test/aws/smithy/kotlin/runtime/http/test/ConnectionTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ class ConnectionTest : AbstractEngineTest() {
108108
ServerType.TLS_1_2,
109109
TlsContext { minVersion = TlsVersion.TLS_1_2 },
110110
okHttpConfigBlock = {
111-
trustManagerProvider = createTestTrustManagerProvider(testCert)
111+
trustManager = createTestTrustManager(testCert)
112112
},
113113
)
114114
}
@@ -120,7 +120,7 @@ class ConnectionTest : AbstractEngineTest() {
120120
ServerType.TLS_1_3,
121121
TlsContext { minVersion = TlsVersion.TLS_1_3 },
122122
okHttpConfigBlock = {
123-
trustManagerProvider = createTestTrustManagerProvider(testCert)
123+
trustManager = createTestTrustManager(testCert)
124124
},
125125
)
126126
}

‎runtime/protocol/http-client-engines/test-suite/jvm/test/aws/smithy/kotlin/runtime/http/test/util/ConnectionTestUtils.kt‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
*/
66
package aws.smithy.kotlin.runtime.http.test.util
77

8-
import aws.smithy.kotlin.runtime.http.engine.okhttp.TlsTrustManagersProvider
98
import aws.smithy.kotlin.runtime.net.toUrlString
109
import okhttp3.CertificatePinner
1110
import org.bouncycastle.asn1.x500.X500Name
@@ -16,8 +15,8 @@ import java.nio.file.Paths
1615
import java.security.KeyPairGenerator
1716
import java.security.cert.X509Certificate
1817
import java.util.*
19-
import javax.net.ssl.TrustManager
2018
import javax.net.ssl.TrustManagerFactory
19+
import javax.net.ssl.X509TrustManager
2120

2221
internal val testSslConfig by lazy {
2322
val sslConfigPath = System.getProperty("SSL_CONFIG_PATH")
@@ -28,18 +27,15 @@ internal val testCert by lazy {
2827
testSslConfig.keyStore.getCertificate(testSslConfig.certificateAlias) as X509Certificate
2928
}
3029

31-
fun createTestTrustManagerProvider(testCert: X509Certificate): TlsTrustManagersProvider =
32-
object : TlsTrustManagersProvider {
33-
override fun trustManagers(): Array<TrustManager> {
34-
val keyStore = java.security.KeyStore.getInstance(java.security.KeyStore.getDefaultType())
35-
keyStore.load(null, null)
36-
keyStore.setCertificateEntry("test-cert", testCert)
30+
fun createTestTrustManager(testCert: X509Certificate): X509TrustManager {
31+
val keyStore = java.security.KeyStore.getInstance(java.security.KeyStore.getDefaultType())
32+
keyStore.load(null, null)
33+
keyStore.setCertificateEntry("test-cert", testCert)
3734

38-
val trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm())
39-
trustManagerFactory.init(keyStore)
40-
return trustManagerFactory.trustManagers
41-
}
42-
}
35+
val trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm())
36+
trustManagerFactory.init(keyStore)
37+
return trustManagerFactory.trustManagers[0] as X509TrustManager
38+
}
4339

4440
fun createTestCertificatePinner(testCert: X509Certificate, serverType: ServerType): CertificatePinner {
4541
val sha256Hash = java.security.MessageDigest.getInstance("SHA-256").digest(testCert.publicKey.encoded)

0 commit comments

Comments
 (0)