Don't use unqualified generic types for validator

CrisBarreiro · CrisBarreiro · commit 977a57413a39 · 2026-05-19T17:07:03.000+02:00
diff --git a/ad-blocking/ad-blocking-impl/src/main/java/com/duckduckgo/adblocking/impl/di/AdBlockingExtensionModule.kt b/ad-blocking/ad-blocking-impl/src/main/java/com/duckduckgo/adblocking/impl/di/AdBlockingExtensionModule.kt
@@ -16,7 +16,6 @@
 
 package com.duckduckgo.adblocking.impl.di
 
-import com.duckduckgo.adblocking.impl.domain.PublicKeyProvider
 import com.duckduckgo.adblocking.impl.remoteconfig.AdBlockingExtensionSettings
 import com.duckduckgo.adblocking.impl.remoteconfig.DomainJsonAdapter
 import com.duckduckgo.adblocking.impl.remoteconfig.ScriptletsSettings
@@ -34,12 +33,6 @@ import com.squareup.moshi.kotlin.reflect.KotlinJsonAdapterFactory
 import dagger.Module
 import dagger.Provides
 import dagger.SingleInstanceIn
-import java.nio.charset.CharsetDecoder
-import java.nio.charset.StandardCharsets
-import java.security.KeyFactory
-import java.security.PublicKey
-import java.security.Signature
-import java.util.Base64
 
 @Module
 @ContributesTo(AppScope::class)
