Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/url_launcher/url_launcher_android/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## NEXT

* Supports `intent://` URIs in `launchUrl` and `canLaunchUrl` by parsing them
with `Intent.parseUri`, preserving the action, data URI, and package encoded
in the URI rather than silently replacing the action with `ACTION_VIEW`.

## 6.3.32

* Bumps the androidx group across 10 directories with 1 update.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import androidx.annotation.VisibleForTesting;
import androidx.browser.customtabs.CustomTabsClient;
import androidx.browser.customtabs.CustomTabsIntent;
import java.net.URISyntaxException;
import java.util.Collections;
import java.util.Locale;
import java.util.Map;
Expand Down Expand Up @@ -64,8 +65,17 @@ void setActivity(@Nullable Activity activity) {

@Override
public boolean canLaunchUrl(@NonNull String url) {
Intent launchIntent = new Intent(Intent.ACTION_VIEW);
launchIntent.setData(Uri.parse(url));
Intent launchIntent;
if (url.startsWith("intent://")) {
try {
launchIntent = Intent.parseUri(url, Intent.URI_INTENT_SCHEME);
} catch (URISyntaxException e) {
return false;
}
} else {
launchIntent = new Intent(Intent.ACTION_VIEW);
launchIntent.setData(Uri.parse(url));
}
Comment on lines +68 to +78

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using url.startsWith("intent://") only matches URIs starting with the exact lowercase prefix and containing //. However, Android intent URIs can start with intent: (without //) and are case-insensitive per RFC 3986. Using url.regionMatches(true, 0, "intent:", 0, 7) is more robust and correctly supports all valid intent URIs.

Suggested change
Intent launchIntent;
if (url.startsWith("intent://")) {
try {
launchIntent = Intent.parseUri(url, Intent.URI_INTENT_SCHEME);
} catch (URISyntaxException e) {
return false;
}
} else {
launchIntent = new Intent(Intent.ACTION_VIEW);
launchIntent.setData(Uri.parse(url));
}
Intent launchIntent;
if (url.regionMatches(true, 0, "intent:", 0, 7)) {
try {
launchIntent = Intent.parseUri(url, Intent.URI_INTENT_SCHEME);
} catch (URISyntaxException e) {
return false;
}
} else {
launchIntent = new Intent(Intent.ACTION_VIEW);
launchIntent.setData(Uri.parse(url));
}

String componentName = intentResolver.getHandlerComponentName(launchIntent);
if (BuildConfig.DEBUG) {
Log.i(TAG, "component name for " + url + " is " + componentName);
Expand All @@ -84,10 +94,19 @@ public boolean launchUrl(
ensureActivity();
assert activity != null;

Intent launchIntent =
new Intent(Intent.ACTION_VIEW)
.setData(Uri.parse(url))
.putExtra(Browser.EXTRA_HEADERS, extractBundle(headers));
Intent launchIntent;
if (url.startsWith("intent://")) {
try {
launchIntent = Intent.parseUri(url, Intent.URI_INTENT_SCHEME);
} catch (URISyntaxException e) {
return false;
}
} else {
launchIntent =
new Intent(Intent.ACTION_VIEW)
.setData(Uri.parse(url))
.putExtra(Browser.EXTRA_HEADERS, extractBundle(headers));
}
Comment on lines +97 to +109

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This implementation has two issues:

  1. It only matches intent:// URIs (case-sensitively). It should support any valid intent: URI case-insensitively.
  2. It silently ignores the passed headers when launching an intent: URI. Even for intent URIs, we should preserve and attach the headers so that if the intent resolves to a browser or an app that handles HTTP headers, they are correctly forwarded.

We can fix both issues by using regionMatches and applying the headers to the resulting intent regardless of the scheme.

    Intent launchIntent;
    if (url.regionMatches(true, 0, "intent:", 0, 7)) {
      try {
        launchIntent = Intent.parseUri(url, Intent.URI_INTENT_SCHEME);
      } catch (URISyntaxException e) {
        return false;
      }
    } else {
      launchIntent = new Intent(Intent.ACTION_VIEW).setData(Uri.parse(url));
    }
    launchIntent.putExtra(Browser.EXTRA_HEADERS, extractBundle(headers));

if (requireNonBrowser && Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
launchIntent.addFlags(Intent.FLAG_ACTIVITY_REQUIRE_NON_BROWSER);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,35 @@ public void canLaunch_returnsFalse() {
assertFalse(result);
}

@Test
public void canLaunch_parsesIntentSchemeUri() {
UrlLauncher.IntentResolver resolver = mock(UrlLauncher.IntentResolver.class);
UrlLauncher api = new UrlLauncher(ApplicationProvider.getApplicationContext(), resolver);
String intentUrl =
"intent://details?id=com.example.app"
+ "#Intent;scheme=bazaar;package=com.example.store;"
+ "action=android.intent.action.EDIT;end";
when(resolver.getHandlerComponentName(any())).thenReturn(null);

api.canLaunchUrl(intentUrl);

final ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class);
verify(resolver).getHandlerComponentName(intentCaptor.capture());
assertEquals(Intent.ACTION_EDIT, intentCaptor.getValue().getAction());
assertEquals(
Uri.parse("bazaar://details?id=com.example.app"), intentCaptor.getValue().getData());
assertEquals("com.example.store", intentCaptor.getValue().getPackage());
}

@Test
public void canLaunch_returnsFalseForMalformedIntentSchemeUri() {
UrlLauncher api = new UrlLauncher(ApplicationProvider.getApplicationContext(), intent -> null);

boolean result = api.canLaunchUrl("intent://[malformed");

assertFalse(result);
}

// Integration testing on emulators won't work as expected without the workaround this tests
// for, since it will be returned even for intentionally bogus schemes.
@Test
Expand Down Expand Up @@ -147,6 +176,38 @@ public void launch_returnsTrue() {
assertTrue(result);
}

@Test
public void launch_parsesIntentSchemeUri() {
Activity activity = mock(Activity.class);
UrlLauncher api = new UrlLauncher(ApplicationProvider.getApplicationContext());
api.setActivity(activity);
String intentUrl =
"intent://details?id=com.example.app"
+ "#Intent;scheme=bazaar;package=com.example.store;"
+ "action=android.intent.action.EDIT;end";
doThrow(new ActivityNotFoundException()).when(activity).startActivity(any());

api.launchUrl(intentUrl, new HashMap<>(), false);

final ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class);
verify(activity).startActivity(intentCaptor.capture());
assertEquals(Intent.ACTION_EDIT, intentCaptor.getValue().getAction());
assertEquals(
Uri.parse("bazaar://details?id=com.example.app"), intentCaptor.getValue().getData());
assertEquals("com.example.store", intentCaptor.getValue().getPackage());
}

@Test
public void launch_returnsFalseForMalformedIntentSchemeUri() {
Activity activity = mock(Activity.class);
UrlLauncher api = new UrlLauncher(ApplicationProvider.getApplicationContext());
api.setActivity(activity);

boolean result = api.launchUrl("intent://[malformed", new HashMap<>(), false);

assertFalse(result);
}

@Test
public void openUrlInApp_opensUrlInWebViewIfNecessary() {
Activity activity = mock(Activity.class);
Expand Down