Bug 1377287 - review: getHostSLD returns empty str on error. r=mcomella
authorMichael Comella <michael.l.comella@gmail.com>
Wed, 26 Jul 2017 15:08:40 -0700
changeset 419912 7a3e7c2caaf7e760ffe8222c1d5e30f8d68a0773
parent 419911 4da81d3d8c65be8fecb9c1d4452ca463cb136c67
child 419913 e677ddb0596512ba640f64c0b35e9b5f0465c8d2
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcomella
bugs1377287
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1377287 - review: getHostSLD returns empty str on error. r=mcomella In response to https://bugzilla.mozilla.org/show_bug.cgi?id=1377287#c26: getHostSLD should only return hosts. MozReview-Commit-ID: IFN5FWjFLx4
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/HighlightItem.java
mobile/android/base/java/org/mozilla/gecko/util/URIUtils.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/util/TestURIUtils.java
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/HighlightItem.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/HighlightItem.java
@@ -168,12 +168,14 @@ public class HighlightItem extends Strea
         @Override
         protected void onPostExecute(final String hostSLD) {
             super.onPostExecute(hostSLD);
             final TextView viewToUpdate = pageDomainViewWeakReference.get();
             if (viewToUpdate == null) {
                 return;
             }
 
+            // hostSLD will be the empty String if the host cannot be determined. This is fine: it's very unlikely
+            // and the page title will be URL if there's an error there so we wouldn't want to write the URL again here.
             viewToUpdate.setText(hostSLD);
         }
     }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/util/URIUtils.java
+++ b/mobile/android/base/java/org/mozilla/gecko/util/URIUtils.java
@@ -32,33 +32,36 @@ public class URIUtils {
             return null;
         }
     }
 
     /**
      * Returns the second level domain (SLD) of a url. It removes any subdomain/TLD.
      * e.g. https://m.foo.com/bar/baz?noo=abc#123  => foo
      *
+     * The return value may still contain a public suffix (e.g. .com) if the suffix does not match any of our
+     * known values. If a host cannot be determined from the given url String, the empty String will be returned.
+     *
      * This implementation is taken from Firefox for iOS:
      *   https://github.com/mozilla-mobile/firefox-ios/blob/deb9736c905cdf06822ecc4a20152df7b342925d/Shared/Extensions/NSURLExtensions.swift#L152
      *
      * @param uriString A url from which to extract the second level domain.
-     * @return The second level domain of the url.
+     * @return The second level domain of the url or the empty String when the host cannot be determined.
      */
     @WorkerThread // PublicSuffix methods can touch the disk.
     public static String getHostSecondLevelDomain(@NonNull final Context context, @NonNull final String uriString)
             throws URISyntaxException {
         if (context == null) { throw new NullPointerException("Expected non-null Context argument"); }
         if (uriString == null) { throw new NullPointerException("Expected non-null uri argument"); }
 
         final URI uri = new URI(uriString);
         final String baseDomain = getBaseDomain(context, uri);
         if (baseDomain == null) {
             final String normalizedHost = StringUtils.stripCommonSubdomains(uri.getHost());
-            return !TextUtils.isEmpty(normalizedHost) ? normalizedHost : uriString;
+            return !TextUtils.isEmpty(normalizedHost) ? normalizedHost : "";
         }
 
         return PublicSuffix.stripPublicSuffix(context, baseDomain);
     }
 
     /**
      * Returns the base domain from a given hostname. The base domain name is defined as the public domain suffix
      * with the base private domain attached to the front. For example, for the URL www.bbc.co.uk, the base domain
@@ -93,17 +96,18 @@ public class URIUtils {
     private static boolean isIPv6(final URI uri) {
         final String host = uri.getHost();
         return !TextUtils.isEmpty(host) && host.contains(":");
     }
 
     /**
      * An async task that will take a URI formatted as a String and will retrieve
      * {@link #getHostSecondLevelDomain(Context, String)}. To use this, extend the class and override
-     * {@link #onPostExecute(Object)}, where the secondLevelDomain will be returned.
+     * {@link #onPostExecute(Object)}, where the secondLevelDomain, or the empty String if the host cannot determined,
+     * will be returned.
      */
     public static abstract class GetHostSecondLevelDomainAsyncTask extends AsyncTask<Void, Void, String> {
         protected final WeakReference<Context> contextWeakReference;
         protected final String uriString;
 
         public GetHostSecondLevelDomainAsyncTask(final Context contextWeakReference, final String uriString) {
             this.contextWeakReference = new WeakReference<>(contextWeakReference);
             this.uriString = uriString;
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/util/TestURIUtils.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/util/TestURIUtils.java
@@ -23,16 +23,21 @@ public class TestURIUtils {
     @Test
     public void testGetHostSecondLevelDomain() throws Exception {
         assertGetHostSLD("https://www.example.com/index.html", "example");
         assertGetHostSLD("https://m.foo.com/bar/baz?noo=abc#123", "foo");
         assertGetHostSLD("https://user:pass@m.foo.com/bar/baz?noo=abc#123", "foo");
     }
 
     @Test
+    public void testGetHostSecondLevelDomainURIHasNoHost() throws Exception {
+        assertGetHostSLD("file:///usr/bin", "");
+    }
+
+    @Test
     public void testGetHostSecondLevelDomainIPv4() throws Exception {
         assertGetHostSLD("http://192.168.1.1", "192.168.1.1");
     }
 
     @Test
     public void testGetHostSecondLevelDomainIPv6() throws Exception {
         assertGetHostSLD("http://[3ffe:1900:4545:3:200:f8ff:fe21:67cf]", "[3ffe:1900:4545:3:200:f8ff:fe21:67cf]");
     }