From a7edd430e5d2ad41649dff04508392bf45e8e90f Mon Sep 17 00:00:00 2001 From: Michael Thiessen Date: Fri, 9 Apr 2021 02:51:45 +0000 Subject: [PATCH] Convert ExternalNavigationHandler to GURL No functional changes intended, but GURL parsing could subtly differ from the previous Uri and String parsing in unexpected ways. Bug: 783819 Change-Id: I118fae14c47e28058f9735d0bc74b832d5790578 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2792241 Reviewed-by: Charlie Harrison Reviewed-by: Colin Blundell Reviewed-by: Yaron Friedman Commit-Queue: Michael Thiessen Cr-Commit-Position: refs/heads/master@{#870825} --- .../chrome/browser/IntentHandler.java | 7 +- .../ContextualSearchManager.java | 3 +- .../customtabs/CustomTabDelegateFactory.java | 5 +- .../ExternalNavigationDelegateImpl.java | 30 +- .../instantapps/InstantAppsHandler.java | 16 +- .../instantapps/InstantAppsSettings.java | 5 +- .../CustomTabExternalNavigationTest.java | 11 +- .../ExternalNavigationDelegateImplTest.java | 19 +- .../instantapps/InstantAppsHandlerTest.java | 23 +- .../instantapps/instant_apps_settings.cc | 7 +- chrome/browser/android/tab_android.cc | 6 + .../embedder_support/util/UrlConstants.java | 6 +- .../embedder_support/util/UrlUtilities.java | 31 +- .../util/UrlUtilitiesUnitTest.java | 29 +- .../android/util/url_utilities.cc | 26 +- components/external_intents/android/BUILD.gn | 1 + .../ExternalNavigationDelegate.java | 14 +- .../ExternalNavigationHandler.java | 322 ++++++++---------- .../ExternalNavigationParams.java | 22 +- .../InterceptNavigationDelegateImpl.java | 10 +- .../ExternalNavigationHandlerTest.java | 144 ++++---- .../content_public/browser/LoadUrlParams.java | 9 + url/android/gurl_android.cc | 10 + .../java/src/org/chromium/url/GURL.java | 12 + .../src/org/chromium/url/GURLJavaTest.java | 16 + .../ExternalNavigationDelegateImpl.java | 13 +- .../chromium/weblayer_private/TabImpl.java | 5 + 27 files changed, 423 insertions(+), 379 deletions(-) diff --git a/chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java b/chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java index 272a5ffca51100..164e78907f2489 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java @@ -63,6 +63,7 @@ import org.chromium.net.HttpUtil; import org.chromium.network.mojom.ReferrerPolicy; import org.chromium.ui.base.PageTransition; +import org.chromium.url.GURL; import org.chromium.url.Origin; import java.lang.annotation.Retention; @@ -1383,10 +1384,10 @@ public static boolean isGoogleChromeScheme(String url) { * @param intent The intent to which we add a referrer. * @param url The referrer URL. */ - public static void setPendingReferrer(Intent intent, String url) { - intent.putExtra(Intent.EXTRA_REFERRER, Uri.parse(url)); + public static void setPendingReferrer(Intent intent, GURL url) { + intent.putExtra(Intent.EXTRA_REFERRER, Uri.parse(url.getSpec())); intent.putExtra(IntentHandler.EXTRA_REFERRER_ID, ++sReferrerId); - sPendingReferrer = new Pair(sReferrerId, url); + sPendingReferrer = new Pair(sReferrerId, url.getSpec()); } /** diff --git a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java index 59aeb000db6862..4a225fb3f2b1d9 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java @@ -1129,8 +1129,7 @@ public boolean shouldInterceptNavigation( mLastUserInteractionTimeSupplier.get(), RedirectHandler.INVALID_ENTRY_INDEX); ExternalNavigationParams params = new ExternalNavigationParams - .Builder(navigationParams.url.getSpec(), false, - navigationParams.referrer.getSpec(), + .Builder(navigationParams.url, false, navigationParams.referrer, navigationParams.pageTransitionType, navigationParams.isRedirect) .setApplicationMustBeInForeground(true) diff --git a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java index f086ddb2fc0202..9af5036dbd7227 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java @@ -185,10 +185,11 @@ public boolean hasExternalActivityStarted() { } @Override - public boolean shouldDisableExternalIntentRequestsForUrl(String url) { + public boolean shouldDisableExternalIntentRequestsForUrl(GURL url) { // http://crbug.com/647569 : Do not forward URL requests to external intents for URLs // within the Webapp/TWA's scope. - return mVerifier != null && mVerifier.shouldIgnoreExternalIntentHandlers(url); + // TODO(https://crbug.com/783819): Migrate verifier hierarchy to GURL. + return mVerifier != null && mVerifier.shouldIgnoreExternalIntentHandlers(url.getSpec()); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImpl.java b/chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImpl.java index 633b67446d2186..0fa89390c79df1 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImpl.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImpl.java @@ -11,7 +11,6 @@ import android.content.pm.ResolveInfo; import android.net.Uri; import android.provider.Browser; -import android.text.TextUtils; import androidx.annotation.Nullable; @@ -44,6 +43,7 @@ import org.chromium.content_public.browser.LoadUrlParams; import org.chromium.content_public.browser.WebContents; import org.chromium.ui.base.WindowAndroid; +import org.chromium.url.GURL; import org.chromium.url.Origin; import java.util.List; @@ -118,7 +118,7 @@ public boolean willAppHandleIntent(Intent intent) { } @Override - public boolean shouldDisableExternalIntentRequestsForUrl(String url) { + public boolean shouldDisableExternalIntentRequestsForUrl(GURL url) { return false; } @@ -154,10 +154,10 @@ public void didStartActivity(Intent intent) {} @Override public OverrideUrlLoadingResult handleIncognitoIntentTargetingSelf( - final Intent intent, final String referrerUrl, final String fallbackUrl) { + final Intent intent, final GURL referrerUrl, final GURL fallbackUrl) { String primaryUrl = intent.getDataString(); boolean isUrlLoadedInTheSameTab = ExternalNavigationHandler.loadUrlFromIntent( - referrerUrl, primaryUrl, fallbackUrl, this, false, true); + referrerUrl, new GURL(primaryUrl), fallbackUrl.getSpec(), this, false, true); return (isUrlLoadedInTheSameTab) ? OverrideUrlLoadingResult.forClobberingTab() : OverrideUrlLoadingResult.forExternalIntent(); } @@ -168,8 +168,8 @@ public boolean supportsCreatingNewTabs() { } @Override - public void loadUrlInNewTab(final String url, final boolean launchIncognito) { - Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse(url)); + public void loadUrlInNewTab(final GURL url, final boolean launchIncognito) { + Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse(url.getSpec())); String packageName = ContextUtils.getApplicationContext().getPackageName(); intent.putExtra(Browser.EXTRA_APPLICATION_ID, packageName); if (launchIncognito) intent.putExtra(IntentHandler.EXTRA_OPEN_NEW_INCOGNITO_TAB, true); @@ -241,7 +241,7 @@ public void maybeSetRequestMetadata(Intent intent, boolean hasUserGesture, } @Override - public void maybeSetPendingReferrer(Intent intent, String referrerUrl) { + public void maybeSetPendingReferrer(Intent intent, GURL referrerUrl) { IntentHandler.setPendingReferrer(intent, referrerUrl); } @@ -252,7 +252,7 @@ public void maybeSetPendingIncognitoUrl(Intent intent) { @Override public boolean maybeLaunchInstantApp( - String url, String referrerUrl, boolean isIncomingRedirect, boolean isSerpReferrer) { + GURL url, GURL referrerUrl, boolean isIncomingRedirect, boolean isSerpReferrer) { if (!hasValidTab() || mTab.getWebContents() == null) return false; InstantAppsHandler handler = InstantAppsHandler.getInstance(); @@ -263,14 +263,13 @@ public boolean maybeLaunchInstantApp( // Set the URL the redirect was resolved to for checking the existence of the // instant app inside handleIncomingIntent(). Intent resolvedIntent = new Intent(intent); - resolvedIntent.setData(Uri.parse(url)); + resolvedIntent.setData(Uri.parse(url.getSpec())); return handler.handleIncomingIntent(getAvailableContext(), resolvedIntent, LaunchIntentDispatcher.isCustomTabIntent(resolvedIntent), true); } else if (!isIncomingRedirect) { // Check if the navigation is coming from SERP and skip instant app handling. if (isSerpReferrer) return false; - return handler.handleNavigation(getAvailableContext(), url, - TextUtils.isEmpty(referrerUrl) ? null : Uri.parse(referrerUrl), mTab); + return handler.handleNavigation(getAvailableContext(), url, referrerUrl, mTab); } return false; } @@ -300,10 +299,9 @@ public void dispatchAuthenticatedIntent(Intent intent) { * Starts the autofill assistant with the given intent. Exists to allow tests to stub out this * functionality. */ - protected void startAutofillAssistantWithIntent( - Intent targetIntent, String browserFallbackUrl) { + protected void startAutofillAssistantWithIntent(Intent targetIntent, GURL browserFallbackUrl) { AutofillAssistantFacade.start( - TabUtils.getActivity(mTab), targetIntent.getExtras(), browserFallbackUrl); + TabUtils.getActivity(mTab), targetIntent.getExtras(), browserFallbackUrl.getSpec()); } /** @@ -336,8 +334,8 @@ public boolean isIntentToAutofillAssistant(Intent intent) { @Override public boolean handleWithAutofillAssistant(ExternalNavigationParams params, Intent targetIntent, - String browserFallbackUrl, boolean isGoogleReferrer) { - if (browserFallbackUrl != null && !params.isIncognito() + GURL browserFallbackUrl, boolean isGoogleReferrer) { + if (!browserFallbackUrl.isEmpty() && !params.isIncognito() && AutofillAssistantFacade.isAutofillAssistantByIntentTriggeringEnabled( targetIntent) && isGoogleReferrer) { diff --git a/chrome/android/java/src/org/chromium/chrome/browser/instantapps/InstantAppsHandler.java b/chrome/android/java/src/org/chromium/chrome/browser/instantapps/InstantAppsHandler.java index 3e8bfd54b62cf6..756836b850c30f 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/instantapps/InstantAppsHandler.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/instantapps/InstantAppsHandler.java @@ -23,6 +23,7 @@ import org.chromium.chrome.browser.preferences.SharedPreferencesManager; import org.chromium.chrome.browser.tab.Tab; import org.chromium.content_public.browser.WebContents; +import org.chromium.url.GURL; /** A launcher for Instant Apps. */ public class InstantAppsHandler { @@ -231,13 +232,14 @@ protected boolean tryLaunchingInstantApp( * App banner. * @return Whether an Instant App intent was started. */ - public boolean handleNavigation(Context context, String url, Uri referrer, Tab tab) { + public boolean handleNavigation(Context context, GURL url, GURL referrer, Tab tab) { boolean urlIsInstantAppDefault = InstantAppsSettings.isInstantAppDefault(tab.getWebContents(), url); + Uri referrerUri = referrer.isEmpty() ? null : Uri.parse(referrer.getSpec()); if (shouldLaunchInstantApp(tab.getWebContents(), url, referrer, urlIsInstantAppDefault)) { - return launchInstantAppForNavigation(context, url, referrer); + return launchInstantAppForNavigation(context, url.getSpec(), referrerUri); } - maybeShowInstantAppBanner(context, url, referrer, tab, urlIsInstantAppDefault); + maybeShowInstantAppBanner(context, url.getSpec(), referrerUri, tab, urlIsInstantAppDefault); return false; } @@ -250,7 +252,7 @@ public boolean handleNavigation(Context context, String url, Uri referrer, Tab t * @return Whether we should launch the instant app. */ private boolean shouldLaunchInstantApp( - WebContents webContents, String url, Uri referrer, boolean urlIsInstantAppDefault) { + WebContents webContents, GURL url, GURL referrer, boolean urlIsInstantAppDefault) { // Launch the instant app automatically on these conditions: // a) The host of the current URL and referrer are different, and the user has chosen to // launch this instant app in the past. @@ -258,10 +260,8 @@ private boolean shouldLaunchInstantApp( // handled by an instant app and the current one is. if (!urlIsInstantAppDefault) return false; - String urlHost = Uri.parse(url).getHost(); - boolean sameHosts = - referrer != null && urlHost != null && urlHost.equals(referrer.getHost()); - return (sameHosts && getInstantAppIntentForUrl(referrer.toString()) == null) || !sameHosts; + boolean sameHosts = !referrer.isEmpty() && url.getHost().equals(referrer.getHost()); + return (sameHosts && getInstantAppIntentForUrl(referrer.getSpec()) == null) || !sameHosts; } /** diff --git a/chrome/android/java/src/org/chromium/chrome/browser/instantapps/InstantAppsSettings.java b/chrome/android/java/src/org/chromium/chrome/browser/instantapps/InstantAppsSettings.java index 116138e3149c03..56eb98a38c53d4 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/instantapps/InstantAppsSettings.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/instantapps/InstantAppsSettings.java @@ -6,6 +6,7 @@ import org.chromium.base.annotations.NativeMethods; import org.chromium.content_public.browser.WebContents; +import org.chromium.url.GURL; /** * A bridge class for retrieving Instant Apps-related settings. @@ -15,7 +16,7 @@ public class InstantAppsSettings { /** * Check whether the instant app at the given url should be opened by default. */ - public static boolean isInstantAppDefault(WebContents webContents, String url) { + public static boolean isInstantAppDefault(WebContents webContents, GURL url) { return InstantAppsSettingsJni.get().getInstantAppDefault(webContents, url); } @@ -35,7 +36,7 @@ public static boolean shouldShowBanner(WebContents webContents, String url) { @NativeMethods interface Natives { - boolean getInstantAppDefault(WebContents webContents, String url); + boolean getInstantAppDefault(WebContents webContents, GURL url); void setInstantAppDefault(WebContents webContents, String url); boolean shouldShowBanner(WebContents webContents, String url); } diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabExternalNavigationTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabExternalNavigationTest.java index 7475b2c591d93e..74fe3ef69e363c 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabExternalNavigationTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabExternalNavigationTest.java @@ -33,6 +33,7 @@ import org.chromium.components.external_intents.ExternalNavigationHandler.OverrideUrlLoadingResultType; import org.chromium.components.external_intents.ExternalNavigationParams; import org.chromium.net.test.EmbeddedTestServer; +import org.chromium.url.GURL; import java.util.concurrent.TimeoutException; @@ -107,7 +108,7 @@ private void launchTwa(String twaPackageName, String url) throws TimeoutExceptio @Test @SmallTest public void testExternalActivityStartedForDefaultUrl() { - final String testUrl = "customtab://customtabtest/intent"; + final GURL testUrl = new GURL("customtab://customtabtest/intent"); ExternalNavigationParams params = new ExternalNavigationParams.Builder(testUrl, false) .build(); OverrideUrlLoadingResult result = mUrlHandler.shouldOverrideUrlLoading(params); @@ -127,7 +128,7 @@ public void testExternalActivityStartedForDefaultUrl() { sdk_is_less_than = VERSION_CODES.Q, message = "crbug.com/1188920") public void testIntentPickerNotShownForNormalUrl() { - final String testUrl = "http://customtabtest.com"; + final GURL testUrl = new GURL("http://customtabtest.com"); ExternalNavigationParams params = new ExternalNavigationParams.Builder(testUrl, false) .build(); OverrideUrlLoadingResult result = mUrlHandler.shouldOverrideUrlLoading(params); @@ -149,9 +150,9 @@ public void testExternalActivityStartedForDefaultUrl() { @Test @SmallTest public void testShouldDisableExternalIntentRequestsForUrl() throws TimeoutException { - String insideVerifiedOriginUrl = - mTestServer.getURL("/chrome/test/data/android/simple.html"); - String outsideVerifiedOriginUrl = "https://example.com/test.html"; + GURL insideVerifiedOriginUrl = + new GURL(mTestServer.getURL("/chrome/test/data/android/simple.html")); + GURL outsideVerifiedOriginUrl = new GURL("https://example.com/test.html"); TrustedWebActivityTestUtil.waitForCurrentPageVerifierToFinish( mCustomTabActivityTestRule.getActivity()); diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImplTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImplTest.java index fb44936fca24d9..33d397738b7e0e 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImplTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImplTest.java @@ -27,6 +27,7 @@ import org.chromium.components.external_intents.ExternalNavigationHandler; import org.chromium.components.external_intents.ExternalNavigationParams; import org.chromium.content_public.browser.test.util.TestThreadUtils; +import org.chromium.url.GURL; import org.chromium.url.Origin; /** @@ -56,7 +57,7 @@ public ExternalNavigationDelegateImplForTesting(Tab activityTab) { @Override protected void startAutofillAssistantWithIntent( - Intent targetIntent, String browserFallbackUrl) { + Intent targetIntent, GURL browserFallbackUrl) { mWasAutofillAssistantStarted = true; } @@ -76,7 +77,7 @@ public boolean handleWithAutofillAssistant( return false; } - String fallbackUrl = "https://www.example.com"; + GURL fallbackUrl = new GURL("https://www.example.com"); return handleWithAutofillAssistant(params, intent, fallbackUrl, isGoogleReferrer); } @@ -213,12 +214,12 @@ public void maybeSetRequestMetadata() { @Test @SmallTest public void testMaybeSetPendingReferrer() { - String url = "http://www.example.com"; + String url = "http://www.example.com/"; Intent intent = new Intent(Intent.ACTION_VIEW); intent.setData(Uri.parse(url)); - String referrerUrl = "http://www.example-referrer.com"; - mExternalNavigationDelegateImpl.maybeSetPendingReferrer(intent, referrerUrl); + String referrerUrl = "http://www.example-referrer.com/"; + mExternalNavigationDelegateImpl.maybeSetPendingReferrer(intent, new GURL(referrerUrl)); Assert.assertEquals( Uri.parse(referrerUrl), intent.getParcelableExtra(Intent.EXTRA_REFERRER)); @@ -234,7 +235,7 @@ public void testMaybeSetPendingReferrer() { testHandleWithAutofillAssistant_TriggersFromSearch() { ExternalNavigationParams params = new ExternalNavigationParams - .Builder(AUTOFILL_ASSISTANT_INTENT_URL, /*isIncognito=*/false) + .Builder(new GURL(AUTOFILL_ASSISTANT_INTENT_URL), /*isIncognito=*/false) .build(); Assert.assertTrue(mExternalNavigationDelegateImplForTesting.handleWithAutofillAssistant( @@ -250,7 +251,7 @@ public void testMaybeSetPendingReferrer() { testHandleWithAutofillAssistant_DoesNotTriggerFromSearchInIncognito() { ExternalNavigationParams params = new ExternalNavigationParams - .Builder(AUTOFILL_ASSISTANT_INTENT_URL, /*isIncognito=*/true) + .Builder(new GURL(AUTOFILL_ASSISTANT_INTENT_URL), /*isIncognito=*/true) .build(); Assert.assertFalse(mExternalNavigationDelegateImplForTesting.handleWithAutofillAssistant( @@ -266,7 +267,7 @@ public void testMaybeSetPendingReferrer() { testHandleWithAutofillAssistant_DoesNotTriggerFromDifferentOrigin() { ExternalNavigationParams params = new ExternalNavigationParams - .Builder(AUTOFILL_ASSISTANT_INTENT_URL, /*isIncognito=*/false) + .Builder(new GURL(AUTOFILL_ASSISTANT_INTENT_URL), /*isIncognito=*/false) .build(); Assert.assertFalse(mExternalNavigationDelegateImplForTesting.handleWithAutofillAssistant( @@ -282,7 +283,7 @@ public void testMaybeSetPendingReferrer() { testHandleWithAutofillAssistant_DoesNotTriggerWhenFeatureDisabled() { ExternalNavigationParams params = new ExternalNavigationParams - .Builder(AUTOFILL_ASSISTANT_INTENT_URL, /*isIncognito=*/false) + .Builder(new GURL(AUTOFILL_ASSISTANT_INTENT_URL), /*isIncognito=*/false) .build(); Assert.assertFalse(mExternalNavigationDelegateImplForTesting.handleWithAutofillAssistant( diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/instantapps/InstantAppsHandlerTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/instantapps/InstantAppsHandlerTest.java index a2b7a91b8a0eed..cf077256494cdb 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/instantapps/InstantAppsHandlerTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/instantapps/InstantAppsHandlerTest.java @@ -31,6 +31,7 @@ import org.chromium.chrome.test.ChromeJUnit4ClassRunner; import org.chromium.chrome.test.ChromeTabbedActivityTestRule; import org.chromium.content_public.browser.test.util.TestThreadUtils; +import org.chromium.url.GURL; /** * Unit tests for {@link InstantAppsHandler}. @@ -162,11 +163,12 @@ public void testNfcIntent() { @SmallTest public void testHandleNavigation_startAsyncCheck() { TestThreadUtils.runOnUiThreadBlocking( - () -> Assert.assertFalse( - mHandler.handleNavigation(mContext, INSTANT_APP_URL, REFERRER_URI, - mActivityTestRule.getActivity() - .getTabModelSelector() - .getCurrentTab()))); + () + -> Assert.assertFalse(mHandler.handleNavigation(mContext, + new GURL(INSTANT_APP_URL), new GURL(REFERRER_URI.toString()), + mActivityTestRule.getActivity() + .getTabModelSelector() + .getCurrentTab()))); Assert.assertFalse(mHandler.mLaunchInstantApp); Assert.assertTrue(mHandler.mStartedAsyncCall); } @@ -204,11 +206,12 @@ public void testLaunchFromBanner() { // After a banner launch, test that the next launch happens automatically TestThreadUtils.runOnUiThreadBlocking( - () -> Assert.assertTrue( - mHandler.handleNavigation(mContext, INSTANT_APP_URL, REFERRER_URI, - mActivityTestRule.getActivity() - .getTabModelSelector() - .getCurrentTab()))); + () + -> Assert.assertTrue(mHandler.handleNavigation(mContext, + new GURL(INSTANT_APP_URL), new GURL(REFERRER_URI.toString()), + mActivityTestRule.getActivity() + .getTabModelSelector() + .getCurrentTab()))); Assert.assertFalse(mHandler.mStartedAsyncCall); Assert.assertTrue(mHandler.mLaunchInstantApp); } diff --git a/chrome/browser/android/instantapps/instant_apps_settings.cc b/chrome/browser/android/instantapps/instant_apps_settings.cc index 63b5433b17b9c2..9298891043438e 100644 --- a/chrome/browser/android/instantapps/instant_apps_settings.cc +++ b/chrome/browser/android/instantapps/instant_apps_settings.cc @@ -13,6 +13,7 @@ #include "components/webapps/browser/banners/app_banner_settings_helper.h" #include "components/webapps/browser/installable/installable_logging.h" #include "content/public/browser/web_contents.h" +#include "url/android/gurl_android.h" #include "url/gurl.h" using base::android::JavaParamRef; @@ -80,16 +81,14 @@ static void JNI_InstantAppsSettings_SetInstantAppDefault( static jboolean JNI_InstantAppsSettings_GetInstantAppDefault( JNIEnv* env, const JavaParamRef& jweb_contents, - const JavaParamRef& jurl) { + const JavaParamRef& jurl) { content::WebContents* web_contents = content::WebContents::FromJavaWebContents(jweb_contents); DCHECK(web_contents); - std::string url(ConvertJavaStringToUTF8(env, jurl)); - base::Optional added_time = webapps::AppBannerSettingsHelper::GetSingleBannerEvent( - web_contents, GURL(url), + web_contents, *url::GURLAndroid::ToNativeGURL(env, jurl), webapps::AppBannerSettingsHelper::kInstantAppsKey, webapps::AppBannerSettingsHelper:: APP_BANNER_EVENT_DID_ADD_TO_HOMESCREEN); diff --git a/chrome/browser/android/tab_android.cc b/chrome/browser/android/tab_android.cc index fdb22506d03bfe..b4eed95c88e135 100644 --- a/chrome/browser/android/tab_android.cc +++ b/chrome/browser/android/tab_android.cc @@ -404,6 +404,12 @@ TabAndroid::TabLoadStatus TabAndroid::LoadUrl( if (gurl.is_empty()) return PAGE_LOAD_FAILED; + // TODO(https://crbug.com/783819): Don't fix up all URLs. Documentation on + // FixupURL explicitly says not to use it on URLs coming from untrustworthy + // sources, like other apps. Once migrations of Java code to GURL are complete + // and incoming URLs are converted to GURLs at their source, we can make + // decisions of whether or not to fix up GURLs on a case-by-case basis based + // on trustworthiness of the incoming URL. GURL fixed_url( url_formatter::FixupURL(gurl.possibly_invalid_spec(), std::string())); if (!fixed_url.is_valid()) diff --git a/components/embedder_support/android/java/src/org/chromium/components/embedder_support/util/UrlConstants.java b/components/embedder_support/android/java/src/org/chromium/components/embedder_support/util/UrlConstants.java index 7a578181912a7e..03b9f0137730e4 100644 --- a/components/embedder_support/android/java/src/org/chromium/components/embedder_support/util/UrlConstants.java +++ b/components/embedder_support/android/java/src/org/chromium/components/embedder_support/util/UrlConstants.java @@ -8,18 +8,21 @@ * Java side version of chrome/common/url_constants.cc */ public class UrlConstants { + public static final String APP_INTENT_SCHEME = "android-app"; public static final String BLOB_SCHEME = "blob"; public static final String CHROME_SCHEME = "chrome"; public static final String CHROME_NATIVE_SCHEME = "chrome-native"; public static final String CONTENT_SCHEME = "content"; public static final String CUSTOM_TAB_SCHEME = "customtab"; public static final String DATA_SCHEME = "data"; + public static final String DEVTOOLS_SCHEME = "devtools"; public static final String DOCUMENT_SCHEME = "document"; public static final String FILE_SCHEME = "file"; public static final String FTP_SCHEME = "ftp"; public static final String HTTP_SCHEME = "http"; public static final String HTTPS_SCHEME = "https"; public static final String INLINE_SCHEME = "inline"; + public static final String INTENT_SCHEME = "intent"; public static final String JAR_SCHEME = "jar"; public static final String JAVASCRIPT_SCHEME = "javascript"; public static final String SMS_SCHEME = "sms"; @@ -29,9 +32,6 @@ public class UrlConstants { public static final String CHROME_URL_SHORT_PREFIX = "chrome:"; public static final String CHROME_NATIVE_URL_SHORT_PREFIX = "chrome-native:"; public static final String FILE_URL_SHORT_PREFIX = "file:"; - public static final String DEVTOOLS_URL_SHORT_PREFIX = "devtools:"; - public static final String INTENT_URL_SHORT_PREFIX = "intent:"; - public static final String APP_INTENT_URL_SHORT_PREFIX = "android-app:"; public static final String CHROME_URL_PREFIX = "chrome://"; public static final String CHROME_NATIVE_URL_PREFIX = "chrome-native://"; diff --git a/components/embedder_support/android/java/src/org/chromium/components/embedder_support/util/UrlUtilities.java b/components/embedder_support/android/java/src/org/chromium/components/embedder_support/util/UrlUtilities.java index ef5cfd45cbe5b4..315816e88146bd 100644 --- a/components/embedder_support/android/java/src/org/chromium/components/embedder_support/util/UrlUtilities.java +++ b/components/embedder_support/android/java/src/org/chromium/components/embedder_support/util/UrlUtilities.java @@ -76,15 +76,6 @@ public static boolean isAcceptedScheme(String uri) { return UrlUtilitiesJni.get().isAcceptedScheme(uri); } - /** - * @param uri A URI. - * - * @return True if the URI is valid for Intent fallback navigation. - */ - public static boolean isValidForIntentFallbackNavigation(String uri) { - return UrlUtilitiesJni.get().isValidForIntentFallbackNavigation(uri); - } - /** * @param uri A URI. * @@ -270,10 +261,29 @@ public static String extractPublisherFromPublisherUrl(String publisherUrl) { return BidiFormatter.getInstance().unicodeWrap(trimmedPublisher); } + /** + * See native url_util::GetValueForKeyInQuery(). + * + * Equivalent to {@link Uri#getQueryParameter(String)}. + * + * @return null if the key doesn't exist in the query string for the URL. Otherwise, returns the + * value for the key in the query string. + */ + public static String getValueForKeyInQuery(GURL url, String key) { + return UrlUtilitiesJni.get().getValueForKeyInQuery(url, key); + } + + /** + * @return true if |url|'s scheme is for an Android intent. + */ + public static boolean hasIntentScheme(GURL url) { + return url.getScheme().equals(UrlConstants.APP_INTENT_SCHEME) + || url.getScheme().equals(UrlConstants.INTENT_SCHEME); + } + @NativeMethods public interface Natives { boolean isDownloadable(GURL url); - boolean isValidForIntentFallbackNavigation(String url); boolean isAcceptedScheme(String url); boolean sameDomainOrHost( String primaryUrl, String secondaryUrl, boolean includePrivateRegistries); @@ -295,5 +305,6 @@ boolean sameDomainOrHost( boolean urlsFragmentsDiffer(String url, String url2); String escapeQueryParamValue(String url, boolean usePlus); + String getValueForKeyInQuery(GURL url, String key); } } diff --git a/components/embedder_support/android/javatests/src/org/chromium/components/embedder_support/util/UrlUtilitiesUnitTest.java b/components/embedder_support/android/javatests/src/org/chromium/components/embedder_support/util/UrlUtilitiesUnitTest.java index 37188b78e75479..8337f9da6766c4 100644 --- a/components/embedder_support/android/javatests/src/org/chromium/components/embedder_support/util/UrlUtilitiesUnitTest.java +++ b/components/embedder_support/android/javatests/src/org/chromium/components/embedder_support/util/UrlUtilitiesUnitTest.java @@ -139,22 +139,6 @@ public void testIsDownloadableScheme() { Assert.assertFalse(UrlUtilities.isDownloadableScheme(GURL.emptyGURL())); } - @Test - @SmallTest - public void testIsValidForIntentFallbackUrl() { - Assert.assertTrue(UrlUtilities.isValidForIntentFallbackNavigation( - "https://user:pass@awesome.com:9000/bad-scheme:#fake:")); - Assert.assertTrue( - UrlUtilities.isValidForIntentFallbackNavigation("http://awesome.example.com/")); - Assert.assertFalse(UrlUtilities.isValidForIntentFallbackNavigation("inline:skates.co.uk")); - Assert.assertFalse(UrlUtilities.isValidForIntentFallbackNavigation("javascript:alert(1)")); - Assert.assertFalse( - UrlUtilities.isValidForIntentFallbackNavigation("file://hostname/path/to/file")); - Assert.assertFalse(UrlUtilities.isValidForIntentFallbackNavigation("data:data")); - Assert.assertFalse(UrlUtilities.isValidForIntentFallbackNavigation("about:awesome")); - Assert.assertFalse(UrlUtilities.isValidForIntentFallbackNavigation("")); - } - @Test @SmallTest public void testIsUrlWithinScope() { @@ -253,4 +237,17 @@ public void testEscapeQueryParamValue() { Assert.assertEquals("foo+bar", UrlUtilities.escapeQueryParamValue("foo bar", true)); Assert.assertEquals("foo%2B%2B", UrlUtilities.escapeQueryParamValue("foo++", true)); } + + // Note that this just tests the plumbing of the Java code to the native + // net::GetValueForKeyInQuery function, which is tested much more thoroughly there. + @Test + @SmallTest + public void testGetValueForKeyInQuery() { + GURL url = new GURL("https://www.example.com/?q1=foo&q2=bar&q11=#q2=notbar&q3=baz"); + Assert.assertEquals("foo", UrlUtilities.getValueForKeyInQuery(url, "q1")); + Assert.assertEquals("bar", UrlUtilities.getValueForKeyInQuery(url, "q2")); + Assert.assertEquals("", UrlUtilities.getValueForKeyInQuery(url, "q11")); + Assert.assertNull(UrlUtilities.getValueForKeyInQuery(url, "1")); + Assert.assertNull(UrlUtilities.getValueForKeyInQuery(url, "q3")); + } } diff --git a/components/embedder_support/android/util/url_utilities.cc b/components/embedder_support/android/util/url_utilities.cc index 3415b78565b25d..b1468b2fa3db63 100644 --- a/components/embedder_support/android/util/url_utilities.cc +++ b/components/embedder_support/android/util/url_utilities.cc @@ -11,6 +11,7 @@ #include "components/google/core/common/google_util.h" #include "net/base/escape.h" #include "net/base/registry_controlled_domains/registry_controlled_domain.h" +#include "net/base/url_util.h" #include "url/android/gurl_android.h" #include "url/gurl.h" @@ -29,9 +30,6 @@ static const char* const g_supported_schemes[] = { static const char* const g_downloadable_schemes[] = { "data", "blob", "file", "filesystem", "http", "https", nullptr}; -static const char* const g_fallback_valid_schemes[] = {"http", "https", - nullptr}; - GURL JNI_UrlUtilities_ConvertJavaStringToGURL(JNIEnv* env, jstring url) { return url ? GURL(ConvertJavaStringToUTF8(env, url)) : GURL(); } @@ -196,13 +194,6 @@ static jboolean JNI_UrlUtilities_IsAcceptedScheme( return CheckSchemeBelongsToList(env, gurl, g_supported_schemes); } -static jboolean JNI_UrlUtilities_IsValidForIntentFallbackNavigation( - JNIEnv* env, - const JavaParamRef& url) { - GURL gurl = JNI_UrlUtilities_ConvertJavaStringToGURL(env, url); - return CheckSchemeBelongsToList(env, gurl, g_fallback_valid_schemes); -} - static jboolean JNI_UrlUtilities_IsDownloadable( JNIEnv* env, const JavaParamRef& url) { @@ -219,4 +210,19 @@ static ScopedJavaLocalRef JNI_UrlUtilities_EscapeQueryParamValue( base::android::ConvertJavaStringToUTF8(url), use_plus)); } +static ScopedJavaLocalRef JNI_UrlUtilities_GetValueForKeyInQuery( + JNIEnv* env, + const JavaParamRef& j_url, + const JavaParamRef& j_key) { + DCHECK(j_url); + DCHECK(j_key); + const std::string& key = ConvertJavaStringToUTF8(env, j_key); + std::string out; + if (!net::GetValueForKeyInQuery(*url::GURLAndroid::ToNativeGURL(env, j_url), + key, &out)) { + return ScopedJavaLocalRef(); + } + return base::android::ConvertUTF8ToJavaString(env, out); +} + } // namespace embedder_support diff --git a/components/external_intents/android/BUILD.gn b/components/external_intents/android/BUILD.gn index 86d85e2c9d08ef..bc5815b4cbe828 100644 --- a/components/external_intents/android/BUILD.gn +++ b/components/external_intents/android/BUILD.gn @@ -23,6 +23,7 @@ android_library("java") { "//base:base_java", "//components/embedder_support/android:util_java", "//components/navigation_interception/android:navigation_interception_java", + "//components/url_formatter/android:url_formatter_java", "//components/webapk/android/libs/client:java", "//content/public/android:content_java", "//services/network/public/mojom:url_loader_base_java", diff --git a/components/external_intents/android/java/src/org/chromium/components/external_intents/ExternalNavigationDelegate.java b/components/external_intents/android/java/src/org/chromium/components/external_intents/ExternalNavigationDelegate.java index 72a6336f77817b..216833f45a1490 100644 --- a/components/external_intents/android/java/src/org/chromium/components/external_intents/ExternalNavigationDelegate.java +++ b/components/external_intents/android/java/src/org/chromium/components/external_intents/ExternalNavigationDelegate.java @@ -8,13 +8,13 @@ import android.content.Intent; import androidx.annotation.IntDef; -import androidx.annotation.NonNull; import androidx.annotation.Nullable; import org.chromium.components.external_intents.ExternalNavigationHandler.OverrideUrlLoadingResult; import org.chromium.content_public.browser.LoadUrlParams; import org.chromium.content_public.browser.WebContents; import org.chromium.ui.base.WindowAndroid; +import org.chromium.url.GURL; import org.chromium.url.Origin; import java.lang.annotation.Retention; @@ -44,7 +44,7 @@ public interface ExternalNavigationDelegate { /** * Returns whether to disable forwarding URL requests to external intents for the passed-in URL. */ - boolean shouldDisableExternalIntentRequestsForUrl(String url); + boolean shouldDisableExternalIntentRequestsForUrl(GURL url); /** * Returns whether the embedder has custom integration with InstantApps (most embedders will not @@ -103,7 +103,7 @@ public interface ExternalNavigationDelegate { * @return The OverrideUrlLoadingResult for the action taken by the embedder. */ OverrideUrlLoadingResult handleIncognitoIntentTargetingSelf( - Intent intent, String referrerUrl, String fallbackUrl); + Intent intent, GURL referrerUrl, GURL fallbackUrl); /** * Loads a URL as specified by |loadUrlParams| if possible. May fail in exceptional conditions @@ -116,7 +116,7 @@ OverrideUrlLoadingResult handleIncognitoIntentTargetingSelf( void maybeSetWindowId(Intent intent); /** Records the pending referrer if desired. */ - void maybeSetPendingReferrer(Intent intent, @NonNull String referrerUrl); + void maybeSetPendingReferrer(Intent intent, GURL referrerUrl); /** * Adjusts any desired extras related to intents to instant apps based on the value of @@ -152,7 +152,7 @@ void maybeSetRequestMetadata(Intent intent, boolean hasUserGesture, boolean isRe * @return Whether we launched an instant app. */ boolean maybeLaunchInstantApp( - String url, String referrerUrl, boolean isIncomingRedirect, boolean isSerpReferrer); + GURL url, GURL referrerUrl, boolean isIncomingRedirect, boolean isSerpReferrer); /** * @return The WindowAndroid instance associated with this delegate instance. @@ -190,7 +190,7 @@ boolean maybeLaunchInstantApp( * @param url The URL to load. * @param launchIncognito whether the new tab should be incognito. */ - void loadUrlInNewTab(final String url, final boolean launchIncognito); + void loadUrlInNewTab(final GURL url, final boolean launchIncognito); /** * @return whether it's possible to load a URL in the current tab. @@ -225,5 +225,5 @@ boolean maybeLaunchInstantApp( * Gives the embedder a chance to handle the intent via the autofill assistant. */ boolean handleWithAutofillAssistant(ExternalNavigationParams params, Intent targetIntent, - String browserFallbackUrl, boolean isGoogleReferrer); + GURL browserFallbackUrl, boolean isGoogleReferrer); } diff --git a/components/external_intents/android/java/src/org/chromium/components/external_intents/ExternalNavigationHandler.java b/components/external_intents/android/java/src/org/chromium/components/external_intents/ExternalNavigationHandler.java index f6c300620c655a..8e7f916ee7377b 100644 --- a/components/external_intents/android/java/src/org/chromium/components/external_intents/ExternalNavigationHandler.java +++ b/components/external_intents/android/java/src/org/chromium/components/external_intents/ExternalNavigationHandler.java @@ -58,7 +58,6 @@ import org.chromium.ui.base.PageTransition; import org.chromium.ui.base.PermissionCallback; import org.chromium.url.GURL; -import org.chromium.url.URI; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -249,21 +248,25 @@ public ExternalNavigationHandler(ExternalNavigationDelegate delegate) { * current tab, or wasn't handled at all. */ public OverrideUrlLoadingResult shouldOverrideUrlLoading(ExternalNavigationParams params) { - if (DEBUG) Log.i(TAG, "shouldOverrideUrlLoading called on " + params.getUrl()); + if (DEBUG) Log.i(TAG, "shouldOverrideUrlLoading called on " + params.getUrl().getSpec()); Intent targetIntent; // Perform generic parsing of the URI to turn it into an Intent. - try { - targetIntent = Intent.parseUri(params.getUrl(), Intent.URI_INTENT_SCHEME); - } catch (Exception ex) { - Log.w(TAG, "Bad URI %s", params.getUrl(), ex); - return OverrideUrlLoadingResult.forNoOverride(); + if (UrlUtilities.hasIntentScheme(params.getUrl())) { + try { + targetIntent = Intent.parseUri(params.getUrl().getSpec(), Intent.URI_INTENT_SCHEME); + } catch (Exception ex) { + Log.w(TAG, "Bad URI %s", params.getUrl().getSpec(), ex); + return OverrideUrlLoadingResult.forNoOverride(); + } + } else { + targetIntent = new Intent(Intent.ACTION_VIEW); + targetIntent.setData(Uri.parse(params.getUrl().getSpec())); } - String browserFallbackUrl = - IntentUtils.safeGetStringExtra(targetIntent, EXTRA_BROWSER_FALLBACK_URL); - if (browserFallbackUrl != null - && !UrlUtilities.isValidForIntentFallbackNavigation(browserFallbackUrl)) { - browserFallbackUrl = null; + GURL browserFallbackUrl = + new GURL(IntentUtils.safeGetStringExtra(targetIntent, EXTRA_BROWSER_FALLBACK_URL)); + if (!browserFallbackUrl.isValid() || !UrlUtilities.isHttpOrHttps(browserFallbackUrl)) { + browserFallbackUrl = GURL.emptyGURL(); } // TODO(https://crbug.com/1096099): Refactor shouldOverrideUrlLoadingInternal, splitting it @@ -287,7 +290,7 @@ public OverrideUrlLoadingResult shouldOverrideUrlLoading(ExternalNavigationParam params.hasUserGesture()); } } else if (result.getResultType() == OverrideUrlLoadingResultType.NO_OVERRIDE - && browserFallbackUrl != null + && !browserFallbackUrl.isEmpty() && (params.getRedirectHandler() == null // For instance, if this is a chained fallback URL, we ignore it. || !params.getRedirectHandler().shouldNotOverrideUrlLoading())) { @@ -299,7 +302,7 @@ public OverrideUrlLoadingResult shouldOverrideUrlLoading(ExternalNavigationParam } private OverrideUrlLoadingResult handleFallbackUrl(ExternalNavigationParams params, - Intent targetIntent, String browserFallbackUrl, boolean canLaunchExternalFallback) { + Intent targetIntent, GURL browserFallbackUrl, boolean canLaunchExternalFallback) { if (mDelegate.isIntentToInstantApp(targetIntent)) { RecordHistogram.recordEnumeratedHistogram("Android.InstantApps.DirectInstantAppsIntent", AiaIntent.FALLBACK_USED, AiaIntent.NUM_ENTRIES); @@ -311,7 +314,8 @@ private OverrideUrlLoadingResult handleFallbackUrl(ExternalNavigationParams para } // Launch WebAPK if it can handle the URL. try { - Intent intent = Intent.parseUri(browserFallbackUrl, Intent.URI_INTENT_SCHEME); + Intent intent = + Intent.parseUri(browserFallbackUrl.getSpec(), Intent.URI_INTENT_SCHEME); sanitizeQueryIntentActivitiesIntent(intent); List resolvingInfos = queryIntentActivities(intent); if (!isAlreadyInTargetWebApk(resolvingInfos, params) @@ -447,11 +451,10 @@ private boolean isInternalPdfDownload( * If accessing a file URL, ensure that the user has granted the necessary file access * to the app. */ - private boolean startFileIntentIfNecessary( - ExternalNavigationParams params, Intent targetIntent) { - if (params.getUrl().startsWith(UrlConstants.FILE_URL_SHORT_PREFIX) + private boolean startFileIntentIfNecessary(ExternalNavigationParams params) { + if (params.getUrl().getScheme().equals(UrlConstants.FILE_SCHEME) && shouldRequestFileAccess(params.getUrl())) { - startFileIntent(targetIntent, params.getReferrerUrl(), + startFileIntent(params.getUrl(), params.getReferrerUrl(), params.shouldCloseContentsOnOverrideUrlLoadingAndLaunchIntent()); if (DEBUG) Log.i(TAG, "Requesting filesystem access"); return true; @@ -469,14 +472,14 @@ && shouldRequestFileAccess(params.getUrl())) { */ @VisibleForTesting protected void startFileIntent( - final Intent intent, final String referrerUrl, final boolean needsToCloseTab) { + final GURL fileUri, final GURL referrerUrl, final boolean needsToCloseTab) { PermissionCallback permissionCallback = new PermissionCallback() { @Override public void onRequestPermissionsResult(String[] permissions, int[] grantResults) { if (grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED && mDelegate.hasValidTab()) { - loadUrlFromIntent(referrerUrl, intent.getDataString(), null, mDelegate, - needsToCloseTab, mDelegate.isIncognito()); + loadUrlFromIntent(referrerUrl, fileUri, null, mDelegate, needsToCloseTab, + mDelegate.isIncognito()); } else { // TODO(tedchoc): Show an indication to the user that the navigation failed // instead of silently dropping it on the floor. @@ -502,42 +505,31 @@ public void onRequestPermissionsResult(String[] permissions, int[] grantResults) * intent.) */ @VisibleForTesting - protected OverrideUrlLoadingResult clobberCurrentTab(String url, String referrerUrl) { + protected OverrideUrlLoadingResult clobberCurrentTab(GURL url, GURL referrerUrl) { int transitionType = PageTransition.LINK; final LoadUrlParams loadUrlParams = new LoadUrlParams(url, transitionType); - if (!TextUtils.isEmpty(referrerUrl)) { - Referrer referrer = new Referrer(referrerUrl, ReferrerPolicy.ALWAYS); + if (!referrerUrl.isEmpty()) { + Referrer referrer = new Referrer(referrerUrl.getSpec(), ReferrerPolicy.ALWAYS); loadUrlParams.setReferrer(referrer); } - if (mDelegate.hasValidTab()) { - // Loading URL will start a new navigation which cancels the current one - // that this clobbering is being done for. It leads to UAF. To avoid that, - // we're loading URL asynchronously. See https://crbug.com/732260. - PostTask.postTask(UiThreadTaskTraits.DEFAULT, new Runnable() { - @Override - public void run() { - mDelegate.loadUrlIfPossible(loadUrlParams); - } - }); - return OverrideUrlLoadingResult.forClobberingTab(); - } else { - assert false : "clobberCurrentTab was called with an empty tab."; - Uri uri = Uri.parse(url); - Intent intent = new Intent(Intent.ACTION_VIEW, uri); - String packageName = ContextUtils.getApplicationContext().getPackageName(); - intent.putExtra(Browser.EXTRA_APPLICATION_ID, packageName); - intent.addCategory(Intent.CATEGORY_BROWSABLE); - intent.setPackage(packageName); - startActivity(intent, false, mDelegate); - return OverrideUrlLoadingResult.forExternalIntent(); - } + assert mDelegate.hasValidTab() : "clobberCurrentTab was called with an empty tab."; + // Loading URL will start a new navigation which cancels the current one + // that this clobbering is being done for. It leads to UAF. To avoid that, + // we're loading URL asynchronously. See https://crbug.com/732260. + PostTask.postTask(UiThreadTaskTraits.DEFAULT, new Runnable() { + @Override + public void run() { + mDelegate.loadUrlIfPossible(loadUrlParams); + } + }); + return OverrideUrlLoadingResult.forClobberingTab(); } private static void loadUrlWithReferrer( - final String url, final String referrerUrl, ExternalNavigationDelegate delegate) { + final GURL url, final GURL referrerUrl, ExternalNavigationDelegate delegate) { LoadUrlParams loadUrlParams = new LoadUrlParams(url, PageTransition.AUTO_TOPLEVEL); - if (!TextUtils.isEmpty(referrerUrl)) { - Referrer referrer = new Referrer(referrerUrl, ReferrerPolicy.ALWAYS); + if (!referrerUrl.isEmpty()) { + Referrer referrer = new Referrer(referrerUrl.getSpec(), ReferrerPolicy.ALWAYS); loadUrlParams.setReferrer(referrer); } delegate.loadUrlIfPossible(loadUrlParams); @@ -553,29 +545,31 @@ private static void loadUrlWithReferrer( * If the url can be loaded in the current tab then we load the url there. * If the url can't be loaded in the current tab then we launch a new tab and load it there. * - * @param referrerUrl The string containing the original url from where the intent was referred. + * @param referrerUrl The url from where the intent was referred. * @param primaryUrl The primary url to load. - * @param alternateUrl The fallback url to use if the primary url is null or invalid. + * @param alternateUri The fallback url to use if the primary url is null or invalid. * @param delegate The delegate instance with this request is associated. * @param launchIncognito Whether the url should be loaded in an incognito tab. * @return true if the url is loaded in the current tab. */ - public static boolean loadUrlFromIntent(String referrerUrl, String primaryUrl, - String alternateUrl, ExternalNavigationDelegate delegate, boolean needsToCloseTab, - boolean launchIncognito) { + public static boolean loadUrlFromIntent(GURL referrerUrl, GURL primaryUrl, String alternateUri, + ExternalNavigationDelegate delegate, boolean needsToCloseTab, boolean launchIncognito) { // Check whether we should load this URL in the current tab or in a new tab. if (!delegate.supportsCreatingNewTabs() && !delegate.canLoadUrlInCurrentTab()) return false; boolean loadInNewTab = delegate.supportsCreatingNewTabs() && (!delegate.canLoadUrlInCurrentTab() || needsToCloseTab); + // TODO(https://crbug.com/783819): Pass a GURL into this function. + GURL alternateUrl = new GURL(alternateUri); + + // TODO(https://crbug.com/783819): Convert UrlUtilities functions to GURL. boolean isPrimaryUrlValid = - (primaryUrl != null) ? UrlUtilities.isAcceptedScheme(primaryUrl) : false; - boolean isAlternateUrlValid = - (alternateUrl != null) ? UrlUtilities.isAcceptedScheme(alternateUrl) : false; + (primaryUrl != null) ? UrlUtilities.isAcceptedScheme(primaryUrl.getSpec()) : false; + boolean isAlternateUrlValid = UrlUtilities.isAcceptedScheme(alternateUrl.getSpec()); if (!isPrimaryUrlValid && !isAlternateUrlValid) return false; - String url = (isPrimaryUrlValid) ? primaryUrl : alternateUrl; + GURL url = (isPrimaryUrlValid) ? primaryUrl : alternateUrl; if (loadInNewTab) { delegate.loadUrlInNewTab(url, launchIncognito); @@ -682,10 +676,8 @@ && blockExternalFormRedirectsWithoutGesture()) { * there is clear intent to complete the navigation in Chrome. */ private boolean isLinkFromChromeInternalPage(ExternalNavigationParams params) { - if (params.getReferrerUrl() == null) return false; - if (params.getReferrerUrl().startsWith(UrlConstants.CHROME_URL_PREFIX) - && (params.getUrl().startsWith(UrlConstants.HTTP_URL_PREFIX) - || params.getUrl().startsWith(UrlConstants.HTTPS_URL_PREFIX))) { + if (params.getReferrerUrl().getScheme().equals(UrlConstants.CHROME_SCHEME) + && UrlUtilities.isHttpOrHttps(params.getUrl())) { if (DEBUG) Log.i(TAG, "Link from an internal chrome:// page"); return true; } @@ -693,12 +685,11 @@ private boolean isLinkFromChromeInternalPage(ExternalNavigationParams params) { } private boolean handleWtaiMcProtocol(ExternalNavigationParams params) { - if (!params.getUrl().startsWith(WTAI_MC_URL_PREFIX)) return false; + if (!params.getUrl().getSpec().startsWith(WTAI_MC_URL_PREFIX)) return false; // wtai://wp/mc;number // number=string(phone-number) - startActivity(new Intent(Intent.ACTION_VIEW, - Uri.parse(WebView.SCHEME_TEL - + params.getUrl().substring(WTAI_MC_URL_PREFIX.length()))), + String phoneNumber = params.getUrl().getSpec().substring(WTAI_MC_URL_PREFIX.length()); + startActivity(new Intent(Intent.ACTION_VIEW, Uri.parse(WebView.SCHEME_TEL + phoneNumber)), false, mDelegate); if (DEBUG) Log.i(TAG, "wtai:// link handled"); RecordUserAction.record("Android.PhoneIntent"); @@ -706,7 +697,7 @@ private boolean handleWtaiMcProtocol(ExternalNavigationParams params) { } private boolean isUnhandledWtaiProtocol(ExternalNavigationParams params) { - if (!params.getUrl().startsWith(WTAI_URL_PREFIX)) return false; + if (!params.getUrl().getSpec().startsWith(WTAI_URL_PREFIX)) return false; if (DEBUG) Log.i(TAG, "Unsupported wtai:// link"); return true; } @@ -715,41 +706,41 @@ private boolean isUnhandledWtaiProtocol(ExternalNavigationParams params) { * The "about:", "chrome:", "chrome-native:", and "devtools:" schemes * are internal to the browser; don't want these to be dispatched to other apps. */ - private boolean hasInternalScheme( - ExternalNavigationParams params, Intent targetIntent, boolean hasIntentScheme) { - String url; - if (hasIntentScheme) { - // TODO(https://crbug.com/783819): When this function is converted to GURL, we should - // also call fixUpUrl on this user-provided URL as the fixed-up URL is what we would end - // up navigating to. - url = targetIntent.getDataString(); - if (url == null) return false; - } else { - url = params.getUrl(); + private boolean hasInternalScheme(GURL targetUrl, Intent targetIntent) { + if (isInternalScheme(targetUrl.getScheme())) { + if (DEBUG) Log.i(TAG, "Navigating to a chrome-internal page"); + return true; } - if (url.startsWith(ContentUrlConstants.ABOUT_SCHEME) - || url.startsWith(UrlConstants.CHROME_URL_SHORT_PREFIX) - || url.startsWith(UrlConstants.CHROME_NATIVE_URL_SHORT_PREFIX) - || url.startsWith(UrlConstants.DEVTOOLS_URL_SHORT_PREFIX)) { + if (UrlUtilities.hasIntentScheme(targetUrl) && targetIntent.getData() != null + && isInternalScheme(targetIntent.getData().getScheme())) { if (DEBUG) Log.i(TAG, "Navigating to a chrome-internal page"); return true; } return false; } - /** The "content:" scheme is disabled in Clank. Do not try to start an activity. */ - private boolean hasContentScheme( - ExternalNavigationParams params, Intent targetIntent, boolean hasIntentScheme) { - String url; - if (hasIntentScheme) { - url = targetIntent.getDataString(); - if (url == null) return false; + private static boolean isInternalScheme(String scheme) { + if (TextUtils.isEmpty(scheme)) return false; + return scheme.equals(ContentUrlConstants.ABOUT_SCHEME) + || scheme.equals(UrlConstants.CHROME_SCHEME) + || scheme.equals(UrlConstants.CHROME_NATIVE_SCHEME) + || scheme.equals(UrlConstants.DEVTOOLS_SCHEME); + } + + /** + * The "content:" scheme is disabled in Clank. Do not try to start an external activity, or + * load the URL in-browser. + */ + private boolean hasContentScheme(GURL targetUrl, Intent targetIntent) { + boolean hasContentScheme = false; + if (UrlUtilities.hasIntentScheme(targetUrl) && targetIntent.getData() != null) { + hasContentScheme = + UrlConstants.CONTENT_SCHEME.equals(targetIntent.getData().getScheme()); } else { - url = params.getUrl(); + hasContentScheme = UrlConstants.CONTENT_SCHEME.equals(targetUrl.getScheme()); } - if (!url.startsWith(UrlConstants.CONTENT_URL_SHORT_PREFIX)) return false; - if (DEBUG) Log.i(TAG, "Navigation to content: URL"); - return true; + if (DEBUG && hasContentScheme) Log.i(TAG, "Navigation to content: URL"); + return hasContentScheme; } /** @@ -761,9 +752,9 @@ private boolean hasContentScheme( * * [1]: https://developer.android.com/reference/android/os/FileUriExposedException */ - private boolean hasFileSchemeInIntentURI(Intent targetIntent, boolean hasIntentScheme) { + private boolean hasFileSchemeInIntentURI(GURL targetUrl, Intent targetIntent) { // We are only concerned with targetIntent that was generated due to intent:// schemes only. - if (!hasIntentScheme) return false; + if (!UrlUtilities.hasIntentScheme(targetUrl)) return false; Uri data = targetIntent.getData(); @@ -782,9 +773,10 @@ private boolean hasFileSchemeInIntentURI(Intent targetIntent, boolean hasIntentS * or similar) it is supposed to be controlling. Using a different application * that isn't expecting this (in particular YouTube) doesn't work. */ - private boolean isYoutubePairingCode(ExternalNavigationParams params) { - // TODO(https://crbug.com/1009539): Replace this regex with proper URI parsing. - if (params.getUrl().matches(".*youtube\\.com(\\/.*)?\\?(.+&)?pairingCode=[^&].+")) { + @VisibleForTesting + protected boolean isYoutubePairingCode(GURL url) { + if (url.domainIs("youtube.com") + && !TextUtils.isEmpty(UrlUtilities.getValueForKeyInQuery(url, "pairingCode"))) { if (DEBUG) Log.i(TAG, "YouTube URL with a pairing code"); return true; } @@ -811,10 +803,12 @@ private boolean externalIntentRequestsDisabledForUrl(ExternalNavigationParams pa * find the app on the market if no fallback is provided. */ private OverrideUrlLoadingResult handleUnresolvableIntent( - ExternalNavigationParams params, Intent targetIntent, String browserFallbackUrl) { + ExternalNavigationParams params, Intent targetIntent, GURL browserFallbackUrl) { // Fallback URL will be handled by the caller of shouldOverrideUrlLoadingInternal. - if (browserFallbackUrl != null) return OverrideUrlLoadingResult.forNoOverride(); - if (targetIntent.getPackage() != null) return handleWithMarketIntent(params, targetIntent); + if (!browserFallbackUrl.isEmpty()) return OverrideUrlLoadingResult.forNoOverride(); + if (targetIntent.getPackage() != null) { + return handleWithMarketIntent(params, targetIntent); + } if (DEBUG) Log.i(TAG, "Could not find an external activity to use"); return OverrideUrlLoadingResult.forNoOverride(); @@ -896,37 +890,20 @@ private boolean shouldStayWithinHost(ExternalNavigationParams params, boolean is boolean isFormSubmit, List resolvingInfos, boolean isExternalProtocol) { if (isExternalProtocol) return false; - // TODO(https://crbug.com/1009539): Replace this host parsing with a UrlUtilities or GURL - // function call. - GURL lastCommittedUrl = getLastCommittedUrl(); - String previousUriString = - lastCommittedUrl != null ? lastCommittedUrl.getSpec() : params.getReferrerUrl(); - if (previousUriString == null || (!isLink && !isFormSubmit)) return false; + GURL previousUrl = getLastCommittedUrl(); + if (previousUrl == null) previousUrl = params.getReferrerUrl(); + if (previousUrl.isEmpty() || (!isLink && !isFormSubmit)) return false; - URI currentUri; - URI previousUri; + GURL currentUrl = params.getUrl(); - try { - currentUri = new URI(params.getUrl()); - previousUri = new URI(previousUriString); - } catch (Exception e) { + if (!TextUtils.equals(currentUrl.getHost(), previousUrl.getHost())) { return false; } - if (currentUri == null || previousUri == null - || !TextUtils.equals(currentUri.getHost(), previousUri.getHost())) { - return false; - } - - Intent previousIntent; - try { - previousIntent = Intent.parseUri(previousUriString, Intent.URI_INTENT_SCHEME); - } catch (Exception e) { - return false; - } + Intent previousIntent = new Intent(Intent.ACTION_VIEW); + previousIntent.setData(Uri.parse(previousUrl.getSpec())); - if (previousIntent != null - && resolversSubsetOf(resolvingInfos, queryIntentActivities(previousIntent))) { + if (resolversSubsetOf(resolvingInfos, queryIntentActivities(previousIntent))) { if (DEBUG) Log.i(TAG, "Same host, no new resolvers"); return true; } @@ -964,7 +941,7 @@ private void prepareExternalIntent(Intent targetIntent, ExternalNavigationParams mDelegate.maybeSetWindowId(targetIntent); targetIntent.putExtra(EXTRA_EXTERNAL_NAV_PACKAGES, getSpecializedHandlers(resolvingInfos)); - if (params.getReferrerUrl() != null) { + if (!params.getReferrerUrl().isEmpty()) { mDelegate.maybeSetPendingReferrer(targetIntent, params.getReferrerUrl()); } @@ -982,7 +959,7 @@ private void prepareExternalIntent(Intent targetIntent, ExternalNavigationParams } private OverrideUrlLoadingResult handleExternalIncognitoIntent(Intent targetIntent, - ExternalNavigationParams params, String browserFallbackUrl, + ExternalNavigationParams params, GURL browserFallbackUrl, boolean shouldProxyForInstantApps) { // This intent may leave this app. Warn the user that incognito does not carry over // to external apps. @@ -1010,8 +987,8 @@ private OverrideUrlLoadingResult handleExternalIncognitoIntent(Intent targetInte * used by Instant Apps intents. * @return True if the function returned error free, false if it threw an exception. */ - private boolean startIncognitoIntent(final Intent intent, final String referrerUrl, - final String fallbackUrl, final boolean needsToCloseTab, final boolean proxy) { + private boolean startIncognitoIntent(final Intent intent, final GURL referrerUrl, + final GURL fallbackUrl, final boolean needsToCloseTab, final boolean proxy) { try { return startIncognitoIntentInternal( intent, referrerUrl, fallbackUrl, needsToCloseTab, proxy); @@ -1024,8 +1001,8 @@ private boolean startIncognitoIntent(final Intent intent, final String referrerU * Internal implementation of startIncognitoIntent(), with all the same parameters. */ @VisibleForTesting - protected boolean startIncognitoIntentInternal(final Intent intent, final String referrerUrl, - final String fallbackUrl, final boolean needsToCloseTab, final boolean proxy) { + protected boolean startIncognitoIntentInternal(final Intent intent, final GURL referrerUrl, + final GURL fallbackUrl, final boolean needsToCloseTab, final boolean proxy) { if (!mDelegate.hasValidTab()) return false; Context context = mDelegate.getContext(); if (ContextUtils.activityFromContext(context) == null) return false; @@ -1142,7 +1119,7 @@ private boolean launchExternalIntent(Intent targetIntent, boolean shouldProxyFor // This will handle external navigations only for intent meant for Autofill Assistant. private boolean handleWithAutofillAssistant( - ExternalNavigationParams params, Intent targetIntent, String browserFallbackUrl) { + ExternalNavigationParams params, Intent targetIntent, GURL browserFallbackUrl) { if (mDelegate.isIntentToAutofillAssistant(targetIntent)) { if (mDelegate.handleWithAutofillAssistant( params, targetIntent, browserFallbackUrl, isGoogleReferrer())) { @@ -1162,8 +1139,8 @@ private boolean shouldBlockAllExternalAppLaunches(ExternalNavigationParams param } private OverrideUrlLoadingResult shouldOverrideUrlLoadingInternal( - ExternalNavigationParams params, Intent targetIntent, - @Nullable String browserFallbackUrl, MutableBoolean canLaunchExternalFallbackResult) { + ExternalNavigationParams params, Intent targetIntent, GURL browserFallbackUrl, + MutableBoolean canLaunchExternalFallbackResult) { sanitizeQueryIntentActivitiesIntent(targetIntent); // Don't allow external fallback URLs by default. canLaunchExternalFallbackResult.set(false); @@ -1176,7 +1153,7 @@ private OverrideUrlLoadingResult shouldOverrideUrlLoadingInternal( return OverrideUrlLoadingResult.forNoOverride(); } - boolean isExternalProtocol = !UrlUtilities.isAcceptedScheme(params.getUrl()); + boolean isExternalProtocol = !UrlUtilities.isAcceptedScheme(params.getUrl().getSpec()); if (isInternalPdfDownload(isExternalProtocol, params)) { return OverrideUrlLoadingResult.forNoOverride(); @@ -1184,7 +1161,7 @@ private OverrideUrlLoadingResult shouldOverrideUrlLoadingInternal( // This check should happen for reloads, navigations, etc..., which is why // it occurs before the subsequent blocks. - if (startFileIntentIfNecessary(params, targetIntent)) { + if (startFileIntentIfNecessary(params)) { return OverrideUrlLoadingResult.forAsyncAction( OverrideUrlLoadingAsyncActionType.UI_GATING_BROWSER_NAVIGATION); } @@ -1229,21 +1206,13 @@ private OverrideUrlLoadingResult shouldOverrideUrlLoadingInternal( // TODO: handle other WTAI schemes. if (isUnhandledWtaiProtocol(params)) return OverrideUrlLoadingResult.forNoOverride(); - boolean hasIntentScheme = params.getUrl().startsWith(UrlConstants.INTENT_URL_SHORT_PREFIX) - || params.getUrl().startsWith(UrlConstants.APP_INTENT_URL_SHORT_PREFIX); - if (hasInternalScheme(params, targetIntent, hasIntentScheme)) { + if (hasInternalScheme(params.getUrl(), targetIntent) + || hasContentScheme(params.getUrl(), targetIntent) + || hasFileSchemeInIntentURI(params.getUrl(), targetIntent)) { return OverrideUrlLoadingResult.forNoOverride(); } - if (hasContentScheme(params, targetIntent, hasIntentScheme)) { - return OverrideUrlLoadingResult.forNoOverride(); - } - - if (hasFileSchemeInIntentURI(targetIntent, hasIntentScheme)) { - return OverrideUrlLoadingResult.forNoOverride(); - } - - if (isYoutubePairingCode(params)) return OverrideUrlLoadingResult.forNoOverride(); + if (isYoutubePairingCode(params.getUrl())) return OverrideUrlLoadingResult.forNoOverride(); if (shouldStayInIncognito(params, isExternalProtocol)) { return OverrideUrlLoadingResult.forNoOverride(); @@ -1261,7 +1230,7 @@ private OverrideUrlLoadingResult shouldOverrideUrlLoadingInternal( return handleUnresolvableIntent(params, targetIntent, browserFallbackUrl); } - if (browserFallbackUrl != null) targetIntent.removeExtra(EXTRA_BROWSER_FALLBACK_URL); + if (!browserFallbackUrl.isEmpty()) targetIntent.removeExtra(EXTRA_BROWSER_FALLBACK_URL); boolean hasSpecializedHandler = countSpecializedHandlers(resolvingInfos) > 0; if (!isExternalProtocol && !hasSpecializedHandler) { @@ -1356,8 +1325,8 @@ private OverrideUrlLoadingResult sendIntentToMarket( intent.addCategory(Intent.CATEGORY_BROWSABLE); intent.setPackage("com.android.vending"); intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); - if (params.getReferrerUrl() != null) { - intent.putExtra(Intent.EXTRA_REFERRER, Uri.parse(params.getReferrerUrl())); + if (!params.getReferrerUrl().isEmpty()) { + intent.putExtra(Intent.EXTRA_REFERRER, Uri.parse(params.getReferrerUrl().getSpec())); } if (!deviceCanHandleIntent(intent)) { @@ -1368,7 +1337,6 @@ private OverrideUrlLoadingResult sendIntentToMarket( if (params.isIncognito()) { if (!startIncognitoIntent(intent, params.getReferrerUrl(), null, - params.shouldCloseContentsOnOverrideUrlLoadingAndLaunchIntent(), false)) { if (DEBUG) Log.i(TAG, "Failed to show incognito alert dialog."); return OverrideUrlLoadingResult.forNoOverride(); @@ -1387,13 +1355,12 @@ private OverrideUrlLoadingResult sendIntentToMarket( * 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. */ - private Pair maybeGetPlayStoreAppIdAndReferrer(String url) { - Uri uri = Uri.parse(url); - if (PLAY_HOSTNAME.equals(uri.getHost()) && uri.getPath() != null - && uri.getPath().startsWith(PLAY_APP_PATH) - && !TextUtils.isEmpty(uri.getQueryParameter(PLAY_PACKAGE_PARAM))) { - return new Pair(uri.getQueryParameter(PLAY_PACKAGE_PARAM), - uri.getQueryParameter(PLAY_REFERRER_PARAM)); + private Pair maybeGetPlayStoreAppIdAndReferrer(GURL url) { + if (PLAY_HOSTNAME.equals(url.getHost()) && url.getPath().startsWith(PLAY_APP_PATH)) { + String playPackage = UrlUtilities.getValueForKeyInQuery(url, PLAY_PACKAGE_PARAM); + if (TextUtils.isEmpty(playPackage)) return null; + return new Pair( + playPackage, UrlUtilities.getValueForKeyInQuery(url, PLAY_REFERRER_PARAM)); } return null; } @@ -1485,8 +1452,8 @@ private static boolean intentResolutionMatches(Intent intent, Intent other) { * @return Whether the URL is a file download. */ @VisibleForTesting - boolean isPdfDownload(String url) { - String fileExtension = MimeTypeMap.getFileExtensionFromUrl(url); + boolean isPdfDownload(GURL url) { + String fileExtension = MimeTypeMap.getFileExtensionFromUrl(url.getSpec()); if (TextUtils.isEmpty(fileExtension)) return false; return PDF_EXTENSION.equals(fileExtension); @@ -1761,15 +1728,13 @@ protected GURL getLastCommittedUrl() { * @return Whether we should block the navigation and request file access before proceeding. */ @VisibleForTesting - protected boolean shouldRequestFileAccess(String url) { + protected boolean shouldRequestFileAccess(GURL url) { // If the tab is null, then do not attempt to prompt for access. if (!mDelegate.hasValidTab()) return false; - + assert url.getScheme().equals(UrlConstants.FILE_SCHEME); // If the url points inside of Chromium's data directory, no permissions are necessary. // This is required to prevent permission prompt when uses wants to access offline pages. - if (url.startsWith(UrlConstants.FILE_URL_PREFIX + PathUtils.getDataDirectory())) { - return false; - } + if (url.getPath().startsWith(PathUtils.getDataDirectory())) return false; return !mDelegate.getWindowAndroid().hasPermission(permission.READ_EXTERNAL_STORAGE) && mDelegate.getWindowAndroid().canRequestPermission( @@ -1777,9 +1742,10 @@ protected boolean shouldRequestFileAccess(String url) { } @Nullable - private String getReferrerUrl() { - // TODO (thildebr): Investigate whether or not we can use getLastCommittedUrl() instead of - // the NavigationController. + // TODO(https://crbug.com/1194721): Investigate whether or not we can use + // getLastCommittedUrl() instead of the NavigationController. Or maybe we can just replace this + // with ExternalNavigationParams#getReferrerUrl? + private GURL getReferrerUrl() { if (!mDelegate.hasValidTab() || mDelegate.getWebContents() == null) return null; NavigationController nController = mDelegate.getWebContents().getNavigationController(); @@ -1789,7 +1755,7 @@ private String getReferrerUrl() { NavigationEntry entry = nController.getEntryAtIndex(index); if (entry == null) return null; - return entry.getUrl().getSpec(); + return entry.getUrl(); } /** @@ -1797,16 +1763,16 @@ private String getReferrerUrl() { */ @VisibleForTesting protected boolean isSerpReferrer() { - String referrerUrl = getReferrerUrl(); - if (referrerUrl == null) return false; + GURL referrerUrl = getReferrerUrl(); + if (referrerUrl == null || referrerUrl.isEmpty()) return false; - return UrlUtilitiesJni.get().isGoogleSearchUrl(referrerUrl); + return UrlUtilitiesJni.get().isGoogleSearchUrl(referrerUrl.getSpec()); } private boolean isGoogleReferrer() { - String referrerUrl = getReferrerUrl(); - if (referrerUrl == null) return false; + GURL referrerUrl = getReferrerUrl(); + if (referrerUrl == null || referrerUrl.isEmpty()) return false; - return UrlUtilitiesJni.get().isGoogleSubDomainUrl(referrerUrl); + return UrlUtilitiesJni.get().isGoogleSubDomainUrl(referrerUrl.getSpec()); } } diff --git a/components/external_intents/android/java/src/org/chromium/components/external_intents/ExternalNavigationParams.java b/components/external_intents/android/java/src/org/chromium/components/external_intents/ExternalNavigationParams.java index 5c22f607db8003..701e0f9cde8314 100644 --- a/components/external_intents/android/java/src/org/chromium/components/external_intents/ExternalNavigationParams.java +++ b/components/external_intents/android/java/src/org/chromium/components/external_intents/ExternalNavigationParams.java @@ -6,6 +6,7 @@ import androidx.annotation.Nullable; +import org.chromium.url.GURL; import org.chromium.url.Origin; /** @@ -13,13 +14,13 @@ */ public class ExternalNavigationParams { /** The URL which we are navigating to. */ - private final String mUrl; + private final GURL mUrl; /** Whether we are currently in an incognito context. */ private final boolean mIsIncognito; /** The referrer URL for the current navigation. */ - private final String mReferrerUrl; + private final GURL mReferrerUrl; /** The page transition type for the current navigation. */ private final int mPageTransition; @@ -70,7 +71,7 @@ public class ExternalNavigationParams { */ private Origin mInitiatorOrigin; - private ExternalNavigationParams(String url, boolean isIncognito, String referrerUrl, + private ExternalNavigationParams(GURL url, boolean isIncognito, GURL referrerUrl, int pageTransition, boolean isRedirect, boolean appMustBeInForeground, RedirectHandler redirectHandler, boolean openInNewTab, boolean isBackgroundTabNavigation, boolean intentLaunchesAllowedInBackgroundTabs, @@ -78,9 +79,10 @@ private ExternalNavigationParams(String url, boolean isIncognito, String referre boolean shouldCloseContentsOnOverrideUrlLoadingAndLaunchIntent, boolean isRendererInitiated, @Nullable Origin initiatorOrigin) { mUrl = url; + assert mUrl != null; mIsIncognito = isIncognito; mPageTransition = pageTransition; - mReferrerUrl = referrerUrl; + mReferrerUrl = (referrerUrl == null) ? GURL.emptyGURL() : referrerUrl; mIsRedirect = isRedirect; mApplicationMustBeInForeground = appMustBeInForeground; mRedirectHandler = redirectHandler; @@ -97,7 +99,7 @@ private ExternalNavigationParams(String url, boolean isIncognito, String referre } /** @return The URL to potentially open externally. */ - public String getUrl() { + public GURL getUrl() { return mUrl; } @@ -107,7 +109,7 @@ public boolean isIncognito() { } /** @return The referrer URL. */ - public String getReferrerUrl() { + public GURL getReferrerUrl() { return mReferrerUrl; } @@ -193,13 +195,13 @@ public Origin getInitiatorOrigin() { /** The builder for {@link ExternalNavigationParams} objects. */ public static class Builder { /** The URL which we are navigating to. */ - private String mUrl; + private GURL mUrl; /** Whether we are currently in an incognito context. */ private boolean mIsIncognito; /** The referrer URL for the current navigation. */ - private String mReferrerUrl; + private GURL mReferrerUrl; /** The page transition type for the current navigation. */ private int mPageTransition; @@ -250,12 +252,12 @@ public static class Builder { */ private Origin mInitiatorOrigin; - public Builder(String url, boolean isIncognito) { + public Builder(GURL url, boolean isIncognito) { mUrl = url; mIsIncognito = isIncognito; } - public Builder(String url, boolean isIncognito, String referrer, int pageTransition, + public Builder(GURL url, boolean isIncognito, GURL referrer, int pageTransition, boolean isRedirect) { mUrl = url; mIsIncognito = isIncognito; diff --git a/components/external_intents/android/java/src/org/chromium/components/external_intents/InterceptNavigationDelegateImpl.java b/components/external_intents/android/java/src/org/chromium/components/external_intents/InterceptNavigationDelegateImpl.java index 645fc016267948..44d7cdb783cfce 100644 --- a/components/external_intents/android/java/src/org/chromium/components/external_intents/InterceptNavigationDelegateImpl.java +++ b/components/external_intents/android/java/src/org/chromium/components/external_intents/InterceptNavigationDelegateImpl.java @@ -84,9 +84,7 @@ public boolean shouldIgnoreNewTab(GURL url, boolean incognito) { } ExternalNavigationParams params = - new ExternalNavigationParams.Builder(url.getSpec(), incognito) - .setOpenInNewTab(true) - .build(); + new ExternalNavigationParams.Builder(url, incognito).setOpenInNewTab(true).build(); mLastOverrideUrlLoadingResultType = mExternalNavHandler.shouldOverrideUrlLoading(params).getResultType(); return mLastOverrideUrlLoadingResultType @@ -179,11 +177,9 @@ public ExternalNavigationParams.Builder buildExternalNavigationParams( mClient.wasTabLaunchedFromLongPressInBackground() && shouldCloseTab; // http://crbug.com/448977: If a new tab is closed by this overriding, we should open an // Intent in a new tab when Chrome receives it again. - // TODO(https://crbug.com/783819): Covert ExternalNavigationParams to GURL. return new ExternalNavigationParams - .Builder(navigationParams.url.getSpec(), mClient.isIncognito(), - navigationParams.referrer.getSpec(), navigationParams.pageTransitionType, - navigationParams.isRedirect) + .Builder(navigationParams.url, mClient.isIncognito(), navigationParams.referrer, + navigationParams.pageTransitionType, navigationParams.isRedirect) .setApplicationMustBeInForeground(true) .setRedirectHandler(redirectHandler) .setOpenInNewTab(shouldCloseTab) diff --git a/components/external_intents/android/javatests/src/org/chromium/components/external_intents/ExternalNavigationHandlerTest.java b/components/external_intents/android/javatests/src/org/chromium/components/external_intents/ExternalNavigationHandlerTest.java index 058724f668643f..69bceec6ed4404 100644 --- a/components/external_intents/android/javatests/src/org/chromium/components/external_intents/ExternalNavigationHandlerTest.java +++ b/components/external_intents/android/javatests/src/org/chromium/components/external_intents/ExternalNavigationHandlerTest.java @@ -513,41 +513,30 @@ public void testIntentScheme() { public void testYouTubePairingCode() { mDelegate.add(new IntentActivity(YOUTUBE_MOBILE_URL, YOUTUBE_PACKAGE_NAME)); + String mobileUrl = "http://m.youtube.com/watch?v=1234&pairingCode=5678"; int transitionTypeIncomingIntent = PageTransition.LINK | PageTransition.FROM_API; - final String[] goodUrls = {"http://m.youtube.com/watch?v=1234&pairingCode=5678", - "youtube.com?pairingCode=xyz", "youtube.com/tv?pairingCode=xyz", - "youtube.com/watch?v=1234&version=3&autohide=1&pairingCode=xyz", - "youtube.com/watch?v=1234&pairingCode=xyz&version=3&autohide=1"}; - final String[] badUrls = {"youtube.com.foo.com/tv?pairingCode=xyz", - "youtube.com.foo.com?pairingCode=xyz", - "youtube.com/watch?v=tEsT&version=3&autohide=1&pairingCode=", - "youtube.com&pairingCode=xyz", - "youtube.com/watch?v=tEsT?version=3&pairingCode=&autohide=1"}; + final String[] goodUrls = {mobileUrl, "http://youtube.com?pairingCode=xyz", + "http://youtube.com/tv?pairingCode=xyz", + "http://youtube.com/watch?v=1234&version=3&autohide=1&pairingCode=xyz", + "http://youtube.com/watch?v=1234&pairingCode=xyz&version=3&autohide=1"}; + final String[] badUrls = {"http://youtube.com.foo.com/tv?pairingCode=xyz", + "http://youtube.com.foo.com?pairingCode=xyz", "http://youtube.com&pairingCode=xyz", + "http://youtube.com/watch?v=1234#pairingCode=xyz"}; // Make sure we don't override when faced with valid pairing code URLs. for (String url : goodUrls) { - // http://crbug/386600 - it makes no sense to switch activities for pairing code URLs. - checkUrl(url).withIsRedirect(true).expecting( - OverrideUrlLoadingResultType.NO_OVERRIDE, IGNORE); - - checkUrl(url) - .withPageTransition(transitionTypeIncomingIntent) - .withIsRedirect(true) - .expecting(OverrideUrlLoadingResultType.NO_OVERRIDE, IGNORE); + Assert.assertTrue(mUrlHandler.isYoutubePairingCode(new GURL(url))); } - - // The pairing code URL regex shouldn't cause NO_OVERRIDE on invalid URLs. for (String url : badUrls) { - checkUrl(url).withIsRedirect(true).expecting( - OverrideUrlLoadingResultType.OVERRIDE_WITH_EXTERNAL_INTENT, - START_OTHER_ACTIVITY); - - checkUrl(url) - .withPageTransition(transitionTypeIncomingIntent) - .withIsRedirect(true) - .expecting(OverrideUrlLoadingResultType.OVERRIDE_WITH_EXTERNAL_INTENT, - START_OTHER_ACTIVITY); + Assert.assertFalse(mUrlHandler.isYoutubePairingCode(new GURL(url))); } + // http://crbug/386600 - it makes no sense to switch activities for pairing code URLs. + checkUrl(mobileUrl).withIsRedirect(true).expecting( + OverrideUrlLoadingResultType.NO_OVERRIDE, IGNORE); + checkUrl(mobileUrl) + .withPageTransition(transitionTypeIncomingIntent) + .withIsRedirect(true) + .expecting(OverrideUrlLoadingResultType.NO_OVERRIDE, IGNORE); } @Test @@ -1237,7 +1226,7 @@ public void testCreatesIntentsToOpenInNewTab() { mUrlHandler = new ExternalNavigationHandlerForTesting(mDelegate); ExternalNavigationParams params = - new ExternalNavigationParams.Builder(YOUTUBE_MOBILE_URL, false) + new ExternalNavigationParams.Builder(new GURL(YOUTUBE_MOBILE_URL), false) .setOpenInNewTab(true) .build(); OverrideUrlLoadingResult result = mUrlHandler.shouldOverrideUrlLoading(params); @@ -1337,7 +1326,7 @@ public void testIntentToPdfFileOpensApp() { @Test @SmallTest public void testUsafeIntentFlagsFiltered() { - checkUrl("intent://#Intent;package=com.test.package;launchFlags=0x7FFFFFFF;end;") + checkUrl("intent:#Intent;package=com.test.package;launchFlags=0x7FFFFFFF;end;") .expecting(OverrideUrlLoadingResultType.OVERRIDE_WITH_EXTERNAL_INTENT, START_OTHER_ACTIVITY); Assert.assertEquals(ExternalNavigationHandler.ALLOWED_INTENT_FLAGS, @@ -1354,7 +1343,7 @@ public void testIntentWithFileSchemeFiltered() { @Test @SmallTest public void testIntentWithNoSchemeLaunched() { - checkUrl("intent://#Intent;package=com.test.package;end;") + checkUrl("intent:#Intent;package=com.test.package;end;") .expecting(OverrideUrlLoadingResultType.OVERRIDE_WITH_EXTERNAL_INTENT, START_OTHER_ACTIVITY); } @@ -1373,6 +1362,8 @@ public void testIntentWithWeirdSchemeLaunched() { checkUrl("intent://#Intent;package=com.test.package;scheme=w3irD;end;") .expecting(OverrideUrlLoadingResultType.OVERRIDE_WITH_EXTERNAL_INTENT, START_OTHER_ACTIVITY); + // Schemes on Android are case-sensitive, so ensure the scheme is passed through as-is. + Assert.assertEquals("w3irD", mDelegate.startActivityIntent.getScheme()); } @Test @@ -1399,7 +1390,7 @@ public void testIntentWithMissingReferrer() { public void testReferrerExtra() { mDelegate.add(new IntentActivity(YOUTUBE_URL, YOUTUBE_PACKAGE_NAME)); - String referrer = "http://www.google.com"; + String referrer = "http://www.google.com/"; checkUrl("http://youtube.com:90/foo/bar") .withReferrer(referrer) .withPageTransition(PageTransition.FORM_SUBMIT) @@ -1735,15 +1726,16 @@ public void testAutofillAssistantIntentWithoutFallback_InIncognito() { @SmallTest public void testIsDownload_noSystemDownloadManager() { Assert.assertTrue("pdf should be a download, no viewer in Android Chrome", - mUrlHandler.isPdfDownload("http://somesampeleurldne.com/file.pdf")); + mUrlHandler.isPdfDownload(new GURL("http://somesampeleurldne.com/file.pdf"))); Assert.assertFalse("URL is not a file, but web page", - mUrlHandler.isPdfDownload("http://somesampleurldne.com/index.html")); + mUrlHandler.isPdfDownload(new GURL("http://somesampleurldne.com/index.html"))); Assert.assertFalse("URL is not a file url", - mUrlHandler.isPdfDownload("http://somesampeleurldne.com/not.a.real.extension")); + mUrlHandler.isPdfDownload( + new GURL("http://somesampeleurldne.com/not.a.real.extension"))); Assert.assertFalse("URL is an image, can be viewed in Chrome", - mUrlHandler.isPdfDownload("http://somesampleurldne.com/image.jpg")); + mUrlHandler.isPdfDownload(new GURL("http://somesampleurldne.com/image.jpg"))); Assert.assertFalse("URL is a text file can be viewed in Chrome", - mUrlHandler.isPdfDownload("http://somesampleurldne.com/copy.txt")); + mUrlHandler.isPdfDownload(new GURL("http://somesampleurldne.com/copy.txt"))); } @Test @@ -1983,8 +1975,8 @@ public boolean blockExternalFormRedirectsWithoutGesture() { } @Override - protected boolean startIncognitoIntentInternal(Intent intent, String referrerUrl, - String fallbackUrl, boolean needsToCloseTab, boolean proxy) { + protected boolean startIncognitoIntentInternal(Intent intent, GURL referrerUrl, + GURL fallbackUrl, boolean needsToCloseTab, boolean proxy) { mStartActivityInIncognitoIntent = intent; mStartIncognitoIntentCalled = true; return true; @@ -2006,47 +1998,56 @@ protected boolean isSerpReferrer() { } @Override - protected boolean shouldRequestFileAccess(String url) { + protected boolean shouldRequestFileAccess(GURL url) { return mShouldRequestFileAccess; } @Override - protected void startFileIntent(Intent intent, String referrerUrl, boolean needsToCloseTab) { + protected void startFileIntent(GURL fileUri, GURL referrerUrl, boolean needsToCloseTab) { mStartFileIntentCalled = true; } @Override - protected OverrideUrlLoadingResult clobberCurrentTab(String url, String referrerUrl) { - mNewUrlAfterClobbering = url; - mReferrerUrlForClobbering = referrerUrl; + protected OverrideUrlLoadingResult clobberCurrentTab(GURL url, GURL referrerUrl) { + mNewUrlAfterClobbering = url.getSpec(); + mReferrerUrlForClobbering = referrerUrl.getSpec(); return OverrideUrlLoadingResult.forClobberingTab(); } + + @Override + public boolean isYoutubePairingCode(GURL url) { + return super.isYoutubePairingCode(url); + } }; private static class TestExternalNavigationDelegate implements ExternalNavigationDelegate { public List queryIntentActivities(Intent intent) { List list = new ArrayList<>(); String dataString = intent.getDataString(); - if (dataString == null) return list; - if (dataString.startsWith("http://") || dataString.startsWith("https://")) { - list.add(newResolveInfo("chrome")); - } - for (IntentActivity intentActivity : mIntentActivities) { - if (dataString.startsWith(intentActivity.urlPrefix())) { - list.add(newSpecializedResolveInfo( - intentActivity.packageName(), intentActivity)); + if (intent.getScheme() != null) { + if (dataString.startsWith("http://") || dataString.startsWith("https://")) { + list.add(newResolveInfo("chrome")); } - } - if (!list.isEmpty()) return list; + for (IntentActivity intentActivity : mIntentActivities) { + if (dataString.startsWith(intentActivity.urlPrefix())) { + list.add(newSpecializedResolveInfo( + intentActivity.packageName(), intentActivity)); + } + } + if (!list.isEmpty()) return list; - String schemeString = intent.getData().getScheme(); - boolean isMarketScheme = schemeString != null && schemeString.startsWith("market"); - if (mCanResolveActivityForMarket && isMarketScheme) { - list.add(newResolveInfo("market")); - return list; - } - if (mCanResolveActivityForExternalSchemes && !isMarketScheme) { - list.add(newResolveInfo(intent.getData().getScheme())); + String schemeString = intent.getScheme(); + boolean isMarketScheme = schemeString != null && schemeString.startsWith("market"); + if (mCanResolveActivityForMarket && isMarketScheme) { + list.add(newResolveInfo("market")); + return list; + } + if (mCanResolveActivityForExternalSchemes && !isMarketScheme) { + list.add(newResolveInfo(intent.getData().getScheme())); + } + } else if (mCanResolveActivityForExternalSchemes) { + // Scheme-less intents (eg. Action-based intents like opening Settings). + list.add(newResolveInfo("package")); } return list; } @@ -2071,7 +2072,7 @@ public boolean willAppHandleIntent(Intent intent) { } @Override - public boolean shouldDisableExternalIntentRequestsForUrl(String url) { + public boolean shouldDisableExternalIntentRequestsForUrl(GURL url) { return mShouldDisableExternalIntentRequests; } @@ -2100,7 +2101,7 @@ public void didStartActivity(Intent intent) { @Override public OverrideUrlLoadingResult handleIncognitoIntentTargetingSelf( - Intent intent, String referrerUrl, String fallbackUrl) { + Intent intent, GURL referrerUrl, GURL fallbackUrl) { handleIncognitoIntentTargetingSelfCalled = true; if (mCanLoadUrlInTab) return OverrideUrlLoadingResult.forClobberingTab(); return OverrideUrlLoadingResult.forExternalIntent(); @@ -2112,7 +2113,7 @@ public boolean supportsCreatingNewTabs() { } @Override - public void loadUrlInNewTab(final String url, final boolean launchIncognito) {} + public void loadUrlInNewTab(GURL url, final boolean launchIncognito) {} @Override public boolean canLoadUrlInCurrentTab() { @@ -2134,10 +2135,10 @@ public void loadUrlIfPossible(LoadUrlParams loadUrlParams) {} public void maybeSetWindowId(Intent intent) {} @Override - public void maybeSetPendingReferrer(Intent intent, String referrerUrl) { + public void maybeSetPendingReferrer(Intent intent, GURL referrerUrl) { // This is used in a test to check that ExternalNavigationHandler correctly passes // this data to the delegate when the referrer URL is non-null. - intent.putExtra(Intent.EXTRA_REFERRER, Uri.parse(referrerUrl)); + intent.putExtra(Intent.EXTRA_REFERRER, Uri.parse(referrerUrl.getSpec())); } @Override @@ -2164,8 +2165,8 @@ public boolean isApplicationInForeground() { } @Override - public boolean maybeLaunchInstantApp(String url, String referrerUrl, - boolean isIncomingRedirect, boolean isSerpReferrer) { + public boolean maybeLaunchInstantApp( + GURL url, GURL referrerUrl, boolean isIncomingRedirect, boolean isSerpReferrer) { return mCanHandleWithInstantApp; } @@ -2206,7 +2207,7 @@ public boolean isIntentToAutofillAssistant(Intent intent) { @Override public boolean handleWithAutofillAssistant(ExternalNavigationParams params, - Intent targetIntent, String browserFallbackUrl, boolean isGoogleReferrer) { + Intent targetIntent, GURL browserFallbackUrl, boolean isGoogleReferrer) { return mHandleWithAutofillAssistant; } @@ -2401,7 +2402,8 @@ public void expecting(@OverrideUrlLoadingResultType int expectedOverrideResult, ExternalNavigationParams params = new ExternalNavigationParams - .Builder(mUrl, mIsIncognito, mReferrerUrl, mPageTransition, mIsRedirect) + .Builder(new GURL(mUrl), mIsIncognito, new GURL(mReferrerUrl), + mPageTransition, mIsRedirect) .setApplicationMustBeInForeground(mChromeAppInForegroundRequired) .setRedirectHandler(mRedirectHandler) .setIsBackgroundTabNavigation(mIsBackgroundTabNavigation) diff --git a/content/public/android/java/src/org/chromium/content_public/browser/LoadUrlParams.java b/content/public/android/java/src/org/chromium/content_public/browser/LoadUrlParams.java index d5c3f591eb6da8..6ea6cb327aa526 100644 --- a/content/public/android/java/src/org/chromium/content_public/browser/LoadUrlParams.java +++ b/content/public/android/java/src/org/chromium/content_public/browser/LoadUrlParams.java @@ -67,6 +67,15 @@ public LoadUrlParams(GURL url) { this(url.getSpec(), PageTransition.LINK); } + /** + * Creates an instance with the given page transition type. + * @param url the url to be loaded + * @param transitionType the PageTransitionType constant corresponding to the load + */ + public LoadUrlParams(GURL url, int transitionType) { + this(url.getSpec(), transitionType); + } + /** * Creates an instance with the given page transition type. * @param url the url to be loaded diff --git a/url/android/gurl_android.cc b/url/android/gurl_android.cc index 981ad10f7be82b..861bc6f8f3f0ff 100644 --- a/url/android/gurl_android.cc +++ b/url/android/gurl_android.cc @@ -104,6 +104,16 @@ static void JNI_GURL_GetOrigin(JNIEnv* env, InitFromGURL(env, gurl->GetOrigin(), target); } +static jboolean JNI_GURL_DomainIs(JNIEnv* env, + const JavaParamRef& j_spec, + jboolean is_valid, + jlong parsed_ptr, + const JavaParamRef& j_domain) { + std::unique_ptr gurl = FromJavaGURL(env, j_spec, is_valid, parsed_ptr); + const std::string& domain = ConvertJavaStringToUTF8(env, j_domain); + return gurl->DomainIs(domain); +} + static void JNI_GURL_Init(JNIEnv* env, const base::android::JavaParamRef& uri, const base::android::JavaParamRef& target) { diff --git a/url/android/java/src/org/chromium/url/GURL.java b/url/android/java/src/org/chromium/url/GURL.java index 88c36ac3a730db..75f289d05e49d9 100644 --- a/url/android/java/src/org/chromium/url/GURL.java +++ b/url/android/java/src/org/chromium/url/GURL.java @@ -248,6 +248,13 @@ protected void getOriginInternal(GURL target) { GURLJni.get().getOrigin(mSpec, mIsValid, mParsed.toNativeParsed(), target); } + /** + * See native GURL::DomainIs(). + */ + public boolean domainIs(String domain) { + return GURLJni.get().domainIs(mSpec, mIsValid, mParsed.toNativeParsed(), domain); + } + @Override public final int hashCode() { return mSpec.hashCode(); @@ -330,6 +337,11 @@ interface Natives { */ void getOrigin(String spec, boolean isValid, long nativeParsed, GURL target); + /** + * Reconstructs the native GURL for this Java GURL, and calls GURL.DomainIs. + */ + boolean domainIs(String spec, boolean isValid, long nativeParsed, String domain); + /** * Reconstructs the native GURL for this Java GURL, returning its native pointer. */ diff --git a/url/android/javatests/src/org/chromium/url/GURLJavaTest.java b/url/android/javatests/src/org/chromium/url/GURLJavaTest.java index e567a1f9e13b66..55c154912a1b59 100644 --- a/url/android/javatests/src/org/chromium/url/GURLJavaTest.java +++ b/url/android/javatests/src/org/chromium/url/GURLJavaTest.java @@ -271,4 +271,20 @@ public void testCorruptedSerializations() { url = GURL.deserialize(corruptedVersion); Assert.assertEquals(GURL.emptyGURL(), url); } + + // Test that domainIs is hooked up correctly. + @SmallTest + @Test + public void testDomainIs() { + GURL url1 = new GURL("https://www.google.com"); + GURL url2 = new GURL("https://www.notgoogle.com"); + + Assert.assertTrue(url1.domainIs("com")); + Assert.assertTrue(url2.domainIs("com")); + Assert.assertTrue(url1.domainIs("google.com")); + Assert.assertFalse(url2.domainIs("google.com")); + + Assert.assertTrue(url1.domainIs("www.google.com")); + Assert.assertFalse(url1.domainIs("images.google.com")); + } } diff --git a/weblayer/browser/java/org/chromium/weblayer_private/ExternalNavigationDelegateImpl.java b/weblayer/browser/java/org/chromium/weblayer_private/ExternalNavigationDelegateImpl.java index 39afc1bc9f5040..6e89ea9774af6b 100644 --- a/weblayer/browser/java/org/chromium/weblayer_private/ExternalNavigationDelegateImpl.java +++ b/weblayer/browser/java/org/chromium/weblayer_private/ExternalNavigationDelegateImpl.java @@ -19,6 +19,7 @@ import org.chromium.content_public.browser.LoadUrlParams; import org.chromium.content_public.browser.WebContents; import org.chromium.ui.base.WindowAndroid; +import org.chromium.url.GURL; import org.chromium.url.Origin; /** @@ -48,7 +49,7 @@ public boolean willAppHandleIntent(Intent intent) { } @Override - public boolean shouldDisableExternalIntentRequestsForUrl(String url) { + public boolean shouldDisableExternalIntentRequestsForUrl(GURL url) { return false; } @@ -90,7 +91,7 @@ public void didStartActivity(Intent intent) {} // This method should never be invoked as WebLayer does not handle incoming intents. @Override public OverrideUrlLoadingResult handleIncognitoIntentTargetingSelf( - final Intent intent, final String referrerUrl, final String fallbackUrl) { + final Intent intent, final GURL referrerUrl, final GURL fallbackUrl) { assert false; return OverrideUrlLoadingResult.forNoOverride(); } @@ -121,7 +122,7 @@ public boolean supportsCreatingNewTabs() { } @Override - public void loadUrlInNewTab(final String url, final boolean launchIncognito) { + public void loadUrlInNewTab(final GURL url, final boolean launchIncognito) { // Should never be invoked based on the implementation of supportsCreatingNewTabs(). assert false; } @@ -153,7 +154,7 @@ public void maybeSetRequestMetadata(Intent intent, boolean hasUserGesture, @Override // This is relevant only if the intent ends up being handled by this app, which does not happen // for WebLayer. - public void maybeSetPendingReferrer(Intent intent, String referrerUrl) {} + public void maybeSetPendingReferrer(Intent intent, GURL referrerUrl) {} @Override // This is relevant only if the intent ends up being handled by this app, which does not happen @@ -162,7 +163,7 @@ public void maybeSetPendingIncognitoUrl(Intent intent) {} @Override public boolean maybeLaunchInstantApp( - String url, String referrerUrl, boolean isIncomingRedirect, boolean isSerpReferrer) { + GURL url, GURL referrerUrl, boolean isIncomingRedirect, boolean isSerpReferrer) { return false; } @@ -204,7 +205,7 @@ public boolean isIntentToAutofillAssistant(Intent intent) { @Override public boolean handleWithAutofillAssistant(ExternalNavigationParams params, Intent targetIntent, - String browserFallbackUrl, boolean isGoogleReferrer) { + GURL browserFallbackUrl, boolean isGoogleReferrer) { return false; } diff --git a/weblayer/browser/java/org/chromium/weblayer_private/TabImpl.java b/weblayer/browser/java/org/chromium/weblayer_private/TabImpl.java index 5b916fb81a8f33..b2c4a1c56da391 100644 --- a/weblayer/browser/java/org/chromium/weblayer_private/TabImpl.java +++ b/weblayer/browser/java/org/chromium/weblayer_private/TabImpl.java @@ -501,6 +501,11 @@ public void loadUrl(LoadUrlParams loadUrlParams) { String url = loadUrlParams.getUrl(); if (url == null || url.isEmpty()) return; + // TODO(https://crbug.com/783819): Don't fix up all URLs. Documentation on FixupURL + // explicitly says not to use it on URLs coming from untrustworthy sources, like other apps. + // Once migrations of Java code to GURL are complete and incoming URLs are converted to + // GURLs at their source, we can make decisions of whether or not to fix up GURLs on a + // case-by-case basis based on trustworthiness of the incoming URL. GURL fixedUrl = UrlFormatter.fixupUrl(url); if (!fixedUrl.isValid()) return;