Skip to content

Commit 372dbac

Browse files
aitorvsjoshliebe
andauthored
Fix incorrect handling of chrome://... and/or edge://... type of URLs (#6760)
Task/Issue URL: https://app.asana.com/1/137249556945/project/488551667048375/task/1211244094852892?focus=true ### Description Fix the way that we handle non http URI requests. The first attempts fixed it but also introduced a bug because we should only look at user initiated requests, and not redirects. This fix should do that ### Steps to test this PR - [ ] repeat tests in #6687 - [ ] ensure the [side effect issue](https://app.asana.com/1/137249556945/project/1206777341262243/task/1211202801652410) doesn't repro anymore --------- Co-authored-by: joshliebe <[email protected]>
1 parent af9157b commit 372dbac

File tree

3 files changed

+127
-22
lines changed

3 files changed

+127
-22
lines changed

app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,9 @@ class SpecialUrlDetectorImpl(
8181
} else {
8282
URI_ANDROID_APP_SCHEME
8383
}
84-
checkForIntent(scheme, uriString, intentFlags)
84+
// if there's no redirects (initiatingUrl = null) then it's user initiated
85+
val userInitiated = initiatingUrl == null
86+
checkForIntent(scheme, uriString, intentFlags, userInitiated)
8587
}
8688
}
8789
}
@@ -183,31 +185,38 @@ class SpecialUrlDetectorImpl(
183185
scheme: String,
184186
uriString: String,
185187
intentFlags: Int,
188+
userInitiated: Boolean,
186189
): UrlType {
187-
val validUriSchemeRegex = Regex("[a-z][a-zA-Z\\d+.-]+")
188-
if (scheme.matches(validUriSchemeRegex)) {
189-
return buildIntent(uriString, intentFlags)
190-
}
191-
192-
return UrlType.SearchQuery(uriString)
193-
}
190+
fun buildIntent(uriString: String, intentFlags: Int, userInitiated: Boolean): UrlType {
191+
return try {
192+
val intent = Intent.parseUri(uriString, intentFlags)
193+
// only proceed if something can handle it
194+
if (userInitiated && (intent == null || packageManager.resolveActivity(intent, 0) == null) &&
195+
androidBrowserConfigFeature.validateIntentResolution().isEnabled()
196+
) {
197+
return UrlType.Unknown(uriString)
198+
}
194199

195-
private fun buildIntent(uriString: String, intentFlags: Int): UrlType {
196-
return try {
197-
val intent = Intent.parseUri(uriString, intentFlags)
200+
if (externalAppIntentFlagsFeature.self().isEnabled()) {
201+
intent.addCategory(Intent.CATEGORY_BROWSABLE)
202+
intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP)
203+
}
198204

199-
if (externalAppIntentFlagsFeature.self().isEnabled()) {
200-
intent.addCategory(Intent.CATEGORY_BROWSABLE)
201-
intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP)
205+
val fallbackUrl = intent.getStringExtra(EXTRA_FALLBACK_URL)
206+
val fallbackIntent = buildFallbackIntent(fallbackUrl)
207+
UrlType.NonHttpAppLink(uriString = uriString, intent = intent, fallbackUrl = fallbackUrl, fallbackIntent = fallbackIntent)
208+
} catch (e: URISyntaxException) {
209+
logcat(WARN) { "Failed to parse uri $uriString: ${e.asLog()}" }
210+
return UrlType.Unknown(uriString)
202211
}
212+
}
203213

204-
val fallbackUrl = intent.getStringExtra(EXTRA_FALLBACK_URL)
205-
val fallbackIntent = buildFallbackIntent(fallbackUrl)
206-
UrlType.NonHttpAppLink(uriString = uriString, intent = intent, fallbackUrl = fallbackUrl, fallbackIntent = fallbackIntent)
207-
} catch (e: URISyntaxException) {
208-
logcat(WARN) { "Failed to parse uri $uriString: ${e.asLog()}" }
209-
return UrlType.Unknown(uriString)
214+
val validUriSchemeRegex = Regex("[a-z][a-zA-Z\\d+.-]+")
215+
if (scheme.matches(validUriSchemeRegex)) {
216+
return buildIntent(uriString, intentFlags, userInitiated)
210217
}
218+
219+
return UrlType.SearchQuery(uriString)
211220
}
212221

213222
private fun buildFallbackIntent(fallbackUrl: String?): Intent? {

app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,4 +161,13 @@ interface AndroidBrowserConfigFeature {
161161

162162
@Toggle.DefaultValue(DefaultFeatureValue.TRUE)
163163
fun hideDuckAiInSerpKillSwitch(): Toggle
164+
165+
/**
166+
* Kill switch for intent resolution validation in SpecialUrlDetector
167+
* @return `true` when the remote config has the global "validateIntentResolution" androidBrowserConfig
168+
* sub-feature flag enabled
169+
* If the remote feature is not present defaults to `true`
170+
*/
171+
@Toggle.DefaultValue(DefaultFeatureValue.TRUE)
172+
fun validateIntentResolution(): Toggle
164173
}

app/src/test/java/com/duckduckgo/app/browser/SpecialUrlDetectorImplTest.kt

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ class SpecialUrlDetectorImplTest {
9999
whenever(mockAIChatQueryDetectionFeatureToggle.isEnabled()).thenReturn(false)
100100
whenever(mockAIChatQueryDetectionFeature.self()).thenReturn(mockAIChatQueryDetectionFeatureToggle)
101101
androidBrowserConfigFeature.handleIntentScheme().setRawStoredState(State(true))
102+
androidBrowserConfigFeature.validateIntentResolution().setRawStoredState(State(true))
102103
}
103104

104105
@Test
@@ -293,21 +294,64 @@ class SpecialUrlDetectorImplTest {
293294
@Test
294295
fun whenUrlIsCustomUriSchemeThenNonHttpAppLinkTypeDetectedWithAdditionalIntentFlags() {
295296
externalAppIntentFlagsFeature.self().setRawStoredState(State(true))
297+
whenever(mockPackageManager.resolveActivity(any(), anyInt())).thenReturn(ResolveInfo())
296298
val type = testee.determineType("myapp:foo bar") as NonHttpAppLink
297299
assertEquals("myapp:foo bar", type.uriString)
298300
assertEquals(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP, type.intent.flags)
299301
assertEquals(Intent.CATEGORY_BROWSABLE, type.intent.categories.first())
300302
}
301303

304+
@Test
305+
fun whenUrlIsCustomUriSchemeAndRedirectThenNonHttpAppLinkTypeDetectedWithAdditionalIntentFlags() {
306+
externalAppIntentFlagsFeature.self().setRawStoredState(State(true))
307+
whenever(mockPackageManager.resolveActivity(any(), anyInt())).thenReturn(ResolveInfo())
308+
val actual = testee.determineType("https://www.example.com", "myapp:foo bar".toUri()) as NonHttpAppLink
309+
assertEquals("myapp:foo bar", actual.uriString)
310+
assertEquals(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP, actual.intent.flags)
311+
assertEquals(Intent.CATEGORY_BROWSABLE, actual.intent.categories.first())
312+
}
313+
314+
@Test
315+
fun whenUrlIsCustomUriSchemeAndNoResolveInfoThenUnknownTypeDetected() {
316+
externalAppIntentFlagsFeature.self().setRawStoredState(State(true))
317+
whenever(mockPackageManager.resolveActivity(any(), anyInt())).thenReturn(null)
318+
val expected = Unknown::class
319+
val actual = testee.determineType("myapp:foo bar")
320+
assertEquals(expected, actual::class)
321+
}
322+
323+
@Test
324+
fun whenUrlIsCustomUriSchemeAndNoResolveInfoAndRedirectThenNonHttpAppLinkDetected() {
325+
externalAppIntentFlagsFeature.self().setRawStoredState(State(true))
326+
whenever(mockPackageManager.resolveActivity(any(), anyInt())).thenReturn(null)
327+
val expected = NonHttpAppLink::class
328+
val actual = testee.determineType("https://www.example.com", "myapp:foo bar".toUri()) as NonHttpAppLink
329+
assertEquals(expected, actual::class)
330+
assertEquals("myapp:foo bar", actual.uriString)
331+
assertEquals(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP, actual.intent.flags)
332+
assertEquals(Intent.CATEGORY_BROWSABLE, actual.intent.categories.first())
333+
}
334+
302335
@Test
303336
fun whenUrlIsCustomUriSchemeThenNonHttpAppLinkTypeDetectedWithoutAdditionalIntentFlags() {
304337
externalAppIntentFlagsFeature.self().setRawStoredState(State(false))
338+
whenever(mockPackageManager.resolveActivity(any(), anyInt())).thenReturn(ResolveInfo())
305339
val type = testee.determineType("myapp:foo bar") as NonHttpAppLink
306340
assertEquals("myapp:foo bar", type.uriString)
307341
assertEquals(0, type.intent.flags)
308342
assertNull(type.intent.categories)
309343
}
310344

345+
@Test
346+
fun whenUrlIsCustomUriSchemeAndRedirectThenNonHttpAppLinkTypeDetectedWithoutAdditionalIntentFlags() {
347+
externalAppIntentFlagsFeature.self().setRawStoredState(State(false))
348+
whenever(mockPackageManager.resolveActivity(any(), anyInt())).thenReturn(ResolveInfo())
349+
val actual = testee.determineType("https://www.example.com", "myapp:foo bar".toUri()) as NonHttpAppLink
350+
assertEquals("myapp:foo bar", actual.uriString)
351+
assertEquals(0, actual.intent.flags)
352+
assertNull(actual.intent.categories)
353+
}
354+
311355
@Test
312356
fun whenUrlIsNotPrivacyProThenQueryTypeDetected() {
313357
whenever(subscriptions.shouldLaunchPrivacyProForUrl(any())).thenReturn(false)
@@ -520,7 +564,7 @@ class SpecialUrlDetectorImplTest {
520564

521565
testee.determineType("intent://path#Intent;scheme=testscheme;package=com.example.app;end")
522566

523-
verify(testee).checkForIntent(eq("intent"), any(), eq(URI_INTENT_SCHEME))
567+
verify(testee).checkForIntent(eq("intent"), any(), eq(URI_INTENT_SCHEME), eq(true))
524568
}
525569

526570
@Test
@@ -529,7 +573,50 @@ class SpecialUrlDetectorImplTest {
529573

530574
testee.determineType("intent://path#Intent;scheme=testscheme;package=com.example.app;end")
531575

532-
verify(testee).checkForIntent(eq("intent"), any(), eq(URI_ANDROID_APP_SCHEME))
576+
verify(testee).checkForIntent(eq("intent"), any(), eq(URI_ANDROID_APP_SCHEME), eq(true))
577+
}
578+
579+
@Test
580+
fun whenValidateIntentResolutionEnabledAndNoResolveInfoThenReturnUnknownType() {
581+
androidBrowserConfigFeature.validateIntentResolution().setRawStoredState(State(true))
582+
whenever(mockPackageManager.resolveActivity(any(), anyInt())).thenReturn(null)
583+
584+
val result = testee.determineType("myapp:foo bar")
585+
586+
assertTrue(result is Unknown)
587+
}
588+
589+
@Test
590+
fun whenValidateIntentResolutionDisabledAndNoResolveInfoThenReturnNonHttpAppLinkType() {
591+
androidBrowserConfigFeature.validateIntentResolution().setRawStoredState(State(false))
592+
whenever(mockPackageManager.resolveActivity(any(), anyInt())).thenReturn(null)
593+
594+
val result = testee.determineType("myapp:foo bar")
595+
596+
assertTrue(result is NonHttpAppLink)
597+
assertEquals("myapp:foo bar", (result as NonHttpAppLink).uriString)
598+
}
599+
600+
@Test
601+
fun whenValidateIntentResolutionEnabledAndResolveInfoExistsThenReturnNonHttpAppLinkType() {
602+
androidBrowserConfigFeature.validateIntentResolution().setRawStoredState(State(true))
603+
whenever(mockPackageManager.resolveActivity(any(), anyInt())).thenReturn(ResolveInfo())
604+
605+
val result = testee.determineType("myapp:foo bar")
606+
607+
assertTrue(result is NonHttpAppLink)
608+
assertEquals("myapp:foo bar", (result as NonHttpAppLink).uriString)
609+
}
610+
611+
@Test
612+
fun whenValidateIntentResolutionDisabledAndResolveInfoExistsThenReturnNonHttpAppLinkType() {
613+
androidBrowserConfigFeature.validateIntentResolution().setRawStoredState(State(false))
614+
whenever(mockPackageManager.resolveActivity(any(), anyInt())).thenReturn(ResolveInfo())
615+
616+
val result = testee.determineType("myapp:foo bar")
617+
618+
assertTrue(result is NonHttpAppLink)
619+
assertEquals("myapp:foo bar", (result as NonHttpAppLink).uriString)
533620
}
534621

535622
private fun randomString(length: Int): String {

0 commit comments

Comments
 (0)