diff --git a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationStore.kt b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationStore.kt index b0432090d..7772d5d1c 100644 --- a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationStore.kt +++ b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationStore.kt @@ -13,8 +13,8 @@ import mozilla.components.lib.publicsuffixlist.PublicSuffixList import mozilla.components.lib.state.Action import mozilla.components.lib.state.State import mozilla.components.lib.state.Store +import mozilla.components.support.ktx.kotlin.toShortUrl import org.mozilla.fenix.components.TabCollectionStorage -import org.mozilla.fenix.ext.toShortUrl class CollectionCreationStore( initialState: CollectionCreationState, diff --git a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationView.kt b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationView.kt index bf47fbd16..cd98d98de 100644 --- a/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationView.kt +++ b/app/src/main/java/org/mozilla/fenix/collections/CollectionCreationView.kt @@ -20,12 +20,12 @@ import androidx.transition.TransitionManager import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.support.ktx.android.view.hideKeyboard import mozilla.components.support.ktx.android.view.showKeyboard +import mozilla.components.support.ktx.kotlin.toShortUrl import mozilla.telemetry.glean.private.NoExtras import org.mozilla.fenix.GleanMetrics.Collections import org.mozilla.fenix.R import org.mozilla.fenix.databinding.ComponentCollectionCreationBinding import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.toShortUrl class CollectionCreationView( private val container: ViewGroup, diff --git a/app/src/main/java/org/mozilla/fenix/components/TabCollectionStorage.kt b/app/src/main/java/org/mozilla/fenix/components/TabCollectionStorage.kt index b76fc778b..7b5a5160c 100644 --- a/app/src/main/java/org/mozilla/fenix/components/TabCollectionStorage.kt +++ b/app/src/main/java/org/mozilla/fenix/components/TabCollectionStorage.kt @@ -19,8 +19,8 @@ import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.feature.tab.collections.TabCollectionStorage import mozilla.components.support.base.observer.Observable import mozilla.components.support.base.observer.ObserverRegistry +import mozilla.components.support.ktx.kotlin.toShortUrl import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.perf.StrictModeManager private const val COLLECTION_MAX_TITLE_LENGTH = 20 diff --git a/app/src/main/java/org/mozilla/fenix/ext/String.kt b/app/src/main/java/org/mozilla/fenix/ext/String.kt index 618172d8e..c390ef9fc 100644 --- a/app/src/main/java/org/mozilla/fenix/ext/String.kt +++ b/app/src/main/java/org/mozilla/fenix/ext/String.kt @@ -4,76 +4,12 @@ package org.mozilla.fenix.ext -import android.net.InetAddresses -import android.os.Build import android.text.Editable -import android.util.Patterns -import android.webkit.URLUtil import androidx.compose.runtime.Composable -import androidx.core.net.toUri -import mozilla.components.lib.publicsuffixlist.PublicSuffixList -import mozilla.components.support.ktx.android.net.hostWithoutCommonPrefixes import mozilla.components.support.ktx.kotlin.MAX_URI_LENGTH +import mozilla.components.support.ktx.kotlin.toShortUrl import org.mozilla.fenix.components.components import org.mozilla.fenix.compose.inComposePreview -import java.net.IDN -import java.util.Locale - -const val FILE_PREFIX = "file://" -const val MAX_VALID_PORT = 65_535 - -/** - * Shortens URLs to be more user friendly. - * - * The algorithm used to generate these strings is a combination of FF desktop 'top sites', - * feedback from the security team, and documentation regarding url elision. See - * StringTest.kt for details. - * - * This method is complex because URLs have a lot of edge cases. Be sure to thoroughly unit - * test any changes you make to it. - */ -// Unused Parameter: We may resume stripping eTLD, depending on conversations between security and UX -// Return count: This is a complex method, but it would not be more understandable if broken up -// ComplexCondition: Breaking out the complex condition would make this logic harder to follow -@Suppress("UNUSED_PARAMETER", "ReturnCount", "ComplexCondition") -fun String.toShortUrl(publicSuffixList: PublicSuffixList): String { - val inputString = this - val uri = inputString.toUri() - - if ( - inputString.isEmpty() || - !URLUtil.isValidUrl(inputString) || - inputString.startsWith(FILE_PREFIX) || - uri.port !in -1..MAX_VALID_PORT - ) { - return inputString - } - - if (uri.host?.isIpv4OrIpv6() == true || - // If inputString is just a hostname and not a FQDN, use the entire hostname. - uri.host?.contains(".") == false - ) { - return uri.host ?: inputString - } - - fun String.stripUserInfo(): String { - val userInfo = this.toUri().encodedUserInfo - return if (userInfo != null) { - val infoIndex = this.indexOf(userInfo) - this.removeRange(infoIndex..infoIndex + userInfo.length) - } else { - this - } - } - fun String.stripPrefixes(): String = this.toUri().hostWithoutCommonPrefixes ?: this - fun String.toUnicode() = IDN.toUnicode(this) - - return inputString - .stripUserInfo() - .lowercase(Locale.getDefault()) - .stripPrefixes() - .toUnicode() -} /** * Shortens URLs to be more user friendly, by applying [String.toShortUrl] @@ -94,29 +30,6 @@ fun String.toShortUrl(): String { } } -// impl via FFTV https://searchfox.org/mozilla-mobile/source/firefox-echo-show/app/src/main/java/org/mozilla/focus/utils/FormattedDomain.java#129 -@Suppress("DEPRECATION") -internal fun String.isIpv4(): Boolean = Patterns.IP_ADDRESS.matcher(this).matches() - -// impl via FFiOS: https://github.com/mozilla-mobile/firefox-ios/blob/deb9736c905cdf06822ecc4a20152df7b342925d/Shared/Extensions/NSURLExtensions.swift#L292 -// True IPv6 validation is difficult. This is slightly better than nothing -internal fun String.isIpv6(): Boolean { - return this.isNotEmpty() && this.contains(":") -} - -/** - * Returns true if the string represents a valid Ipv4 or Ipv6 IP address. - * Note: does not validate a dual format Ipv6 ( "y:y:y:y:y:y:x.x.x.x" format). - * - */ -fun String.isIpv4OrIpv6(): Boolean { - return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { - InetAddresses.isNumericAddress(this) - } else { - this.isIpv4() || this.isIpv6() - } -} - /** * Trims a URL string of its scheme and common prefixes. * diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt index 316228583..6306a29de 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt @@ -39,6 +39,7 @@ import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.support.base.feature.UserInteractionHandler +import mozilla.components.support.ktx.kotlin.toShortUrl import mozilla.telemetry.glean.private.NoExtras import org.mozilla.fenix.GleanMetrics.BookmarksManagement import org.mozilla.fenix.HomeActivity @@ -54,7 +55,6 @@ import org.mozilla.fenix.ext.minus import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.setTextColor -import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.library.LibraryPageFragment import org.mozilla.fenix.utils.allowUndo diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt index 8299cc757..de770270e 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt @@ -36,6 +36,7 @@ import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.support.ktx.android.content.getColorFromAttr import mozilla.components.support.ktx.android.view.hideKeyboard import mozilla.components.support.ktx.android.view.showKeyboard +import mozilla.components.support.ktx.kotlin.toShortUrl import mozilla.telemetry.glean.private.NoExtras import org.mozilla.fenix.GleanMetrics.BookmarksManagement import org.mozilla.fenix.NavHostActivity @@ -48,7 +49,6 @@ import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.placeCursorAtEnd import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.setToolbarColors -import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel import org.mozilla.fenix.library.bookmarks.friendlyRootTitle diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt index 5df4e92cd..1121b254d 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt @@ -36,6 +36,7 @@ import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.lib.state.ext.flowScoped import mozilla.components.service.fxa.sync.SyncReason import mozilla.components.support.base.feature.UserInteractionHandler +import mozilla.components.support.ktx.kotlin.toShortUrl import mozilla.telemetry.glean.private.NoExtras import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity @@ -52,7 +53,6 @@ import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.runIfFragmentIsAttached import org.mozilla.fenix.ext.setTextColor -import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.library.LibraryPageFragment import org.mozilla.fenix.utils.allowUndo import org.mozilla.fenix.GleanMetrics.History as GleanHistory diff --git a/app/src/main/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragment.kt b/app/src/main/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragment.kt index 2d61d1cae..38c252175 100644 --- a/app/src/main/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragment.kt @@ -27,6 +27,7 @@ import kotlinx.coroutines.flow.map import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.lib.state.ext.flowScoped import mozilla.components.support.base.feature.UserInteractionHandler +import mozilla.components.support.ktx.kotlin.toShortUrl import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.addons.showSnackBar @@ -39,7 +40,6 @@ import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.runIfFragmentIsAttached import org.mozilla.fenix.ext.setTextColor import org.mozilla.fenix.ext.showToolbar -import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.library.LibraryPageFragment import org.mozilla.fenix.library.history.History import org.mozilla.fenix.library.historymetadata.controller.DefaultHistoryMetadataGroupController diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt index 57af576a8..ed5d0ca4e 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/AbstractBrowserTabViewHolder.kt @@ -29,6 +29,7 @@ import mozilla.components.concept.base.images.ImageLoadRequest import mozilla.components.concept.base.images.ImageLoader import mozilla.components.concept.engine.mediasession.MediaSession import mozilla.components.support.ktx.kotlin.MAX_URI_LENGTH +import mozilla.components.support.ktx.kotlin.toShortUrl import mozilla.telemetry.glean.private.NoExtras import org.mozilla.fenix.GleanMetrics.Tab import org.mozilla.fenix.R @@ -37,7 +38,6 @@ import org.mozilla.fenix.ext.increaseTapArea import org.mozilla.fenix.ext.removeAndDisable import org.mozilla.fenix.ext.removeTouchDelegate import org.mozilla.fenix.ext.showAndEnable -import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.selection.SelectionHolder import org.mozilla.fenix.tabstray.TabsTrayState import org.mozilla.fenix.tabstray.TabsTrayStore diff --git a/app/src/test/java/org/mozilla/fenix/ext/StringTest.kt b/app/src/test/java/org/mozilla/fenix/ext/StringTest.kt index cb1507491..948a211bd 100644 --- a/app/src/test/java/org/mozilla/fenix/ext/StringTest.kt +++ b/app/src/test/java/org/mozilla/fenix/ext/StringTest.kt @@ -4,24 +4,11 @@ package org.mozilla.fenix.ext -import mozilla.components.lib.publicsuffixlist.PublicSuffixList -import mozilla.components.support.test.robolectric.testContext import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue -import org.junit.Ignore import org.junit.Test -import org.junit.runner.RunWith -import org.mozilla.fenix.helpers.FenixRobolectricTestRunner -const val PUNYCODE = "xn--kpry57d" -const val IDN = "台灣" - -@RunWith(FenixRobolectricTestRunner::class) class StringTest { - private val publicSuffixList = PublicSuffixList(testContext) - @Test fun `Simplified Url`() { val urlTest = "https://www.amazon.com" @@ -29,219 +16,6 @@ class StringTest { assertEquals(new, "amazon.com") } - @Test - fun `when the full hostname cannot be displayed, elide labels starting from the front`() { - // See https://url.spec.whatwg.org/#url-rendering-elision - // See https://chromium.googlesource.com/chromium/src/+/master/docs/security/url_display_guidelines/url_display_guidelines.md#eliding-urls - - val display = "http://1.2.3.4.5.6.7.8.9.10.11.12.13.14.15.16.17.18.19.20.21.22.23.24.25.com" - .shortened() - - val split = display.split(".") - - // If the list ends with 25.com... - assertEquals("25", split.dropLast(1).last()) - // ...and each value is 1 larger than the last... - split.dropLast(1) - .map { it.toInt() } - .windowed(2, 1) - .forEach { (prev, next) -> - assertEquals(next, prev + 1) - } - // ...that means that all removed values came from the front of the list - } - - @Test - fun `the registrable domain is always displayed`() { - // https://url.spec.whatwg.org/#url-rendering-elision - // See https://chromium.googlesource.com/chromium/src/+/master/docs/security/url_display_guidelines/url_display_guidelines.md#eliding-urls - - val bigRegistrableDomain = "evil-but-also-shockingly-long-registrable-domain.com" - assertTrue("https://login.your-bank.com.$bigRegistrableDomain/enter/your/password".shortened().contains(bigRegistrableDomain)) - } - - @Test - fun `url username and password fields should not be displayed`() { - // See https://url.spec.whatwg.org/#url-rendering-simplification - // See https://chromium.googlesource.com/chromium/src/+/master/docs/security/url_display_guidelines/url_display_guidelines.md#simplify - - assertFalse("https://examplecorp.com@attacker.example/".shortened().contains("examplecorp")) - assertFalse("https://examplecorp.com@attacker.example/".shortened().contains("com")) - assertFalse("https://user:password@example.com/".shortened().contains("user")) - assertFalse("https://user:password@example.com/".shortened().contains("password")) - } - - @Test - fun `eTLDs should not be dropped`() { - // See https://bugzilla.mozilla.org/show_bug.cgi?id=1554984#c11 - "http://mozfreddyb.github.io/" shortenedShouldBecome "mozfreddyb.github.io" - "http://web.security.plumbing/" shortenedShouldBecome "web.security.plumbing" - } - - @Test - fun `ipv4 addresses should be returned as is`() { - // See https://bugzilla.mozilla.org/show_bug.cgi?id=1554984#c11 - "192.168.85.1" shortenedShouldBecome "192.168.85.1" - } - - @Test - fun `about buildconfig should not be modified`() { - // See https://bugzilla.mozilla.org/show_bug.cgi?id=1554984#c11 - "about:buildconfig" shortenedShouldBecome "about:buildconfig" - } - - @Test - fun `encoded userinfo should still be considered userinfo`() { - "https://user:password%40really.evil.domain%2F@mail.google.com" shortenedShouldBecome "mail.google.com" - } - - @Test - @Ignore("This would be more correct, but does not appear to be an attack vector") - fun `should decode DWORD IP addresses`() { - "https://16843009" shortenedShouldBecome "1.1.1.1" - } - - @Test - @Ignore("This would be more correct, but does not appear to be an attack vector") - fun `should decode octal IP addresses`() { - "https://000010.000010.000010.000010" shortenedShouldBecome "8.8.8.8" - } - - @Test - @Ignore("This would be more correct, but does not appear to be an attack vector") - fun `should decode hex IP addresses`() { - "http://0x01010101" shortenedShouldBecome "1.1.1.1" - } - - // BEGIN test cases borrowed from desktop (shortUrl is used for Top Sites on new tab) - // Test cases are modified, as we show the eTLD - // (https://searchfox.org/mozilla-central/source/browser/components/newtab/test/unit/lib/ShortUrl.test.js) - @Test - fun `should return a blank string if url is blank`() { - "" shortenedShouldBecome "" - } - - @Test - fun `should return the 'url' if not a valid url`() { - "something" shortenedShouldBecome "something" - "http:" shortenedShouldBecome "http:" - "http::double-colon" shortenedShouldBecome "http::double-colon" - // The largest allowed port is 65,535 - "http://badport:65536/" shortenedShouldBecome "http://badport:65536/" - } - - @Test - fun `should convert host to idn when calling shortURL`() { - "http://$PUNYCODE.blah.com" shortenedShouldBecome "$IDN.blah.com" - } - - @Test - fun `should get the hostname from url`() { - "http://bar.com" shortenedShouldBecome "bar.com" - } - - @Test - fun `should not strip out www if not first subdomain`() { - "http://foo.www.com" shortenedShouldBecome "foo.www.com" - "http://www.foo.www.com" shortenedShouldBecome "foo.www.com" - } - - @Test - fun `should convert to lowercase`() { - "HTTP://FOO.COM" shortenedShouldBecome "foo.com" - } - - @Test - fun `should not include the port`() { - "http://foo.com:8888" shortenedShouldBecome "foo.com" - } - - @Test - fun `should return hostname for localhost`() { - "http://localhost:8000/" shortenedShouldBecome "localhost" - } - - @Test - fun `should return hostname for ip address`() { - "http://127.0.0.1/foo" shortenedShouldBecome "127.0.0.1" - } - - @Test - fun `should return etld for www gov uk (www-only non-etld)`() { - "https://www.gov.uk/countersigning" shortenedShouldBecome "gov.uk" - } - - @Test - fun `should return idn etld for www-only non-etld`() { - "https://www.$PUNYCODE/foo" shortenedShouldBecome IDN - } - - @Test - fun `file uri should return input`() { - "file:///foo/bar.txt" shortenedShouldBecome "file:///foo/bar.txt" - } - - @Test - @Ignore("This behavior conflicts with https://bugzilla.mozilla.org/show_bug.cgi?id=1554984#c11") - fun `should return not the protocol for about`() { - "about:newtab" shortenedShouldBecome "newtab" - } - - @Test - fun `should fall back to full url as a last resort`() { - "about:" shortenedShouldBecome "about:" - } - // END test cases borrowed from desktop - - // BEGIN test cases borrowed from FFTV - // (https://searchfox.org/mozilla-mobile/source/firefox-echo-show/app/src/test/java/org/mozilla/focus/utils/TestFormattedDomain.java#228) - @Test - fun testIsIPv4RealAddress() { - assertTrue("192.168.1.1".isIpv4()) - assertTrue("8.8.8.8".isIpv4()) - assertTrue("63.245.215.20".isIpv4()) - } - - @Test - fun testIsIPv4WithProtocol() { - assertFalse("http://8.8.8.8".isIpv4()) - assertFalse("https://8.8.8.8".isIpv4()) - } - - @Test - fun testIsIPv4WithPort() { - assertFalse("8.8.8.8:400".isIpv4()) - assertFalse("8.8.8.8:1337".isIpv4()) - } - - @Test - fun testIsIPv4WithPath() { - assertFalse("8.8.8.8/index.html".isIpv4()) - assertFalse("8.8.8.8/".isIpv4()) - } - - @Test - fun testIsIPv4WithIPv6() { - assertFalse("2001:db8::1 ".isIpv4()) - assertFalse("2001:db8:0:1:1:1:1:1".isIpv4()) - assertFalse("[2001:db8:a0b:12f0::1]".isIpv4()) - assertFalse("2001:db8: 3333:4444:5555:6666:1.2.3.4".isIpv4()) - } - - @Test - fun testIsIPv6WithIPv6() { - assertTrue("2001:db8::1".isIpv6()) - assertTrue("2001:db8:0:1:1:1:1:1".isIpv6()) - } - - @Test - fun testIsIPv6WithIPv4() { - assertFalse("192.168.1.1".isIpv6()) - assertFalse("8.8.8.8".isIpv6()) - assertFalse("63.245.215.20".isIpv6()) - } - // END test cases borrowed from FFTV - @Test fun testReplaceConsecutiveZeros() { assertEquals( @@ -249,10 +23,4 @@ class StringTest { "2001:db8:0:0:0:ff00:42:8329".replaceConsecutiveZeros(), ) } - - private infix fun String.shortenedShouldBecome(expect: String) { - assertEquals(expect, this.shortened()) - } - - private fun String.shortened() = this.toShortUrl(publicSuffixList) }