Bug 1830282 - Replace Focus UrlUtils.java methods with URLStringUtils…

… from AC. Move new methods to AC. Also remove unused methods. Migrate corresponding tests to AC components.
AndiAJ · May 9, 2023 · bf58690 · bf58690
1 parent b1e2500
commit bf58690
Show file tree

Hide file tree

Showing 16 changed files with 98 additions and 345 deletions.
diff --git a/...ts/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/net/Uri.kt b/...ts/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/net/Uri.kt
@@ -16,7 +16,7 @@ import java.io.File
 import java.io.IOException
 import java.util.UUID
 
-private val commonPrefixes = listOf("www.", "mobile.", "m.")
+internal val commonPrefixes = listOf("www.", "mobile.", "m.")
 
 /**
  * Returns the host without common prefixes like "www" or "m".

diff --git a/...ents/components/support/ktx/src/main/java/mozilla/components/support/ktx/kotlin/String.kt b/...ents/components/support/ktx/src/main/java/mozilla/components/support/ktx/kotlin/String.kt
@@ -13,6 +13,7 @@ import android.util.Patterns
 import android.webkit.URLUtil
 import androidx.core.net.toUri
 import mozilla.components.lib.publicsuffixlist.PublicSuffixList
+import mozilla.components.support.ktx.android.net.commonPrefixes
 import mozilla.components.support.ktx.android.net.hostWithoutCommonPrefixes
 import mozilla.components.support.ktx.util.URLStringUtils
 import java.io.File
@@ -380,6 +381,16 @@ fun String.getRepresentativeCharacter(): String {
     return "?"
 }
 
+/**
+ * Strips common mobile subdomains from a [String].
+ */
+fun String.stripCommonSubdomains(): String {
+    for (prefix in commonPrefixes) {
+        if (this.startsWith(prefix)) return this.substring(prefix.length)
+    }
+    return this
+}
+
 /**
  * Returns the last 4 digits from a formatted credit card number string.
  */

diff --git a/.../components/support/ktx/src/test/java/mozilla/components/support/ktx/kotlin/StringTest.kt b/.../components/support/ktx/src/test/java/mozilla/components/support/ktx/kotlin/StringTest.kt
@@ -512,6 +512,15 @@ class StringTest {
     }
     // END test cases borrowed from FFTV
 
+    @Test
+    fun testStripCommonSubdomains() {
+        assertEquals("mozilla.org", ("mozilla.org").stripCommonSubdomains())
+        assertEquals("mozilla.org", ("www.mozilla.org").stripCommonSubdomains())
+        assertEquals("mozilla.org", ("m.mozilla.org").stripCommonSubdomains())
+        assertEquals("mozilla.org", ("mobile.mozilla.org").stripCommonSubdomains())
+        assertEquals("random.mozilla.org", ("random.mozilla.org").stripCommonSubdomains())
+    }
+
     private infix fun String.shortenedShouldBecome(expect: String) {
         assertEquals(expect, this.shortened())
     }

diff --git a/...ponents/support/utils/src/main/java/mozilla/components/support/ktx/util/URLStringUtils.kt b/...ponents/support/utils/src/main/java/mozilla/components/support/ktx/util/URLStringUtils.kt
@@ -143,4 +143,25 @@ object URLStringUtils {
     private fun maybeStripTrailingSlash(url: CharSequence): CharSequence {
         return url.trimEnd('/')
     }
+
+    /**
+     * Determines whether a string is http or https URL
+     */
+    fun isHttpOrHttps(url: String): Boolean {
+        return !TextUtils.isEmpty(url) && (url.startsWith("http:") || url.startsWith("https:"))
+    }
+
+    /**
+     * Determine whether a string is a valid search query URL.
+     */
+    fun isValidSearchQueryUrl(url: String): Boolean {
+        var trimmedUrl = url.trim { it <= ' ' }
+        if (!trimmedUrl.matches("^.+?://.+?".toRegex())) {
+            // UI hint url doesn't have http scheme, so add it if necessary
+            trimmedUrl = "http://$trimmedUrl"
+        }
+        val isNetworkUrl = isHttpOrHttps(trimmedUrl)
+        val containsToken = trimmedUrl.contains("%s")
+        return isNetworkUrl && containsToken
+    }
 }
