From 9286eb828b8e02637eb59ee32d38fdb5a7fa7585 Mon Sep 17 00:00:00 2001
From: Lily Chen <chlily@chromium.org>
Date: Tue, 20 Jan 2026 11:58:34 -0800
Subject: [PATCH] Android downloads UI: Do not display invalid or non-standard URLs

This fixes a util function used for formatting a download URL to display
to the user in various downloads UI surfaces in Clank.

In crrev.com/c/6701188, we previously fixed the fallback logic that
kicks in when formatUrlForSecurityDisplay() produces a string that is
too long. For certain non-standard URL schemes that do not have a host,
that CL made us fall back to the text of the URL itself rather than
trying to parse out an eTLD+1 (which is not meaningful when there is no
host).

However, that previous CL did not account for edge cases in which
formatUrlForSecurityDisplay() itself may produce misleading results for
non-standard URL schemes.

In this CL, we stop attempting to even call
formatUrlForSecurityDisplay() for such non-standard URLs, and we do not
display any fallback. Instead, we always omit the download URL/origin if
it is an opaque origin. Even if there may be some textual representation
of the URL available, in these cases it is not necessarily meaningful to
the user and may allow misleading strings to be injected, so prefer to
omit it.

Adds unit tests for this formatting util function.

Change-Id: I2231d1527def0480710a7120ae861ac3556fd0c0
Bug: 473118648
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7476846
Commit-Queue: Lily Chen <chlily@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1571795}
---

diff --git a/components/browser_ui/util/android/BUILD.gn b/components/browser_ui/util/android/BUILD.gn
index 2843120..a331eaa 100644
--- a/components/browser_ui/util/android/BUILD.gn
+++ b/components/browser_ui/util/android/BUILD.gn
@@ -77,6 +77,7 @@
     "java/src/org/chromium/components/browser_ui/util/BitmapCacheTest.java",
     "java/src/org/chromium/components/browser_ui/util/ComposedBrowserControlsVisibilityDelegateTest.java",
     "java/src/org/chromium/components/browser_ui/util/DimensionCompatUnitTest.java",
+    "java/src/org/chromium/components/browser_ui/util/DownloadUtilsTest.java",
     "java/src/org/chromium/components/browser_ui/util/TimeTextResolverUnitTest.java",
   ]
   deps = [
@@ -86,9 +87,12 @@
     "//base:base_junit_test_support",
     "//base/test:test_support_java",
     "//cc:cc_java",
+    "//components/embedder_support/android:util_java",
+    "//components/url_formatter/android:url_formatter_java",
     "//content/public/android:content_java",
     "//third_party/junit",
     "//third_party/mockito:mockito_java",
+    "//url:gurl_java",
   ]
 }
 
diff --git a/components/browser_ui/util/android/java/src/org/chromium/components/browser_ui/util/DownloadUtils.java b/components/browser_ui/util/android/java/src/org/chromium/components/browser_ui/util/DownloadUtils.java
index c4821dff..28593b00 100644
--- a/components/browser_ui/util/android/java/src/org/chromium/components/browser_ui/util/DownloadUtils.java
+++ b/components/browser_ui/util/android/java/src/org/chromium/components/browser_ui/util/DownloadUtils.java
@@ -19,6 +19,7 @@
 import org.chromium.components.url_formatter.SchemeDisplay;
 import org.chromium.components.url_formatter.UrlFormatter;
 import org.chromium.url.GURL;
+import org.chromium.url.Origin;
 
 /** A class containing some utility static methods. */
 @NullMarked
@@ -93,6 +94,9 @@
      * Adjusts a URL for display to the user in a text view subject to char limits. Could elide
      * parts the URL if it is too long as per readability and security aspects.
      *
+     * <p>This returns null for invalid or non-standard URLs, or if there is no suitable way to
+     * format the URL within the character limit.
+     *
      * @param url The full URL.
      * @param limit Character limit.
      * @return The text to display, or null if the input was invalid or cannot be shortened enough.
