Skip to content

GitLab

T
tangled
Commit 37b5b744 authored by Michael Thiessen's avatar Michael Thiessen Committed by Commit Bot
Browse files
Options

Don't launch external fallback URLs when not allowed to launch external URLs 

Ensure that we don't launch external fallback URLs, when we've otherwise
decided that we should not launch an external navigation (even if it was
possible to). Also prevent auto subframe navigation from launching
external fallback URLs.

Bug: 1094442
Change-Id: I3828a0957c160c6ed2c2a38b54cf54af9b797327
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2248124Reviewed-by: default avatarYaron Friedman <yfriedman@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779574}
parent 8d6bf7c7
Show whitespace changes
Inline Side-by-side
Showing with 199 additions and 63 deletions
chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/UrlOverridingTest.java
View file @ 37b5b744
......@@ -99,6 +99,8 @@ public class UrlOverridingTest {
BASE_PATH + "navigation_to_cct_via_intent_uri.html";
private static final String NAVIGATION_TO_FILE_SCHEME_FROM_INTENT_URI =
BASE_PATH + "navigation_to_file_scheme_via_intent_uri.html";
private static final String SUBFRAME_REDIRECT_WITH_PLAY_FALLBACK =
BASE_PATH + "subframe_navigation_with_play_fallback.html";
private static class TestTabObserver extends EmptyTabObserver {
private final CallbackHelper mFinishCallback;
......@@ -512,4 +514,12 @@ public class UrlOverridingTest {
String originalUrl = mTestServer.getURL(NAVIGATION_TO_FILE_SCHEME_FROM_INTENT_URI);
loadUrlAndWaitForIntentUrl(originalUrl, true, false, false, null, false, "empty_scheme");
}
@Test
@LargeTest
public void testSubframeLoadCannotLaunchPlayApp() throws TimeoutException {
mActivityTestRule.startMainActivityOnBlankPage();
loadUrlAndWaitForIntentUrl(
mTestServer.getURL(SUBFRAME_REDIRECT_WITH_PLAY_FALLBACK), false, false);
}
}
chrome/test/data/android/url_overriding/frame_redirect_with_play_fallback.html 0 → 100644
View file @ 37b5b744
<html>
<head>
<meta http-equiv="refresh" content="0;URL='intent://test/#Intent;scheme=test;package=not.real.package;S.browser_fallback_url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.android.chrome;end'" />
</head>
</html>
chrome/test/data/android/url_overriding/subframe_navigation_with_play_fallback.html 0 → 100644
View file @ 37b5b744
<!DOCTYPE html>
<html>
<body>
<iframe src="frame_redirect_with_play_fallback.html"></iframe>
</body>
</html>
components/external_intents/android/java/src/org/chromium/components/external_intents/ExternalNavigationHandler.java
View file @ 37b5b744
......@@ -160,6 +160,17 @@ public class ExternalNavigationHandler {
@VisibleForTesting
static final String INTENT_ACTION_HISTOGRAM = "Android.Intent.OverrideUrlLoadingIntentAction";
// Helper class to return a boolean by reference.
private static class MutableBoolean {
private Boolean mValue = null;
public void set(boolean value) {
mValue = value;
}
public Boolean get() {
return mValue;
}
}
private final ExternalNavigationDelegate mDelegate;
/**
......@@ -217,9 +228,16 @@ public class ExternalNavigationHandler {
&& !UrlUtilities.isValidForIntentFallbackNavigation(browserFallbackUrl)) {
browserFallbackUrl = null;
}
// TODO(https://crbug.com/1096099): Refactor shouldOverrideUrlLoadingInternal, splitting it
// up to separate out the notions wanting to fire an external intent vs being able to.
MutableBoolean canLaunchExternalFallbackResult = new MutableBoolean();
long time = SystemClock.elapsedRealtime();
@OverrideUrlLoadingResult
int result = shouldOverrideUrlLoadingInternal(params, targetIntent, browserFallbackUrl);
int result = shouldOverrideUrlLoadingInternal(
params, targetIntent, browserFallbackUrl, canLaunchExternalFallbackResult);
assert canLaunchExternalFallbackResult.get() != null;
RecordHistogram.recordTimesHistogram(
"Android.StrictMode.OverrideUrlLoadingTime", SystemClock.elapsedRealtime() - time);
......@@ -236,18 +254,24 @@ public class ExternalNavigationHandler {
&& (params.getRedirectHandler() == null
// For instance, if this is a chained fallback URL, we ignore it.
|| !params.getRedirectHandler().shouldNotOverrideUrlLoading())) {
result = handleFallbackUrl(params, targetIntent, browserFallbackUrl);
result = handleFallbackUrl(params, targetIntent, browserFallbackUrl,
canLaunchExternalFallbackResult.get());
}
if (DEBUG) printDebugShouldOverrideUrlLoadingResult(result);
return result;
}
private @OverrideUrlLoadingResult int handleFallbackUrl(
ExternalNavigationParams params, Intent targetIntent, String browserFallbackUrl) {
private @OverrideUrlLoadingResult int handleFallbackUrl(ExternalNavigationParams params,
Intent targetIntent, String browserFallbackUrl, boolean canLaunchExternalFallback) {
if (mDelegate.isIntentToInstantApp(targetIntent)) {
RecordHistogram.recordEnumeratedHistogram("Android.InstantApps.DirectInstantAppsIntent",
AiaIntent.FALLBACK_USED, AiaIntent.NUM_ENTRIES);
}
if (canLaunchExternalFallback) {
if (shouldBlockAllExternalAppLaunches(params) || params.isIncognito()) {
throw new SecurityException("Context is not allowed to launch an external app.");
}
// Launch WebAPK if it can handle the URL.
try {
Intent intent = Intent.parseUri(browserFallbackUrl, Intent.URI_INTENT_SCHEME);
......@@ -260,7 +284,34 @@ public class ExternalNavigationHandler {
} catch (Exception e) {
if (DEBUG) Log.i(TAG, "Could not parse fallback url as intent");
}
return clobberCurrentTabWithFallbackUrl(browserFallbackUrl, params);
// If the fallback URL is a link to Play Store, send the user to Play Store app
// instead: crbug.com/638672.
Pair<String, String> appInfo = maybeGetPlayStoreAppIdAndReferrer(browserFallbackUrl);
if (appInfo != null) {
String marketReferrer = TextUtils.isEmpty(appInfo.second)
? ContextUtils.getApplicationContext().getPackageName()
: appInfo.second;
return sendIntentToMarket(appInfo.first, marketReferrer, params);
}
}
// For subframes, we don't support fallback url for now.
// http://crbug.com/364522.
if (!params.isMainFrame()) {
if (DEBUG) Log.i(TAG, "Don't support fallback url in subframes");
return OverrideUrlLoadingResult.NO_OVERRIDE;
}
// NOTE: any further redirection from fall-back URL should not override URL loading.
// Otherwise, it can be used in chain for fingerprinting multiple app installation
// status in one shot. In order to prevent this scenario, we notify redirection
// handler that redirection from the current navigation should stay in this app.
if (params.getRedirectHandler() != null) {
params.getRedirectHandler().setShouldNotOverrideUrlLoadingOnCurrentRedirectChain();
}
if (DEBUG) Log.i(TAG, "clobberCurrentTab called");
return clobberCurrentTab(browserFallbackUrl, params.getReferrerUrl());
}
private void printDebugShouldOverrideUrlLoadingResult(int result) {
......@@ -299,6 +350,19 @@ public class ExternalNavigationHandler {
return true;
}
/**
* https://crbug.com/1094442: Don't allow any external navigation on AUTO_SUBFRAME navigation
* (eg. initial ad frame navigation).
*/
private boolean blockExternalNavFromAutoSubframe(ExternalNavigationParams params) {
int pageTransitionCore = params.getPageTransition() & PageTransition.CORE_MASK;
if (pageTransitionCore == PageTransition.AUTO_SUBFRAME) {
if (DEBUG) Log.i(TAG, "Auto navigation in subframe");
return true;
}
return false;
}
/**
* http://crbug.com/441284 : Disallow firing external intent while the app is in the background.
*/
......@@ -965,15 +1029,20 @@ public class ExternalNavigationHandler {
return false;
}
// Check if we're navigating under conditions that should never launch an external app.
private boolean shouldBlockAllExternalAppLaunches(ExternalNavigationParams params) {
return blockExternalNavFromAutoSubframe(params) || blockExternalNavWhileBackgrounded(params)
|| blockExternalNavFromBackgroundTab(params) || ignoreBackForwardNav(params);
}
private @OverrideUrlLoadingResult int shouldOverrideUrlLoadingInternal(
ExternalNavigationParams params, Intent targetIntent,
@Nullable String browserFallbackUrl) {
@Nullable String browserFallbackUrl, MutableBoolean canLaunchExternalFallbackResult) {
sanitizeQueryIntentActivitiesIntent(targetIntent);
// Don't allow external fallback URLs by default.
canLaunchExternalFallbackResult.set(false);
if (blockExternalNavWhileBackgrounded(params) || blockExternalNavFromBackgroundTab(params)
|| ignoreBackForwardNav(params)) {
return OverrideUrlLoadingResult.NO_OVERRIDE;
}
if (shouldBlockAllExternalAppLaunches(params)) return OverrideUrlLoadingResult.NO_OVERRIDE;
if (handleWithAutofillAssistant(params, targetIntent, browserFallbackUrl)) {
return OverrideUrlLoadingResult.NO_OVERRIDE;
......@@ -1055,6 +1124,10 @@ public class ExternalNavigationHandler {
if (hasIntentScheme) recordIntentActionMetrics(targetIntent);
// From this point on, we have determined it is safe to launch an External App from a
// fallback URL, provided the user isn't in incognito.
if (!params.isIncognito()) canLaunchExternalFallbackResult.set(true);
Intent debugIntent = new Intent(targetIntent);
List<ResolveInfo> resolvingInfos = queryIntentActivities(targetIntent);
if (resolvingInfos.isEmpty()) {
......@@ -1182,44 +1255,6 @@ public class ExternalNavigationHandler {
}
}
/**
* Clobber the current tab with fallback URL.
*
* @param browserFallbackUrl The fallback URL.
* @param params The external navigation params.
* @return {@link OverrideUrlLoadingResult} if the tab was clobbered, or we launched an
* intent.
*/
private @OverrideUrlLoadingResult int clobberCurrentTabWithFallbackUrl(
String browserFallbackUrl, ExternalNavigationParams params) {
// If the fallback URL is a link to Play Store, send the user to Play Store app
// instead: crbug.com/638672.
Pair<String, String> appInfo = maybeGetPlayStoreAppIdAndReferrer(browserFallbackUrl);
if (appInfo != null) {
String marketReferrer = TextUtils.isEmpty(appInfo.second)
? ContextUtils.getApplicationContext().getPackageName()
: appInfo.second;
return sendIntentToMarket(appInfo.first, marketReferrer, params);
}
// For subframes, we don't support fallback url for now.
// http://crbug.com/364522.
if (!params.isMainFrame()) {
if (DEBUG) Log.i(TAG, "Don't support fallback url in subframes");
return OverrideUrlLoadingResult.NO_OVERRIDE;
}
// NOTE: any further redirection from fall-back URL should not override URL loading.
// Otherwise, it can be used in chain for fingerprinting multiple app installation
// status in one shot. In order to prevent this scenario, we notify redirection
// handler that redirection from the current navigation should stay in this app.
if (params.getRedirectHandler() != null) {
params.getRedirectHandler().setShouldNotOverrideUrlLoadingOnCurrentRedirectChain();
}
if (DEBUG) Log.i(TAG, "clobberCurrentTab called");
return clobberCurrentTab(browserFallbackUrl, params.getReferrerUrl());
}
/**
* If the given URL is to Google Play, extracts the package name and referrer tracking code
* from the {@param url} and returns as a Pair in that order. Otherwise returns null.
......
components/external_intents/android/javatests/src/org/chromium/components/external_intents/ExternalNavigationHandlerTest.java
View file @ 37b5b744
......@@ -943,18 +943,54 @@ public class ExternalNavigationHandlerTest {
.expecting(OverrideUrlLoadingResult.OVERRIDE_WITH_CLOBBERING_TAB, IGNORE);
}
@Test
@SmallTest
public void testFallbackUrl_SubframeFallbackToMarketApp() {
mDelegate.setCanResolveActivityForExternalSchemes(false);
RedirectHandler redirectHandler = RedirectHandler.create();
redirectHandler.updateNewUrlLoading(PageTransition.LINK, false, true, 0, 0);
String intent = "intent:///name/nm0000158#Intent;scheme=imdb;package=com.imdb.mobile;"
+ "S." + ExternalNavigationHandler.EXTRA_BROWSER_FALLBACK_URL + "="
+ "https://play.google.com/store/apps/details?id=com.imdb.mobile"
+ "&referrer=mypage;end";
checkUrl(intent)
.withIsMainFrame(false)
.withHasUserGesture(true)
.withRedirectHandler(redirectHandler)
.withPageTransition(PageTransition.LINK)
.expecting(OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT,
START_OTHER_ACTIVITY);
Assert.assertEquals("market://details?id=com.imdb.mobile&referrer=mypage",
mDelegate.startActivityIntent.getDataString());
redirectHandler = RedirectHandler.create();
redirectHandler.updateNewUrlLoading(PageTransition.LINK, false, true, 0, 0);
String intentBadUrl = "intent:///name/nm0000158#Intent;scheme=imdb;package=com.imdb.mobile;"
+ "S." + ExternalNavigationHandler.EXTRA_BROWSER_FALLBACK_URL + "="
+ "https://play.google.com/store/search?q=pub:imdb;end";
checkUrl(intentBadUrl)
.withIsMainFrame(false)
.withHasUserGesture(true)
.withRedirectHandler(redirectHandler)
.withPageTransition(PageTransition.LINK)
.expecting(OverrideUrlLoadingResult.NO_OVERRIDE, IGNORE);
}
@Test
@SmallTest
public void testFallbackUrl_RedirectToIntentToMarket() {
mDelegate.setCanResolveActivityForExternalSchemes(false);
RedirectHandler redirectHandler = RedirectHandler.create();
redirectHandler.updateNewUrlLoading(PageTransition.TYPED, false, false, 0, 0);
redirectHandler.updateNewUrlLoading(PageTransition.LINK, false, true, 0, 0);
checkUrl("http://goo.gl/abcdefg")
.withPageTransition(PageTransition.TYPED)
.withPageTransition(PageTransition.LINK)
.withRedirectHandler(redirectHandler)
.expecting(OverrideUrlLoadingResult.NO_OVERRIDE, IGNORE);
redirectHandler.updateNewUrlLoading(PageTransition.LINK, false, false, 0, 0);
redirectHandler.updateNewUrlLoading(PageTransition.LINK, true, true, 0, 0);
String realIntent = "intent:///name/nm0000158#Intent;scheme=imdb;package=com.imdb.mobile;"
+ "S." + ExternalNavigationHandler.EXTRA_BROWSER_FALLBACK_URL + "="
+ "https://play.google.com/store/apps/details?id=com.imdb.mobile"
......@@ -971,6 +1007,44 @@ public class ExternalNavigationHandlerTest {
mDelegate.startActivityIntent.getDataString());
}
@Test
@SmallTest
public void testFallbackUrl_DontFallbackForAutoSubframe() {
// IMDB app isn't installed.
mDelegate.setCanResolveActivityForExternalSchemes(false);
mDelegate.add(new IntentActivity(IMDB_WEBPAGE_FOR_TOM_HANKS, WEBAPK_PACKAGE_NAME)
.withIsWebApk(true));
RedirectHandler redirectHandler = RedirectHandler.create();
redirectHandler.updateNewUrlLoading(PageTransition.AUTO_SUBFRAME, true, true, 0, 0);
checkUrl(INTENT_URL_WITH_FALLBACK_URL)
.withIsMainFrame(false)
.withHasUserGesture(true)
.withRedirectHandler(redirectHandler)
.withPageTransition(PageTransition.AUTO_SUBFRAME)
.withReferrer(SEARCH_RESULT_URL_FOR_TOM_HANKS)
.expecting(OverrideUrlLoadingResult.NO_OVERRIDE, IGNORE);
}
@Test
@SmallTest
public void testFallbackUrl_NoExternalFallbackWithoutGesture() {
mDelegate.add(new IntentActivity(IMDB_WEBPAGE_FOR_TOM_HANKS, WEBAPK_PACKAGE_NAME)
.withIsWebApk(true));
RedirectHandler redirectHandler = RedirectHandler.create();
redirectHandler.updateNewUrlLoading(PageTransition.LINK, false, false, 0, 0);
checkUrl(INTENT_URL_WITH_FALLBACK_URL)
.withHasUserGesture(false)
.withRedirectHandler(redirectHandler)
.withPageTransition(PageTransition.LINK)
.withReferrer(SEARCH_RESULT_URL_FOR_TOM_HANKS)
.expecting(OverrideUrlLoadingResult.OVERRIDE_WITH_CLOBBERING_TAB, IGNORE);
}
@Test
@SmallTest
public void testFallbackUrl_IntentResolutionFailsWithoutPackageName() {
......@@ -2259,6 +2333,7 @@ public class ExternalNavigationHandlerTest {
private boolean mHasUserGesture;
private RedirectHandler mRedirectHandler;
private boolean mIsRendererInitiated;
private boolean mIsMainFrame = true;
private ExternalNavigationTestParams(String url) {
mUrl = url;
......@@ -2311,6 +2386,11 @@ public class ExternalNavigationHandlerTest {
return this;
}
public ExternalNavigationTestParams withIsMainFrame(boolean isMainFrame) {
mIsMainFrame = isMainFrame;
return this;
}
public void expecting(
@OverrideUrlLoadingResult int expectedOverrideResult, int otherExpectation) {
boolean expectStartIncognito = (otherExpectation & START_INCOGNITO) != 0;
......@@ -2331,7 +2411,7 @@ public class ExternalNavigationHandlerTest {
.setApplicationMustBeInForeground(mChromeAppInForegroundRequired)
.setRedirectHandler(mRedirectHandler)
.setIsBackgroundTabNavigation(mIsBackgroundTabNavigation)
.setIsMainFrame(true)
.setIsMainFrame(mIsMainFrame)
.setNativeClientPackageName(mDelegate.getReferrerWebappPackageName())
.setHasUserGesture(mHasUserGesture)
.setIsRendererInitiated(mIsRendererInitiated)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment