Change URL formatter for short URLs in PageInfo

UrlFormatter.formatUrlForSecurityDisplay is too strict and displays
some valid URLs in punycode.
Add new url formatter with the difference that non-dangerous non-ascii
characters are displayed correctly and www subdomains are omitted

Bug: 1077766
Change-Id: I896fbf0a4d89df36c5fc882afaf09d1366f5423b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2532294Reviewed-by: Chris Thompson <cthomp@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826697}

components/page_info/android/java/src/org/chromium/components/page_info/PageInfoController.java

@@ -42,7 +42,6 @@ import org.chromium.components.page_info.PageInfoView.ConnectionInfoParams;
 import org.chromium.components.page_info.PageInfoView.PageInfoViewParams;
 import org.chromium.components.security_state.ConnectionSecurityLevel;
 import org.chromium.components.security_state.SecurityStateModel;
-import org.chromium.components.url_formatter.SchemeDisplay;
 import org.chromium.components.url_formatter.UrlFormatter;
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.content_public.browser.WebContentsObserver;
@@ -257,8 +256,8 @@ public class PageInfoController implements PageInfoMainController, ModalDialogPr
             PageInfoContainer.Params containerParams = new PageInfoContainer.Params();
             containerParams.url = viewParams.url;
             containerParams.urlOriginLength = viewParams.urlOriginLength;
-            containerParams.truncatedUrl = UrlFormatter.formatUrlForSecurityDisplay(
-                    url, SchemeDisplay.OMIT_HTTP_AND_HTTPS);
+            containerParams.truncatedUrl =
+                    UrlFormatter.formatUrlForDisplayOmitSchemePathAndTrivialSubdomains(url);
             containerParams.backButtonClickCallback = this::exitSubpage;
             containerParams.urlTitleClickCallback = mContainer::toggleUrlTruncation;
             containerParams.urlTitleLongClickCallback = viewParams.urlTitleLongClickCallback;

components/url_formatter/android/BUILD.gn

@@ -37,6 +37,7 @@ android_library("url_formatter_javatests") {
     "//third_party/android_deps:androidx_test_runner_java",
     "//third_party/android_support_test_runner:runner_java",
     "//third_party/junit",
+    "//url:gurl_android_test_helper_java",
     "//url:gurl_java",
   ]
 }

components/url_formatter/android/java/src/org/chromium/components/url_formatter/UrlFormatter.java