@@ -101,6 +105,17 @@
             @Nullable GURL url, int limit) {
         if (GURL.isEmptyOrInvalid(url)) return null;
 
+        // Don't attempt to format and display invalid or non-standard URLs which have opaque
+        // origins. For such URLs (e.g. "data:" scheme URLs), it is not quite meaningful to display
+        // (parts of) the URL in the UI, unlike normal "webby" schemes ("http" and "https") where an
+        // eTLD+1 may be meaningfully extracted from the host part to help the user make a security
+        // decision about the download.
+        // TODO(chlily): Consider exposing url::Origin::Resolve() to JNI and using it to get the
+        // precursor origin to display to the user in cases where the origin itself is opaque.
+        if (Origin.create(url).isOpaque()) {
+            return null;
+        }
+
         String formattedUrl =
                 UrlFormatter.formatUrlForSecurityDisplay(url, SchemeDisplay.OMIT_HTTP_AND_HTTPS);
         if (!TextUtils.isEmpty(formattedUrl) && formattedUrl.length() <= limit) {
@@ -110,12 +125,12 @@
         // The formatted URL is unsuitable. One possible fallback is eTLD+1, but we should be
         // careful to only parse for eTLD+1 if the origin has a host portion (some URL schemes
         // don't).
-        GURL origin = url.getOrigin();
+        GURL originAsUrl = url.getOrigin();
         String fallback =
-                !GURL.isEmptyOrInvalid(origin) && !origin.getHost().isEmpty()
+                !GURL.isEmptyOrInvalid(originAsUrl) && !originAsUrl.getHost().isEmpty()
                         ? UrlUtilities.getDomainAndRegistry(
-                                origin.getSpec(), /* includePrivateRegistries= */ true)
-                        : origin.getPossiblyInvalidSpec();
+                                originAsUrl.getSpec(), /* includePrivateRegistries= */ true)
+                        : originAsUrl.getPossiblyInvalidSpec();
         if (!TextUtils.isEmpty(fallback) && fallback.length() <= limit) {
             return fallback;
         }
diff --git a/components/browser_ui/util/android/java/src/org/chromium/components/browser_ui/util/DownloadUtilsTest.java b/components/browser_ui/util/android/java/src/org/chromium/components/browser_ui/util/DownloadUtilsTest.java
new file mode 100644
index 0000000..5a73f63
--- /dev/null
+++ b/components/browser_ui/util/android/java/src/org/chromium/components/browser_ui/util/DownloadUtilsTest.java
@@ -0,0 +1,161 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.components.browser_ui.util;
+
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
+
+import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.components.embedder_support.util.UrlUtilities;
+import org.chromium.components.embedder_support.util.UrlUtilitiesJni;
+import org.chromium.components.url_formatter.SchemeDisplay;
+import org.chromium.components.url_formatter.UrlFormatter;
+import org.chromium.url.GURL;
+
+/** Unit tests for {@link DownloadUtils}. */
+@RunWith(BaseRobolectricTestRunner.class)
+public class DownloadUtilsTest {
+    @Rule public MockitoRule mMockitoRule = MockitoJUnit.rule();
+
+    @Mock private UrlUtilities.Natives mUrlUtilitiesJniMock;
+
+    @Before
+    public void setUp() {
+        UrlUtilitiesJni.setInstanceForTesting(mUrlUtilitiesJniMock);
+        GURL.ensureNativeInitializedForGURL();
+    }
+
+    @Test
+    public void testFormatUrlForDisplayInNotification_InvalidOrEmptyUrl() {
+        Assert.assertNull(DownloadUtils.formatUrlForDisplayInNotification(null, 100));
+        GURL emptyUrl = new GURL("");
+        Assert.assertNull(DownloadUtils.formatUrlForDisplayInNotification(emptyUrl, 100));
+        GURL invalidUrl = new GURL("foo");
+        Assert.assertNull(DownloadUtils.formatUrlForDisplayInNotification(invalidUrl, 100));
+    }
+
+    @Test
+    public void testFormatUrlForDisplayInNotification_OpaqueOrigin() {
+        GURL dataUrl = new GURL("data:text/plain,Hello");
+        Assert.assertNull(DownloadUtils.formatUrlForDisplayInNotification(dataUrl, 100));
+        GURL bogusSchemeUrl = new GURL("asdf://foo.bar.test");
+        Assert.assertNull(DownloadUtils.formatUrlForDisplayInNotification(bogusSchemeUrl, 100));
+    }
+
+    @Test
+    public void testFormatUrlForDisplayInNotification_FitsUnderLimit() {
+        String urlSpec = "https://example.test/path?foo=bar";
+        GURL url = new GURL(urlSpec);
+        useMockGetDomainAndRegistry(url);
+        String expectedFromUrlFormatter = "example.test";
+
+        String urlFormatterOutput =
+                UrlFormatter.formatUrlForSecurityDisplay(url, SchemeDisplay.OMIT_HTTP_AND_HTTPS);
+        Assert.assertEquals(expectedFromUrlFormatter, urlFormatterOutput);
+
+        String formatted = DownloadUtils.formatUrlForDisplayInNotification(url, urlSpec.length());
+        Assert.assertEquals(expectedFromUrlFormatter, formatted);
+    }
+