For #22770 - Use String.toShortUrl from A-C

fork
Alexandru2909 1 year ago committed by mergify[bot]
parent 9361fb1bb8
commit 3135062936

@ -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,

@ -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,

@ -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

@ -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.
*

@ -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

@ -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

@ -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

@ -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

@ -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

@ -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)
}

Loading…
Cancel
Save