diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index edafca37b598..3d9dc388a47f 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -199,7 +199,7 @@ class BrowserWebViewClientTest { whenever(currentTimeProvider.elapsedRealtime()).thenReturn(0) whenever(webViewVersionProvider.getMajorVersion()).thenReturn("1") whenever(deviceInfo.appVersion).thenReturn("1") - whenever(mockMaliciousSiteProtection.shouldOverrideUrlLoading(any(), any(), any(), any())).thenReturn(false) + whenever(mockMaliciousSiteProtection.shouldOverrideUrlLoading(any(), any(), any())).thenReturn(false) } @UiThreadTest diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index afe946f4ef0d..d2d7b5e4892d 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -165,14 +165,11 @@ class BrowserWebViewClient @Inject constructor( Timber.v("shouldOverride webViewUrl: ${webView.url} URL: $url") webViewClientListener?.onShouldOverride() - if (runBlocking { - maliciousSiteProtectionWebViewIntegration.shouldOverrideUrlLoading( - url, - webView.url?.toUri(), - isForMainFrame, - confirmationCallback, - ) - } + if (maliciousSiteProtectionWebViewIntegration.shouldOverrideUrlLoading( + url, + isForMainFrame, + confirmationCallback, + ) ) { // TODO (cbarreiro): Handle site blocked synchronously return true diff --git a/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt b/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt index e7b08b4edfab..ab00b5d78f2a 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt @@ -35,6 +35,7 @@ import java.net.URLDecoder import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import timber.log.Timber interface MaliciousSiteBlockerWebViewIntegration { @@ -45,9 +46,8 @@ interface MaliciousSiteBlockerWebViewIntegration { confirmationCallback: (isMalicious: Boolean) -> Unit, ): WebResourceResponse? - suspend fun shouldOverrideUrlLoading( + fun shouldOverrideUrlLoading( url: Uri, - webViewUrl: Uri?, isForMainFrame: Boolean, confirmationCallback: (isMalicious: Boolean) -> Unit, ): Boolean @@ -123,30 +123,32 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor( return null } - override suspend fun shouldOverrideUrlLoading( + override fun shouldOverrideUrlLoading( url: Uri, - webViewUrl: Uri?, isForMainFrame: Boolean, confirmationCallback: (isMalicious: Boolean) -> Unit, ): Boolean { - if (!isFeatureEnabled) { - return false - } - val decodedUrl = URLDecoder.decode(url.toString(), "UTF-8").lowercase() + return runBlocking { + if (!isFeatureEnabled) { + return@runBlocking false + } + val decodedUrl = URLDecoder.decode(url.toString(), "UTF-8").lowercase() - if (processedUrls.contains(decodedUrl)) { - processedUrls.remove(decodedUrl) - Timber.tag("PhishingAndMalwareDetector").d("Already intercepted, skipping $decodedUrl") - return false - } + if (processedUrls.contains(decodedUrl)) { + processedUrls.remove(decodedUrl) + Timber.tag("PhishingAndMalwareDetector").d("Already intercepted, skipping $decodedUrl") + return@runBlocking false + } - if (isForMainFrame && decodedUrl.toUri() == webViewUrl) { - if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), confirmationCallback) == MALICIOUS) { - return true + // iframes always go through the shouldIntercept method, so we only need to check the main frame here + if (isForMainFrame) { + if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), confirmationCallback) == MALICIOUS) { + return@runBlocking true + } + processedUrls.add(decodedUrl) } - processedUrls.add(decodedUrl) + false } - return false } private fun isForIframe(request: WebResourceRequest) = request.requestHeaders["Sec-Fetch-Dest"] == "iframe" || diff --git a/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt b/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt index b72120e1941f..b4141551c217 100644 --- a/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt +++ b/app/src/test/java/com/duckduckgo/app/browser/webview/RealMaliciousSiteBlockerWebViewIntegrationTest.kt @@ -49,7 +49,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { fun `shouldOverrideUrlLoading returns false when feature is disabled`() = runTest { updateFeatureEnabled(false) - val result = testee.shouldOverrideUrlLoading(exampleUri, null, true) {} + val result = testee.shouldOverrideUrlLoading(exampleUri, true) {} assertFalse(result) } @@ -67,7 +67,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { fun `shouldOverrideUrlLoading returns false when url is already processed`() = runTest { testee.processedUrls.add(exampleUri.toString()) - val result = testee.shouldOverrideUrlLoading(exampleUri, exampleUri, true) {} + val result = testee.shouldOverrideUrlLoading(exampleUri, true) {} assertFalse(result) } @@ -109,7 +109,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { fun `shouldOverride returns false when feature is enabled, is malicious, and is not mainframe`() = runTest { whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) - val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, false) {} + val result = testee.shouldOverrideUrlLoading(maliciousUri, false) {} assertFalse(result) } @@ -117,7 +117,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { fun `shouldOverride returns true when feature is enabled, is malicious, and is mainframe`() = runTest { whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) - val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, true) {} + val result = testee.shouldOverrideUrlLoading(maliciousUri, true) {} assertTrue(result) } @@ -125,15 +125,7 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest { fun `shouldOverride returns false when feature is enabled, is malicious, and not mainframe nor iframe`() = runTest { whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) - val result = testee.shouldOverrideUrlLoading(maliciousUri, maliciousUri, false) {} - assertFalse(result) - } - - @Test - fun `shouldOverride returns true when feature is enabled, is malicious, and is mainframe but webView has different host`() = runTest { - whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS) - - val result = testee.shouldOverrideUrlLoading(maliciousUri, exampleUri, true) {} + val result = testee.shouldOverrideUrlLoading(maliciousUri, false) {} assertFalse(result) }