From 1c2ccd53054f9cbb550122570772bcdbad225b7a Mon Sep 17 00:00:00 2001 From: David Perez Date: Wed, 18 Sep 2024 09:52:28 -0500 Subject: [PATCH] Add logic to ensure url has a valid https protocol (#3933) --- .../parser/AccessibilityParserImpl.kt | 15 +++++- .../data/platform/util/StringExtensions.kt | 6 +++ .../parser/AccessibilityParserTest.kt | 48 ++++++++++++++++++- .../data/util/StringExtensionsTest.kt | 16 +++++++ 4 files changed, 82 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/parser/AccessibilityParserImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/parser/AccessibilityParserImpl.kt index 6bd403ab2..5c8dd9056 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/parser/AccessibilityParserImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/parser/AccessibilityParserImpl.kt @@ -5,6 +5,8 @@ import android.view.accessibility.AccessibilityNodeInfo import androidx.core.net.toUri import com.x8bit.bitwarden.data.autofill.accessibility.model.FillableFields import com.x8bit.bitwarden.data.autofill.accessibility.util.getSupportedBrowserOrNull +import com.x8bit.bitwarden.data.autofill.accessibility.util.toUriOrNull +import com.x8bit.bitwarden.data.platform.util.hasHttpProtocol /** * The default implementation for the [AccessibilityParser]. @@ -29,10 +31,19 @@ class AccessibilityParserImpl : AccessibilityParser { rootNode .findAccessibilityNodeInfosByViewId("$packageName:id/$viewId") .map { accessibilityNodeInfo -> - browser.urlExtractor(accessibilityNodeInfo.text.toString()) + browser + .urlExtractor(accessibilityNodeInfo.text.toString()) + ?.trim() + ?.let { rawUrl -> + if (rawUrl.contains(other = ".") && !rawUrl.hasHttpProtocol()) { + "https://$rawUrl" + } else { + rawUrl + } + } } } .firstOrNull() - ?.toUri() + ?.toUriOrNull() } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/util/StringExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/util/StringExtensions.kt index ba0da0bf1..243248497 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/util/StringExtensions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/util/StringExtensions.kt @@ -26,6 +26,12 @@ fun String.toUriOrNull(): URI? = fun String.isAndroidApp(): Boolean = this.startsWith(ANDROID_APP_PROTOCOL) +/** + * Whether this [String] starts with an http or https protocol. + */ +fun String.hasHttpProtocol(): Boolean = + this.startsWith(prefix = "http://") || this.startsWith(prefix = "https://") + /** * Try and extract the web host from this [String] if it represents an Android app. */ diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/parser/AccessibilityParserTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/parser/AccessibilityParserTest.kt index 163df4482..a6f74798f 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/parser/AccessibilityParserTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/parser/AccessibilityParserTest.kt @@ -61,7 +61,7 @@ class AccessibilityParserTest { @Suppress("MaxLineLength") @Test - fun `parseForUriOrPackageName should return the site url as a URI when package is a supported browser and URL is found`() { + fun `parseForUriOrPackageName should return the site url un-augmented with https protocol as a URI when package is a supported browser and URL is found`() { val testBrowser = Browser(packageName = "com.android.chrome", urlFieldId = "url_bar") val url = "https://www.google.com" val urlNode = mockk { @@ -81,4 +81,50 @@ class AccessibilityParserTest { assertEquals(expectedResult, result) } + + @Suppress("MaxLineLength") + @Test + fun `parseForUriOrPackageName should return the site url un-augmented with http protocol as a URI when package is a supported browser and URL is found`() { + val testBrowser = Browser(packageName = "com.android.chrome", urlFieldId = "url_bar") + val url = "http://www.google.com" + val urlNode = mockk { + every { text } returns url + } + val rootNode = mockk { + every { packageName } returns testBrowser.packageName + every { + findAccessibilityNodeInfosByViewId( + "$packageName:id/${testBrowser.possibleUrlFieldIds.first()}", + ) + } returns listOf(urlNode) + } + val expectedResult = url.toUri() + + val result = accessibilityParser.parseForUriOrPackageName(rootNode = rootNode) + + assertEquals(expectedResult, result) + } + + @Suppress("MaxLineLength") + @Test + fun `parseForUriOrPackageName should return the site url augmented with https protocol as a URI when package is a supported browser and URL is found`() { + val testBrowser = Browser(packageName = "com.android.chrome", urlFieldId = "url_bar") + val url = "www.google.com" + val urlNode = mockk { + every { text } returns url + } + val rootNode = mockk { + every { packageName } returns testBrowser.packageName + every { + findAccessibilityNodeInfosByViewId( + "$packageName:id/${testBrowser.possibleUrlFieldIds.first()}", + ) + } returns listOf(urlNode) + } + val expectedResult = "https://$url".toUri() + + val result = accessibilityParser.parseForUriOrPackageName(rootNode = rootNode) + + assertEquals(expectedResult, result) + } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/util/StringExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/util/StringExtensionsTest.kt index 901985763..fa064d77c 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/util/StringExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/util/StringExtensionsTest.kt @@ -4,6 +4,7 @@ import com.x8bit.bitwarden.data.platform.manager.ResourceCacheManager import com.x8bit.bitwarden.data.platform.util.findLastSubstringIndicesOrNull import com.x8bit.bitwarden.data.platform.util.getDomainOrNull import com.x8bit.bitwarden.data.platform.util.getWebHostFromAndroidUriOrNull +import com.x8bit.bitwarden.data.platform.util.hasHttpProtocol import com.x8bit.bitwarden.data.platform.util.isAndroidApp import com.x8bit.bitwarden.data.platform.util.parseDomainOrNull import com.x8bit.bitwarden.data.platform.util.toUriOrNull @@ -48,6 +49,21 @@ class StringExtensionsTest { assertFalse("com.x8bit.bitwarden".isAndroidApp()) } + @Test + fun `hasHttpProtocol should return false when doesn't start with an appropriate protocol`() { + assertFalse("androidapp://com.x8bit.bitwarden".hasHttpProtocol()) + } + + @Test + fun `hasHttpProtocol should return true when it starts with the http protocol`() { + assertTrue("http://www.google.com".hasHttpProtocol()) + } + + @Test + fun `hasHttpProtocol should return true when it starts with the https protocol`() { + assertTrue("https://www.google.com".hasHttpProtocol()) + } + @Test fun `getWebHostFromAndroidUriOrNull should return null when not android app`() { assertNull("com.x8bit.bitwarden".getWebHostFromAndroidUriOrNull())