@@ -62,29 +55,6 @@ object AdBlockingExtensionModule {
     fun provideAdBlockingExtensionDao(database: AdBlockingExtensionDatabase): AdBlockingExtensionDao =
         database.adBlockingExtensionDao()
 
-    @SingleInstanceIn(AppScope::class)
-    @Provides
-    fun provideBase64Decoder(): Base64.Decoder =
-        Base64.getDecoder()
-
-    @SingleInstanceIn(AppScope::class)
-    @Provides
-    fun provideKeyFactory(): KeyFactory =
-        KeyFactory.getInstance("EC")
-
-    @SingleInstanceIn(AppScope::class)
-    @Provides
-    fun provideAdBlockingExtensionPublicKey(publicKeyProvider: PublicKeyProvider): PublicKey =
-        publicKeyProvider.publicKey
-
-    @Provides
-    fun provideUTF8CharsetDecoder(): CharsetDecoder =
-        StandardCharsets.UTF_8.newDecoder()
-
-    @Provides
-    fun provideSignature(): Signature =
-        Signature.getInstance("SHA256withECDSA")
-
     @SingleInstanceIn(AppScope::class)
     @Provides
     fun provideAdBlockingExtensionSettingsAdapter(): JsonAdapter<AdBlockingExtensionSettings> =
diff --git a/ad-blocking/ad-blocking-impl/src/main/java/com/duckduckgo/adblocking/impl/domain/PublicKeyProvider.kt b/ad-blocking/ad-blocking-impl/src/main/java/com/duckduckgo/adblocking/impl/domain/PublicKeyProvider.kt
@@ -33,12 +33,10 @@ private const val KEY = "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEuqrhn2ztPnPt8QybIGs
 
 @ContributesBinding(AppScope::class)
 @SingleInstanceIn(AppScope::class)
-class RealPublicKeyProvider @Inject constructor(
-    private val keyFactory: KeyFactory,
-    private val decoder: Base64.Decoder,
-) : PublicKeyProvider {
+class RealPublicKeyProvider @Inject constructor() : PublicKeyProvider {
 
     override val publicKey: PublicKey by lazy {
-        keyFactory.generatePublic(X509EncodedKeySpec(decoder.decode(KEY)))
+        KeyFactory.getInstance("EC")
+            .generatePublic(X509EncodedKeySpec(Base64.getDecoder().decode(KEY)))
     }
 }
diff --git a/ad-blocking/ad-blocking-impl/src/main/java/com/duckduckgo/adblocking/impl/domain/ScriptletSignatureValidator.kt b/ad-blocking/ad-blocking-impl/src/main/java/com/duckduckgo/adblocking/impl/domain/ScriptletSignatureValidator.kt
@@ -19,14 +19,12 @@ package com.duckduckgo.adblocking.impl.domain
 import com.duckduckgo.di.scopes.AppScope
 import com.squareup.anvil.annotations.ContributesBinding
 import dagger.SingleInstanceIn
-import kotlinx.coroutines.sync.Mutex
-import kotlinx.coroutines.sync.withLock
 import logcat.LogPriority.WARN
 import logcat.asLog
 import logcat.logcat
 import java.nio.ByteBuffer
 import java.nio.charset.CharacterCodingException
-import java.nio.charset.CharsetDecoder
+import java.nio.charset.StandardCharsets
 import java.security.PublicKey
 import java.security.Signature
 import java.util.Base64
@@ -42,35 +40,32 @@ sealed interface ScriptletValidationResult {
 }
 
 interface ScriptletSignatureValidator {
-    suspend fun validate(content: ByteArray, signatureBase64: String): ScriptletValidationResult
+    fun validate(content: ByteArray, signatureBase64: String): ScriptletValidationResult
 }
 
 @ContributesBinding(AppScope::class)
 @SingleInstanceIn(AppScope::class)
 class RealScriptletSignatureValidator @Inject constructor(
-    private val publicKey: PublicKey,
-    private val signature: Signature,
-    private val charsetDecoder: CharsetDecoder,
-    private val base64Decoder: Base64.Decoder,
+    publicKeyProvider: PublicKeyProvider,
 ) : ScriptletSignatureValidator {
 
-    private val mutex = Mutex()
+    private val publicKey: PublicKey = publicKeyProvider.publicKey
 
-    override suspend fun validate(content: ByteArray, signatureBase64: String): ScriptletValidationResult = mutex.withLock {
+    override fun validate(content: ByteArray, signatureBase64: String): ScriptletValidationResult {
         try {
-            charsetDecoder.decode(ByteBuffer.wrap(content))
+            StandardCharsets.UTF_8.newDecoder().decode(ByteBuffer.wrap(content))
         } catch (e: CharacterCodingException) {
             logcat(WARN) { "Validating scriptlet failed with invalid encoding: ${e.message}" }
-            return@withLock ScriptletValidationResult.Invalid.Encoding
+            return ScriptletValidationResult.Invalid.Encoding
         }
         val signatureBytes = try {
-            base64Decoder.decode(signatureBase64)
+            Base64.getDecoder().decode(signatureBase64)
         } catch (e: IllegalArgumentException) {
             logcat(WARN) { "Validating scriptlet failed with invalid signature format: ${e.message}" }
-            return@withLock ScriptletValidationResult.Invalid.SignatureFormat
+            return ScriptletValidationResult.Invalid.SignatureFormat
         }
-        try {
-            val verified = signature.run {
+        return try {
+            val verified = Signature.getInstance(SIGNATURE_ALGORITHM).run {
                 initVerify(publicKey)
                 update(content)
                 verify(signatureBytes)
@@ -87,4 +82,8 @@ class RealScriptletSignatureValidator @Inject constructor(
             ScriptletValidationResult.Invalid.SignatureVerificationFailed
         }
     }
+
+    companion object {
+        const val SIGNATURE_ALGORITHM = "SHA256withECDSA"
+    }
 }
diff --git a/ad-blocking/ad-blocking-impl/src/test/java/com/duckduckgo/adblocking/impl/domain/ScriptletSignatureValidatorTest.kt b/ad-blocking/ad-blocking-impl/src/test/java/com/duckduckgo/adblocking/impl/domain/ScriptletSignatureValidatorTest.kt
@@ -17,15 +17,14 @@
 package com.duckduckgo.adblocking.impl.domain
 
 import androidx.test.ext.junit.runners.AndroidJUnit4
-import kotlinx.coroutines.test.runTest
 import org.junit.Assert
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
-import java.nio.charset.StandardCharsets
 import java.security.KeyPair
 import java.security.KeyPairGenerator
 import java.security.PrivateKey
+import java.security.PublicKey
 import java.security.Signature
 import java.security.spec.ECGenParameterSpec
 import java.util.Base64
@@ -42,23 +41,22 @@ class ScriptletSignatureValidatorTest {
             initialize(ECGenParameterSpec("secp256r1"))
         }.generateKeyPair()
         validator = RealScriptletSignatureValidator(
-            publicKey = keyPair.public,
-            signature = Signature.getInstance("SHA256withECDSA"),
-            charsetDecoder = StandardCharsets.UTF_8.newDecoder(),
-            base64Decoder = Base64.getDecoder(),
+            publicKeyProvider = object : PublicKeyProvider {
+                override val publicKey: PublicKey = keyPair.public
+            },
         )
     }
 
     @Test
-    fun whenContentAndSignatureMatchThenValidateReturnsValid() = runTest {
+    fun whenContentAndSignatureMatchThenValidateReturnsValid() {
         val content = "console.log('hello')".toByteArray()
         val signature = sign(content, keyPair.private)
 
         Assert.assertEquals(ScriptletValidationResult.Valid, validator.validate(content, signature))
     }
 
     @Test
-    fun whenContentIsTamperedThenValidateReturnsSignatureVerificationFailed() = runTest {
+    fun whenContentIsTamperedThenValidateReturnsSignatureVerificationFailed() {
         val original = "original content".toByteArray()
         val signature = sign(original, keyPair.private)
         val tampered = "tampered content".toByteArray()
@@ -70,22 +68,21 @@ class ScriptletSignatureValidatorTest {
     }
 
     @Test
-    fun whenSignatureIsFromDifferentKeyThenValidateReturnsSignatureVerificationFailed() =
-        runTest {
-            val content = "content".toByteArray()
-            val otherKeyPair = KeyPairGenerator.getInstance("EC").apply {
-                initialize(ECGenParameterSpec("secp256r1"))
-            }.generateKeyPair()
-            val signature = sign(content, otherKeyPair.private)
-
-            Assert.assertEquals(
-                ScriptletValidationResult.Invalid.SignatureVerificationFailed,
-                validator.validate(content, signature),
-            )
-        }
+    fun whenSignatureIsFromDifferentKeyThenValidateReturnsSignatureVerificationFailed() {
+        val content = "content".toByteArray()
+        val otherKeyPair = KeyPairGenerator.getInstance("EC").apply {
+            initialize(ECGenParameterSpec("secp256r1"))
+        }.generateKeyPair()
+        val signature = sign(content, otherKeyPair.private)
+
+        Assert.assertEquals(
+            ScriptletValidationResult.Invalid.SignatureVerificationFailed,
+            validator.validate(content, signature),
+        )
+    }
 
     @Test
-    fun whenSignatureIsMalformedBase64ThenValidateReturnsInvalidSignatureFormat() = runTest {
+    fun whenSignatureIsMalformedBase64ThenValidateReturnsInvalidSignatureFormat() {
         val content = "content".toByteArray()
 
         Assert.assertEquals(
@@ -95,19 +92,18 @@ class ScriptletSignatureValidatorTest {
     }
 
     @Test
-    fun whenSignatureBytesAreNotValidEcdsaSignatureThenValidateReturnsSignatureVerificationFailed() =
-        runTest {
-            val content = "content".toByteArray()
-            val garbage = Base64.getEncoder().encodeToString(ByteArray(64) { 0x42 })
-
-            Assert.assertEquals(
-                ScriptletValidationResult.Invalid.SignatureVerificationFailed,
-                validator.validate(content, garbage),
-            )
-        }
+    fun whenSignatureBytesAreNotValidEcdsaSignatureThenValidateReturnsSignatureVerificationFailed() {
+        val content = "content".toByteArray()
+        val garbage = Base64.getEncoder().encodeToString(ByteArray(64) { 0x42 })
+
+        Assert.assertEquals(
+            ScriptletValidationResult.Invalid.SignatureVerificationFailed,
+            validator.validate(content, garbage),
+        )
+    }
 
     @Test
-    fun whenContentIsNotValidUtf8ThenValidateReturnsInvalidEncoding() = runTest {
+    fun whenContentIsNotValidUtf8ThenValidateReturnsInvalidEncoding() {
         val invalidUtf8 = byteArrayOf(0xFF.toByte(), 0xFE.toByte(), 0x80.toByte(), 0x81.toByte())
         val signature = sign(invalidUtf8, keyPair.private)
 

Original file line number	Diff line number	Diff line change
`@@ -33,12 +33,10 @@ private const val KEY = "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEuqrhn2ztPnPt8QybIGs`
`33`	`33`
`34`	`34`	`@ContributesBinding(AppScope::class)`
`35`	`35`	`@SingleInstanceIn(AppScope::class)`
`36`		`-class RealPublicKeyProvider @Inject constructor(`
`37`		`- private val keyFactory: KeyFactory,`
`38`		`- private val decoder: Base64.Decoder,`
`39`		`-) : PublicKeyProvider {`
	`36`	`+class RealPublicKeyProvider @Inject constructor() : PublicKeyProvider {`
`40`	`37`
`41`	`38`	`override val publicKey: PublicKey by lazy {`
`42`		`- keyFactory.generatePublic(X509EncodedKeySpec(decoder.decode(KEY)))`
	`39`	`+ KeyFactory.getInstance("EC")`
	`40`	`+ .generatePublic(X509EncodedKeySpec(Base64.getDecoder().decode(KEY)))`
`43`	`41`	`}`
`44`	`42`	`}`