@@ -75,14 +75,14 @@ public final class UrlFormatter {
 
     /**
      * Builds a String representation of <code>uri</code> suitable for display to the user,
-     * omitting the HTTP scheme, the username and password, trailing slash on a bare hostname,
+     * omitting the HTTP/HTTPS scheme, the username and password, trailing slash on a bare hostname,
      * converting %20 to spaces, and removing trivial subdomains.
      *
      * The IDN hostname is turned to Unicode if the Unicode representation is deemed safe.
      * For more information, see <code>url_formatter::FormatUrl(const GURL&)</code>.
      *
      * Example:
-     *  - "http://user:password@example.com/%20test" -> "example.com"
+     *  - "http://user:password@example.com/%20test" -> "example.com/ test"
      *  - "http://user:password@example.com/" -> "example.com"
      *  - "http://www.xn--frgbolaget-q5a.se" -> "färgbolaget.se"
      *
@@ -92,6 +92,27 @@ public final class UrlFormatter {
     public static String formatUrlForDisplayOmitSchemeOmitTrivialSubdomains(String uri) {
         return UrlFormatterJni.get().formatUrlForDisplayOmitSchemeOmitTrivialSubdomains(uri);
     }
+
+    /**
+     * Builds a String representation of <code>uri</code> suitable for display to the user,
+     * omitting the HTTP/HTTPS scheme, the username and password, the path and removing trivial
+     * subdomains.
+     *
+     * The IDN hostname is turned to Unicode if the Unicode representation is deemed safe.
+     * For more information, see <code>url_formatter::FormatUrl(const GURL&)</code>.
+     *
+     * Example:
+     *  - "http://user:password@example.com/%20test" -> "example.com"
+     *  - "http://user:password@example.com/" -> "example.com"
+     *  - "http://www.xn--frgbolaget-q5a.se" -> "färgbolaget.se"
+     *
+     * @param uri URI to format.
+     * @return Formatted URL.
+     */
+    public static String formatUrlForDisplayOmitSchemePathAndTrivialSubdomains(GURL uri) {
+        return UrlFormatterJni.get().formatUrlForDisplayOmitSchemePathAndTrivialSubdomains(uri);
+    }
+
     /**
      * Builds a String representation of <code>uri</code> suitable for display to the user,
      * omitting the username and password and trailing slash on a bare hostname.
@@ -163,6 +184,7 @@ public final class UrlFormatter {
         String formatUrlForDisplayOmitScheme(String url);
         String formatUrlForDisplayOmitHTTPScheme(String url);
         String formatUrlForDisplayOmitSchemeOmitTrivialSubdomains(String url);
+        String formatUrlForDisplayOmitSchemePathAndTrivialSubdomains(GURL url);
         String formatUrlForDisplayOmitUsernamePassword(String url);
         String formatUrlForCopy(String url);
         String formatUrlForSecurityDisplay(GURL url, @SchemeDisplay int schemeDisplay);

components/url_formatter/android/javatests/src/org/chromium/components/url_formatter/UrlFormatterUnitTest.java

@@ -4,6 +4,8 @@
 
 package org.chromium.components.url_formatter;
 
+import static org.junit.Assert.assertEquals;
+
 import androidx.test.filters.SmallTest;
 
 import org.junit.Assert;
@@ -11,9 +13,12 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import org.chromium.base.Function;
 import org.chromium.base.test.BaseJUnit4ClassRunner;
 import org.chromium.base.test.util.Batch;
 import org.chromium.content_public.browser.test.NativeLibraryTestUtils;
+import org.chromium.url.GURL;
+import org.chromium.url.GURLJavaTestHelper;
 
 /**
  * Unit tests for {@link UrlFormatter}.
@@ -27,14 +32,15 @@ public class UrlFormatterUnitTest {
     @Before
     public void setUp() {
         NativeLibraryTestUtils.loadNativeLibraryNoBrowserProcess();
+        GURLJavaTestHelper.nativeInitializeICU();
     }
 
     @Test
     @SmallTest
     public void testFixupUrl() {
-        Assert.assertEquals("http://google.com/", UrlFormatter.fixupUrl("google.com").getSpec());
-        Assert.assertEquals("chrome://version/", UrlFormatter.fixupUrl("about:").getSpec());
-        Assert.assertEquals("file:///mail.google.com:/",
+        assertEquals("http://google.com/", UrlFormatter.fixupUrl("google.com").getSpec());
+        assertEquals("chrome://version/", UrlFormatter.fixupUrl("about:").getSpec());
+        assertEquals("file:///mail.google.com:/",
                 UrlFormatter.fixupUrl("//mail.google.com:/").getSpec());
         Assert.assertFalse(UrlFormatter.fixupUrl("0x100.0").isValid());
     }
@@ -42,12 +48,25 @@ public class UrlFormatterUnitTest {
     @Test
     @SmallTest
     public void testFormatUrlForDisplayOmitUsernamePassword() {
-        Assert.assertEquals("http://google.com/path",
+        assertEquals("http://google.com/path",
                 UrlFormatter.formatUrlForDisplayOmitUsernamePassword("http://google.com/path"));
-        Assert.assertEquals("http://google.com",
+        assertEquals("http://google.com",
                 UrlFormatter.formatUrlForDisplayOmitUsernamePassword(
                         "http://user:pass@google.com"));
-        Assert.assertEquals("http://google.com",
+        assertEquals("http://google.com",
                 UrlFormatter.formatUrlForDisplayOmitUsernamePassword("http://user@google.com"));
     }
+
+    @Test
+    @SmallTest
+    public void testFormatUrlForDisplayOmitSchemePathAndTrivialSubdomains() {
+        Function<GURL, String> f =
+                UrlFormatter::formatUrlForDisplayOmitSchemePathAndTrivialSubdomains;
+
+        assertEquals("google.com", f.apply(new GURL("http://user:pass@google.com/path")));
+        assertEquals("chrome://version", f.apply(new GURL("chrome://version")));
+        assertEquals("äää.de", f.apply(new GURL("https://äää.de")));
+        assertEquals("xn--4caaa.com", f.apply(new GURL("https://äää.com")));
+        assertEquals("مثال.إختبار", f.apply(new GURL("https://xn--mgbh0fb.xn--kgbechtv/")));
+    }
 }

components/url_formatter/url_formatter_android.cc

@@ -118,6 +118,22 @@ JNI_UrlFormatter_FormatUrlForDisplayOmitSchemeOmitTrivialSubdomains(
                net::UnescapeRule::SPACES, nullptr, nullptr, nullptr));
 }
 
+static ScopedJavaLocalRef<jstring>
+JNI_UrlFormatter_FormatUrlForDisplayOmitSchemePathAndTrivialSubdomains(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& j_gurl) {
+  DCHECK(j_gurl);
+  std::unique_ptr<GURL> gurl = url::GURLAndroid::ToNativeGURL(env, j_gurl);
+  return base::android::ConvertUTF16ToJavaString(
+      env, url_formatter::FormatUrl(
+               *gurl,
+               url_formatter::kFormatUrlOmitDefaults |
+                   url_formatter::kFormatUrlTrimAfterHost |
+                   url_formatter::kFormatUrlOmitHTTPS |
+                   url_formatter::kFormatUrlOmitTrivialSubdomains,
+               net::UnescapeRule::SPACES, nullptr, nullptr, nullptr));
+}
+
 }  // namespace android
 
 }  // namespace url_formatter