diff --git a/...onents/support/utils/src/test/java/mozilla/components/support/utils/URLStringUtilsTest.kt b/...onents/support/utils/src/test/java/mozilla/components/support/utils/URLStringUtilsTest.kt
@@ -35,6 +35,14 @@ class URLStringUtilsTest {
         assertEquals(expectedUrl, toNormalizedURL("  http://mozilla.org  "))
         assertEquals(expectedUrl, toNormalizedURL("mozilla.org"))
         assertEquals(expectedUrl, toNormalizedURL("HTTP://mozilla.org"))
+        assertEquals("file:///mnt/sdcard/", toNormalizedURL("file:///mnt/sdcard/"))
+        assertEquals("http://mozilla.org", toNormalizedURL(" http://mozilla.org"))
+        assertEquals("http://localhost", toNormalizedURL("localhost"))
+
+        assertEquals(
+            "https://www.mozilla.org/en-US/internet-health/",
+            toNormalizedURL("https://www.mozilla.org/en-US/internet-health/"),
+        )
     }
 
     @Test
@@ -198,6 +206,28 @@ class URLStringUtilsTest {
 
         assertEquals("  ", URLStringUtils.toDisplayUrl("  "))
     }
+
+    @Test
+    fun isHttpOrHttpsUrl() {
+        assertFalse(URLStringUtils.isHttpOrHttps(""))
+        assertFalse(URLStringUtils.isHttpOrHttps("     "))
+        assertFalse(URLStringUtils.isHttpOrHttps("mozilla.org"))
+        assertFalse(URLStringUtils.isHttpOrHttps("httpstrf://example.org"))
+        assertTrue(URLStringUtils.isHttpOrHttps("https://www.mozilla.org"))
+        assertTrue(URLStringUtils.isHttpOrHttps("http://example.org"))
+        assertTrue(URLStringUtils.isHttpOrHttps("http://192.168.0.1"))
+    }
+
+    @Test
+    fun isValidSearchQueryUrl() {
+        assertTrue(URLStringUtils.isValidSearchQueryUrl("https://example.com/search/?q=%s"))
+        assertTrue(URLStringUtils.isValidSearchQueryUrl("http://example.com/search/?q=%s"))
+        assertTrue(URLStringUtils.isValidSearchQueryUrl("http-test-site.com/search/?q=%s"))
+        assertFalse(URLStringUtils.isValidSearchQueryUrl("httpss://example.com/search/?q=%s"))
+        assertTrue(URLStringUtils.isValidSearchQueryUrl("example.com/search/?q=%s"))
+        assertTrue(URLStringUtils.isValidSearchQueryUrl(" example.com/search/?q=%s "))
+        assertFalse(URLStringUtils.isValidSearchQueryUrl("htps://example.com/search/?q=%s"))
+    }
 }
 
 /**

diff --git a/focus-android/app/src/main/java/org/mozilla/focus/ext/String.kt b/focus-android/app/src/main/java/org/mozilla/focus/ext/String.kt
@@ -7,7 +7,7 @@ package org.mozilla.focus.ext
 import androidx.compose.ui.graphics.Color
 import androidx.core.net.toUri
 import mozilla.components.support.ktx.android.net.hostWithoutCommonPrefixes
-import org.mozilla.focus.utils.UrlUtils
+import mozilla.components.support.ktx.util.URLStringUtils
 
 // Extension functions for the String class
 
@@ -17,7 +17,7 @@ import org.mozilla.focus.utils.UrlUtils
  * Spec: https://github.com/mozilla-mobile/focus-android/issues/1231#issuecomment-326237077
  */
 fun String.beautifyUrl(): String {
-    if (isNullOrEmpty() || !UrlUtils.isHttpOrHttps(this)) {
+    if (isNullOrEmpty() || !URLStringUtils.isHttpOrHttps(this)) {
         return this
     }
 

diff --git a/focus-android/app/src/main/java/org/mozilla/focus/fragment/UrlInputFragment.kt b/focus-android/app/src/main/java/org/mozilla/focus/fragment/UrlInputFragment.kt
@@ -28,6 +28,7 @@ import mozilla.components.feature.top.sites.TopSitesFeature
 import mozilla.components.service.glean.private.NoExtras
 import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
 import mozilla.components.support.ktx.android.view.hideKeyboard
+import mozilla.components.support.ktx.util.URLStringUtils
 import mozilla.components.support.utils.StatusBarUtils
 import mozilla.components.support.utils.ThreadUtils
 import org.mozilla.focus.GleanMetrics.BrowserSearch
@@ -55,7 +56,6 @@ import org.mozilla.focus.topsites.TopSitesOverlay
 import org.mozilla.focus.ui.theme.FocusTheme
 import org.mozilla.focus.utils.OneShotOnPreDrawListener
 import org.mozilla.focus.utils.SupportUtils
-import org.mozilla.focus.utils.UrlUtils
 import org.mozilla.focus.utils.ViewUtils
 import kotlin.coroutines.CoroutineContext
 
@@ -543,9 +543,9 @@ class UrlInputFragment :
 
             ViewUtils.hideKeyboard(binding.browserToolbar)
 
-            val isUrl = UrlUtils.isUrl(input)
+            val isUrl = URLStringUtils.isURLLike(input)
             if (isUrl) {
-                openUrl(UrlUtils.normalize(input))
+                openUrl(URLStringUtils.toNormalizedURL(input))
             } else {
                 search(input)
             }
@@ -580,8 +580,8 @@ class UrlInputFragment :
         if (alwaysSearch) {
             search(query)
         } else {
-            if (UrlUtils.isUrl(query)) {
-                openUrl(UrlUtils.normalize(query))
+            if (URLStringUtils.isURLLike(query)) {
+                openUrl(URLStringUtils.toNormalizedURL(query))
             } else {
                 search(query)
             }

diff --git a/focus-android/app/src/main/java/org/mozilla/focus/search/ManualAddSearchEnginePreference.kt b/focus-android/app/src/main/java/org/mozilla/focus/search/ManualAddSearchEnginePreference.kt
@@ -18,9 +18,9 @@ import androidx.preference.Preference
 import androidx.preference.PreferenceViewHolder
 import com.google.android.material.textfield.TextInputLayout
 import mozilla.components.browser.state.search.SearchEngine
+import mozilla.components.support.ktx.util.URLStringUtils
 import mozilla.components.support.utils.ext.getParcelableCompat
 import org.mozilla.focus.R
-import org.mozilla.focus.utils.UrlUtils
 import org.mozilla.focus.utils.ViewUtils
 
 class ManualAddSearchEnginePreference(context: Context, attrs: AttributeSet) :
@@ -97,7 +97,7 @@ class ManualAddSearchEnginePreference(context: Context, attrs: AttributeSet) :
     fun validateSearchQueryAndShowError(searchQuery: String): Boolean {
         val errorMessage = when {
             TextUtils.isEmpty(searchQuery) -> context.getString(R.string.search_add_error_empty_search)
-            !UrlUtils.isValidSearchQueryUrl(searchQuery) -> context.getString(R.string.search_add_error_format)
+            !URLStringUtils.isValidSearchQueryUrl(searchQuery) -> context.getString(R.string.search_add_error_format)
             else -> null
         }
 

diff --git a/focus-android/app/src/main/java/org/mozilla/focus/session/IntentProcessor.kt b/focus-android/app/src/main/java/org/mozilla/focus/session/IntentProcessor.kt
@@ -13,13 +13,13 @@ import mozilla.components.feature.customtabs.createCustomTabConfigFromIntent
 import mozilla.components.feature.customtabs.isCustomTabIntent
 import mozilla.components.feature.tabs.CustomTabsUseCases
 import mozilla.components.feature.tabs.TabsUseCases
+import mozilla.components.support.ktx.util.URLStringUtils
 import mozilla.components.support.utils.SafeIntent
 import mozilla.components.support.utils.WebURLFinder
 import org.mozilla.focus.activity.TextActionActivity
 import org.mozilla.focus.ext.components
 import org.mozilla.focus.shortcut.HomeScreen
 import org.mozilla.focus.utils.SearchUtils
-import org.mozilla.focus.utils.UrlUtils
 
 /**
  * Implementation moved from Focus SessionManager. To be replaced with SessionIntentProcessor from feature-session
@@ -107,7 +107,7 @@ class IntentProcessor(
                     return Result.None
                 }
 
-                return if (!UrlUtils.isUrl(dataString)) {
+                return if (dataString == null || !URLStringUtils.isURLLike(dataString)) {
                     val bestURL = WebURLFinder(dataString).bestWebURL()
                     if (!TextUtils.isEmpty(bestURL)) {
                         createSession(SessionState.Source.External.ActionSend(null), bestURL ?: "")
@@ -119,7 +119,7 @@ class IntentProcessor(
                         )
                     }
                 } else {
-                    createSession(SessionState.Source.External.ActionSend(null), dataString ?: "")
+                    createSession(SessionState.Source.External.ActionSend(null), dataString)
                 }
             }
 

diff --git a/...oid/app/src/main/java/org/mozilla/focus/settings/ManualAddSearchEngineSettingsFragment.kt b/...oid/app/src/main/java/org/mozilla/focus/settings/ManualAddSearchEngineSettingsFragment.kt
@@ -32,6 +32,7 @@ import mozilla.components.concept.fetch.Request
 import mozilla.components.concept.fetch.Request.Redirect.FOLLOW
 import mozilla.components.feature.search.ext.createSearchEngine
 import mozilla.components.service.glean.private.NoExtras
+import mozilla.components.support.ktx.util.URLStringUtils
 import org.mozilla.focus.GleanMetrics.SearchEngines
 import org.mozilla.focus.R
 import org.mozilla.focus.ext.components
@@ -42,7 +43,6 @@ import org.mozilla.focus.search.ManualAddSearchEnginePreference
 import org.mozilla.focus.state.AppAction
 import org.mozilla.focus.telemetry.TelemetryWrapper
 import org.mozilla.focus.utils.SupportUtils
-import org.mozilla.focus.utils.UrlUtils
 import org.mozilla.focus.utils.ViewUtils
 import java.io.IOException
 import java.net.MalformedURLException
@@ -192,7 +192,7 @@ class ManualAddSearchEngineSettingsFragment : BaseSettingsFragment() {
             // we should share the code to substitute and normalize the search string (see SearchEngine.buildSearchUrl).
             val encodedTestQuery = Uri.encode("testSearchEngineValidation")
 
-            val normalizedHttpsSearchURLStr = UrlUtils.normalize(query)
+            val normalizedHttpsSearchURLStr = URLStringUtils.toNormalizedURL(query)
             val searchURLStr = normalizedHttpsSearchURLStr.replace("%s".toRegex(), encodedTestQuery)
 
             try { URL(searchURLStr) } catch (e: MalformedURLException) {

diff --git a/focus-android/app/src/main/java/org/mozilla/focus/shortcut/HomeScreen.java b/focus-android/app/src/main/java/org/mozilla/focus/shortcut/HomeScreen.java
@@ -17,10 +17,12 @@
 import android.text.TextUtils;
 
 import org.mozilla.focus.activity.MainActivity;
-import org.mozilla.focus.utils.UrlUtils;
 
 import java.util.UUID;
 
+import mozilla.components.support.ktx.kotlin.StringKt;
+import mozilla.components.support.ktx.util.URLStringUtils;
+
 public class HomeScreen {
     public static final String ADD_TO_HOMESCREEN_TAG = "add_to_homescreen";
     public static final String BLOCKING_ENABLED = "blocking_enabled";
@@ -77,7 +79,7 @@ private static Intent createShortcutIntent(Context context, String url, boolean
 
     @VisibleForTesting static String generateTitleFromUrl(String url) {
         // For now we just use the host name and strip common subdomains like "www" or "m".
-        return UrlUtils.stripCommonSubdomains(Uri.parse(url).getHost());
+        return StringKt.stripCommonSubdomains(Uri.parse(url).getHost());
     }
 
     /**

diff --git a/focus-android/app/src/main/java/org/mozilla/focus/shortcut/IconGenerator.kt b/focus-android/app/src/main/java/org/mozilla/focus/shortcut/IconGenerator.kt
@@ -14,8 +14,8 @@ import android.os.Build
 import android.util.TypedValue
 import androidx.core.content.ContextCompat
 import androidx.core.net.toUri
+import mozilla.components.support.ktx.kotlin.stripCommonSubdomains
 import org.mozilla.focus.R
-import org.mozilla.focus.utils.UrlUtils
 
 class IconGenerator {
 
@@ -129,7 +129,7 @@ class IconGenerator {
             }
 
             // Strip common prefixes that we do not want to use to determine the representative characters
-            return UrlUtils.stripCommonSubdomains(snippet)
+            return snippet?.stripCommonSubdomains()
         }
     }
 }
diff --git a/focus-android/app/src/main/java/org/mozilla/focus/telemetry/TelemetryWrapper.kt b/focus-android/app/src/main/java/org/mozilla/focus/telemetry/TelemetryWrapper.kt
@@ -13,18 +13,19 @@ import android.os.Build
 import android.os.StrictMode
 import androidx.annotation.CheckResult
 import androidx.annotation.VisibleForTesting
+import androidx.core.net.toUri
 import androidx.preference.PreferenceManager
 import kotlinx.coroutines.runBlocking
 import mozilla.components.browser.state.search.SearchEngine
 import mozilla.components.browser.state.selector.privateTabs
 import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine
+import mozilla.components.support.ktx.android.net.hostWithoutCommonPrefixes
 import org.json.JSONObject
 import org.mozilla.focus.BuildConfig
 import org.mozilla.focus.R
 import org.mozilla.focus.ext.components
 import org.mozilla.focus.utils.AppConstants
 import org.mozilla.focus.utils.MobileMetricsPingStorage
-import org.mozilla.focus.utils.UrlUtils
 import org.mozilla.telemetry.Telemetry
 import org.mozilla.telemetry.TelemetryHolder
 import org.mozilla.telemetry.config.TelemetryConfiguration
@@ -39,7 +40,6 @@ import org.mozilla.telemetry.schedule.jobscheduler.JobSchedulerTelemetrySchedule
 import org.mozilla.telemetry.serialize.JSONPingSerializer
 import org.mozilla.telemetry.storage.FileTelemetryStorage
 import java.net.MalformedURLException
-import java.net.URL
 import java.text.SimpleDateFormat
 import java.util.Date
 import java.util.Locale
@@ -308,7 +308,7 @@ object TelemetryWrapper {
     @JvmStatic
     fun addLoadToHistogram(url: String, newLoadTime: Long) {
         try {
-            domainMap.add(UrlUtils.stripCommonSubdomains(URL(url).host))
+            url.toUri().hostWithoutCommonPrefixes?.let { domainMap.add(it) }
             numUri++
             var histogramLoadIndex = (newLoadTime / BUCKET_SIZE_MS).toInt()