From 4a6912d43616fca1e5584721f19390b54e68f998 Mon Sep 17 00:00:00 2001 From: Arturo Mejia Date: Sat, 24 Aug 2019 02:10:08 -0400 Subject: [PATCH] Closes #4191: Fixed the recommended() tracking category on SystemEngine --- .../browser/engine/system/SystemEngineView.kt | 22 +++- .../engine/system/matcher/UrlMatcher.kt | 23 ++-- .../engine/system/SystemEngineViewTest.kt | 123 ++++++++++++++++++ .../engine/system/matcher/UrlMatcherTest.kt | 104 ++++++++++----- .../concept/engine/EngineSession.kt | 3 + .../concept/engine/EngineSessionTest.kt | 26 ++++ docs/changelog.md | 3 + 7 files changed, 260 insertions(+), 44 deletions(-) diff --git a/components/browser/engine-system/src/main/java/mozilla/components/browser/engine/system/SystemEngineView.kt b/components/browser/engine-system/src/main/java/mozilla/components/browser/engine/system/SystemEngineView.kt index def6c0f1cbb..c3e2b673861 100644 --- a/components/browser/engine-system/src/main/java/mozilla/components/browser/engine/system/SystemEngineView.kt +++ b/components/browser/engine-system/src/main/java/mozilla/components/browser/engine/system/SystemEngineView.kt @@ -208,13 +208,18 @@ class SystemEngineView @JvmOverloads constructor( return WebResourceResponse(null, null, null) } - if (!request.isForMainFrame && - getOrCreateUrlMatcher(resources, it).matches(resourceUri, Uri.parse(session?.currentUrl))) { + val (matches, stringCategory) = getOrCreateUrlMatcher(resources, it).matches( + resourceUri, + Uri.parse(session?.currentUrl) + ) + + if (!request.isForMainFrame && matches) { session?.internalNotifyObservers { + val matchedCategories = stringCategory.toTrackingProtectionCategories() onTrackerBlocked( Tracker( resourceUri.toString(), - emptyList() + matchedCategories ) ) } @@ -729,9 +734,18 @@ class SystemEngineView @JvmOverloads constructor( UrlMatcher.SOCIAL to TrackingProtectionPolicy.TrackingCategory.SOCIAL ) + private fun String?.toTrackingProtectionCategories(): List { + val category = urlMatcherCategoryMap[this] + return if (category != null) { + listOf(category) + } else { + emptyList() + } + } + @Synchronized internal fun getOrCreateUrlMatcher(resources: Resources, policy: TrackingProtectionPolicy): UrlMatcher { - val categories = urlMatcherCategoryMap.filterValues { policy.trackingCategories.contains(it) }.keys + val categories = urlMatcherCategoryMap.filterValues { policy.contains(it) }.keys URL_MATCHER?.setCategoriesEnabled(categories) ?: run { URL_MATCHER = UrlMatcher.createMatcher( diff --git a/components/browser/engine-system/src/main/java/mozilla/components/browser/engine/system/matcher/UrlMatcher.kt b/components/browser/engine-system/src/main/java/mozilla/components/browser/engine/system/matcher/UrlMatcher.kt index 08fa9347fcc..e99531f4417 100644 --- a/components/browser/engine-system/src/main/java/mozilla/components/browser/engine/system/matcher/UrlMatcher.kt +++ b/components/browser/engine-system/src/main/java/mozilla/components/browser/engine/system/matcher/UrlMatcher.kt @@ -91,8 +91,10 @@ class UrlMatcher { * * @param resourceURI URI of a resource to be loaded by the page * @param pageURI URI of the page + * @return a [Pair] of the first indicates, if the URI matches and the second + * indicates the category of the match if available otherwise null. */ - fun matches(resourceURI: String, pageURI: String): Boolean { + fun matches(resourceURI: String, pageURI: String): Pair { return matches(Uri.parse(resourceURI), Uri.parse(pageURI)) } @@ -103,42 +105,45 @@ class UrlMatcher { * * @param resourceURI URI of a resource to be loaded by the page * @param pageURI URI of the page + * @return a [Pair] of the first indicates, if the URI matches and the second + * indicates the category of the match if available otherwise null. */ @Suppress("ReturnCount", "ComplexMethod") - fun matches(resourceURI: Uri, pageURI: Uri): Boolean { + fun matches(resourceURI: Uri, pageURI: Uri): Pair { val resourceURLString = resourceURI.toString() val resourceHost = resourceURI.host val pageHost = pageURI.host + val notMatchesFound = false to null if (previouslyUnmatched.contains(resourceURLString)) { - return false + return notMatchesFound } if (whiteList?.contains(pageURI, resourceURI) == true) { - return false + return notMatchesFound } if (pageHost != null && pageHost == resourceHost) { - return false + return notMatchesFound } if (previouslyMatched.contains(resourceURLString)) { - return true + return true to null } if (resourceHost == null) { - return false + return notMatchesFound } for ((key, value) in categories) { if (enabledCategories.contains(key) && value.findNode(resourceHost.reverse()) != null) { previouslyMatched.add(resourceURLString) - return true + return true to key } } previouslyUnmatched.add(resourceURLString) - return false + return notMatchesFound } companion object { diff --git a/components/browser/engine-system/src/test/java/mozilla/components/browser/engine/system/SystemEngineViewTest.kt b/components/browser/engine-system/src/test/java/mozilla/components/browser/engine/system/SystemEngineViewTest.kt index b016db22a29..ec70cf8ac52 100644 --- a/components/browser/engine-system/src/test/java/mozilla/components/browser/engine/system/SystemEngineViewTest.kt +++ b/components/browser/engine-system/src/test/java/mozilla/components/browser/engine/system/SystemEngineViewTest.kt @@ -69,6 +69,8 @@ import org.robolectric.annotation.Config import java.util.Calendar import java.util.Date import mozilla.components.concept.engine.EngineSession.TrackingProtectionPolicy.TrackingCategory +import mozilla.components.concept.engine.content.blocking.Tracker +import java.io.StringReader @RunWith(AndroidJUnit4::class) class SystemEngineViewTest { @@ -497,11 +499,111 @@ class SystemEngineViewTest { val blockedRequest = mock() whenever(blockedRequest.isForMainFrame).thenReturn(false) whenever(blockedRequest.url).thenReturn(Uri.parse("http://blocked.random")) + + var trackerBlocked: Tracker? = null + engineSession.register(object : EngineSession.Observer { + override fun onTrackerBlocked(tracker: Tracker) { + trackerBlocked = tracker + } + }) + response = webViewClient.shouldInterceptRequest(engineSession.webView, blockedRequest) assertNotNull(response) assertNull(response!!.data) assertNull(response.encoding) assertNull(response.mimeType) + assertTrue(trackerBlocked!!.trackingCategories.isEmpty()) + } + + @Test + fun `blocked trackers are reported with correct categories`() { + val BLOCK_LIST = """{ + "license": "test-license", + "categories": { + "Advertising": [ + { + "AdTest1": { + "http://www.adtest1.com/": [ + "adtest1.com" + ] + } + } + ], + "Analytics": [ + { + "AnalyticsTest": { + "http://analyticsTest1.com/": [ + "analyticsTest1.com" + ] + } + } + ], + "Content": [ + { + "ContentTest1": { + "http://contenttest1.com/": [ + "contenttest1.com" + ] + } + } + ], + "Social": [ + { + "SocialTest1": { + "http://www.socialtest1.com/": [ + "socialtest1.com" + ] + } + } + ] + } + } + """ + SystemEngineView.URL_MATCHER = UrlMatcher.createMatcher( + StringReader(BLOCK_LIST), + null, + StringReader("{}") + ) + + val engineSession = SystemEngineSession(testContext) + val engineView = SystemEngineView(testContext) + var trackerBlocked: Tracker? = null + + engineView.render(engineSession) + val webViewClient = engineSession.webView.webViewClient + + engineSession.trackingProtectionPolicy = TrackingProtectionPolicy.strict() + + engineSession.register(object : EngineSession.Observer { + override fun onTrackerBlocked(tracker: Tracker) { + trackerBlocked = tracker + } + }) + + val blockedRequest = mock() + whenever(blockedRequest.isForMainFrame).thenReturn(false) + + whenever(blockedRequest.url).thenReturn(Uri.parse("http://www.adtest1.com/")) + webViewClient.shouldInterceptRequest(engineSession.webView, blockedRequest) + + assertTrue(trackerBlocked!!.trackingCategories.first() == TrackingCategory.AD) + + whenever(blockedRequest.url).thenReturn(Uri.parse("http://analyticsTest1.com/")) + webViewClient.shouldInterceptRequest(engineSession.webView, blockedRequest) + + assertTrue(trackerBlocked!!.trackingCategories.first() == TrackingCategory.ANALYTICS) + + whenever(blockedRequest.url).thenReturn(Uri.parse("http://contenttest1.com/")) + webViewClient.shouldInterceptRequest(engineSession.webView, blockedRequest) + + assertTrue(trackerBlocked!!.trackingCategories.first() == TrackingCategory.CONTENT) + + whenever(blockedRequest.url).thenReturn(Uri.parse("http://www.socialtest1.com/")) + webViewClient.shouldInterceptRequest(engineSession.webView, blockedRequest) + + assertTrue(trackerBlocked!!.trackingCategories.first() == TrackingCategory.SOCIAL) + + SystemEngineView.URL_MATCHER = null } @Test @@ -923,6 +1025,27 @@ class SystemEngineViewTest { assertEquals(setOf(UrlMatcher.ADVERTISING, UrlMatcher.SOCIAL), urlMatcher.enabledCategories) } + @Test + fun `URL matcher supports compounded categories`() { + val recommendedPolicy = TrackingProtectionPolicy.recommended() + val strictPolicy = TrackingProtectionPolicy.strict() + val resources = testContext.resources + val recommendedCategories = setOf( + UrlMatcher.ADVERTISING, UrlMatcher.ANALYTICS, UrlMatcher.SOCIAL + ) + val strictCategories = setOf( + UrlMatcher.ADVERTISING, UrlMatcher.ANALYTICS, UrlMatcher.SOCIAL, UrlMatcher.CONTENT + ) + + var urlMatcher = SystemEngineView.getOrCreateUrlMatcher(resources, recommendedPolicy) + + assertEquals(recommendedCategories, urlMatcher.enabledCategories) + + urlMatcher = SystemEngineView.getOrCreateUrlMatcher(resources, strictPolicy) + + assertEquals(strictCategories, urlMatcher.enabledCategories) + } + @Test fun `permission requests are forwarded to observers`() { val permissionRequest: android.webkit.PermissionRequest = mock() diff --git a/components/browser/engine-system/src/test/java/mozilla/components/browser/engine/system/matcher/UrlMatcherTest.kt b/components/browser/engine-system/src/test/java/mozilla/components/browser/engine/system/matcher/UrlMatcherTest.kt index e66372c8e20..21c96a11cb3 100644 --- a/components/browser/engine-system/src/test/java/mozilla/components/browser/engine/system/matcher/UrlMatcherTest.kt +++ b/components/browser/engine-system/src/test/java/mozilla/components/browser/engine/system/matcher/UrlMatcherTest.kt @@ -6,6 +6,10 @@ package mozilla.components.browser.engine.system.matcher import android.net.Uri import androidx.test.ext.junit.runners.AndroidJUnit4 +import mozilla.components.browser.engine.system.matcher.UrlMatcher.Companion.ADVERTISING +import mozilla.components.browser.engine.system.matcher.UrlMatcher.Companion.ANALYTICS +import mozilla.components.browser.engine.system.matcher.UrlMatcher.Companion.CONTENT +import mozilla.components.browser.engine.system.matcher.UrlMatcher.Companion.SOCIAL import mozilla.components.support.test.any import mozilla.components.support.test.mock import org.junit.Assert.assertEquals @@ -27,24 +31,24 @@ class UrlMatcherTest { fun basicMatching() { val matcher = UrlMatcher(arrayOf("bcd.random")) - assertTrue(matcher.matches("http://bcd.random/something", "http://mozilla.org")) - assertTrue(matcher.matches("http://bcd.random", "http://mozilla.org")) - assertTrue(matcher.matches("http://www.bcd.random", "http://mozilla.org")) - assertTrue(matcher.matches("http://www.bcd.random/something", "http://mozilla.org")) - assertTrue(matcher.matches("http://foobar.bcd.random", "http://mozilla.org")) - assertTrue(matcher.matches("http://foobar.bcd.random/something", "http://mozilla.org")) - - assertFalse(matcher.matches("http://other.random", "http://mozilla.org")) - assertFalse(matcher.matches("http://other.random/something", "http://mozilla.org")) - assertFalse(matcher.matches("http://www.other.random", "http://mozilla.org")) - assertFalse(matcher.matches("http://www.other.random/something", "http://mozilla.org")) - assertFalse(matcher.matches("http://bcd.specific", "http://mozilla.org")) - assertFalse(matcher.matches("http://bcd.specific/something", "http://mozilla.org")) - assertFalse(matcher.matches("http://www.bcd.specific", "http://mozilla.org")) - assertFalse(matcher.matches("http://www.bcd.specific/something", "http://mozilla.org")) - - assertFalse(matcher.matches("http://mozilla.org/resource", "data:text/html;stuff here")) - assertTrue(matcher.matches("http://bcd.random/resource", "data:text/html;stuff here")) + assertTrue(matcher.matches("http://bcd.random/something", "http://mozilla.org").first) + assertTrue(matcher.matches("http://bcd.random", "http://mozilla.org").first) + assertTrue(matcher.matches("http://www.bcd.random", "http://mozilla.org").first) + assertTrue(matcher.matches("http://www.bcd.random/something", "http://mozilla.org").first) + assertTrue(matcher.matches("http://foobar.bcd.random", "http://mozilla.org").first) + assertTrue(matcher.matches("http://foobar.bcd.random/something", "http://mozilla.org").first) + + assertFalse(matcher.matches("http://other.random", "http://mozilla.org").first) + assertFalse(matcher.matches("http://other.random/something", "http://mozilla.org").first) + assertFalse(matcher.matches("http://www.other.random", "http://mozilla.org").first) + assertFalse(matcher.matches("http://www.other.random/something", "http://mozilla.org").first) + assertFalse(matcher.matches("http://bcd.specific", "http://mozilla.org").first) + assertFalse(matcher.matches("http://bcd.specific/something", "http://mozilla.org").first) + assertFalse(matcher.matches("http://www.bcd.specific", "http://mozilla.org").first) + assertFalse(matcher.matches("http://www.bcd.specific/something", "http://mozilla.org").first) + + assertFalse(matcher.matches("http://mozilla.org/resource", "data:text/html;stuff here").first) + assertTrue(matcher.matches("http://bcd.random/resource", "data:text/html;stuff here").first) } /** @@ -103,7 +107,8 @@ class UrlMatcherTest { val enabled = currentBit and categoryPattern == currentBit val url = "http://category$currentCategory.com" assertEquals("Incorrect category matched for combo=$categoryPattern url=$url", - enabled, matcher.matches(url, "http://www.mozilla.org")) + enabled, matcher.matches(url, "http://www.mozilla.org").first + ) } } } @@ -228,25 +233,57 @@ class UrlMatcherTest { listOf(StringReader(OVERRIDES)), StringReader(WHITE_LIST)) - assertTrue(matcher.matches("http://adtest1.com", "http://www.adtest1.com")) - assertTrue(matcher.matches("http://adtest1.de", "http://www.adtest1.com")) + // Check returns correct category + val (matchesAds, categoryAds) = matcher.matches("http://adtest1.com", "http://www.adtest1.com") + + assertTrue(matchesAds) + assertEquals(categoryAds, ADVERTISING) + + val (matchesAds2, categoryAd2) = matcher.matches("http://adtest1.de", "http://www.adtest1.com") + + assertTrue(matchesAds2) + assertEquals(categoryAd2, ADVERTISING) + + val (matchesSocial, categorySocial) = matcher.matches( + "http://socialtest1.com/", + "http://www.socialtest1.com/" + ) + + assertTrue(matchesSocial) + assertEquals(categorySocial, SOCIAL) + + val (matchesContent, categoryContent) = matcher.matches( + "http://contenttest1.com/", + "http://www.contenttest1.com/" + ) + + assertTrue(matchesContent) + assertEquals(categoryContent, CONTENT) + + val (matchesAnalytics, categoryAnalytics) = matcher.matches( + "http://analyticsTest1.com/", + "http://www.analyticsTest1.com/" + ) + + assertTrue(matchesAnalytics) + assertEquals(categoryAnalytics, ANALYTICS) // Check that override worked - assertTrue(matcher.matches("http://adtest2.com", "http://www.adtest2.com")) - assertTrue(matcher.matches("http://adtest2.at", "http://www.adtest2.com")) - assertTrue(matcher.matches("http://adtest2.de", "http://www.adtest2.com")) + assertTrue(matcher.matches("http://adtest2.com", "http://www.adtest2.com").first) + assertTrue(matcher.matches("http://adtest2.at", "http://www.adtest2.com").first) + assertTrue(matcher.matches("http://adtest2.de", "http://www.adtest2.com").first) // Check that white list worked - assertTrue(matcher.matches("http://socialtest1.com", "http://www.socialtest1.com")) - assertFalse(matcher.matches("http://socialtest1.de", "http://www.socialtest1.com")) + assertTrue(matcher.matches("http://socialtest1.com", "http://www.socialtest1.com").first) + assertFalse(matcher.matches("http://socialtest1.de", "http://www.socialtest1.com").first) // Check ignored categories - assertFalse(matcher.matches("http://ignored1.de", "http://www.ignored1.com")) - assertFalse(matcher.matches("http://ignored2.de", "http://www.ignored2.com")) + assertFalse(matcher.matches("http://ignored1.de", "http://www.ignored1.com").first) + assertFalse(matcher.matches("http://ignored2.de", "http://www.ignored2.com").first) // Check that we find the social URIs we moved from Disconnect - assertTrue(matcher.matches("http://facebook.com", "http://www.facebook.com")) - assertFalse(matcher.matches("http://disconnect1.com", "http://www.disconnect1.com")) + assertTrue(matcher.matches("http://facebook.com", "http://www.facebook.com").first) + assertFalse(matcher.matches("http://disconnect1.com", "http://www.disconnect1.com").first) } @Test @@ -286,6 +323,11 @@ class UrlMatcherTest { StringReader(WHITE_LIST), setOf(UrlMatcher.ADVERTISING, UrlMatcher.ANALYTICS, UrlMatcher.SOCIAL, UrlMatcher.CONTENT)) - assertFalse(matcher.matches("http://mozilla.org/fonts/test.woff2", "http://mozilla.org")) + assertFalse( + matcher.matches( + "http://mozilla.org/fonts/test.woff2", + "http://mozilla.org" + ).first + ) } } diff --git a/components/concept/engine/src/main/java/mozilla/components/concept/engine/EngineSession.kt b/components/concept/engine/src/main/java/mozilla/components/concept/engine/EngineSession.kt index 2ae80290ce2..602709f2118 100644 --- a/components/concept/engine/src/main/java/mozilla/components/concept/engine/EngineSession.kt +++ b/components/concept/engine/src/main/java/mozilla/components/concept/engine/EngineSession.kt @@ -271,6 +271,9 @@ abstract class EngineSession( override fun hashCode() = trackingCategories.sumBy { it.id } + safeBrowsingCategories.sumBy { it.id } + cookiePolicy.id + + fun contains(category: TrackingCategory) = + (trackingCategories.sumBy { it.id } and category.id) != 0 } /** diff --git a/components/concept/engine/src/test/java/mozilla/components/concept/engine/EngineSessionTest.kt b/components/concept/engine/src/test/java/mozilla/components/concept/engine/EngineSessionTest.kt index b67ab2385db..54e3552f032 100644 --- a/components/concept/engine/src/test/java/mozilla/components/concept/engine/EngineSessionTest.kt +++ b/components/concept/engine/src/test/java/mozilla/components/concept/engine/EngineSessionTest.kt @@ -688,6 +688,32 @@ class EngineSessionTest { verifyZeroInteractions(observer) } + + @Test + fun `trackingProtectionPolicy contains should work with compound categories`() { + val recommendedPolicy = TrackingProtectionPolicy.recommended() + + assertTrue(recommendedPolicy.contains(TrackingCategory.RECOMMENDED)) + assertTrue(recommendedPolicy.contains(TrackingCategory.AD)) + assertTrue(recommendedPolicy.contains(TrackingCategory.ANALYTICS)) + assertTrue(recommendedPolicy.contains(TrackingCategory.SOCIAL)) + assertTrue(recommendedPolicy.contains(TrackingCategory.TEST)) + + assertFalse(recommendedPolicy.contains(TrackingCategory.FINGERPRINTING)) + assertFalse(recommendedPolicy.contains(TrackingCategory.CRYPTOMINING)) + assertFalse(recommendedPolicy.contains(TrackingCategory.CONTENT)) + + val strictPolicy = TrackingProtectionPolicy.strict() + + assertTrue(strictPolicy.contains(TrackingCategory.RECOMMENDED)) + assertTrue(strictPolicy.contains(TrackingCategory.AD)) + assertTrue(strictPolicy.contains(TrackingCategory.ANALYTICS)) + assertTrue(strictPolicy.contains(TrackingCategory.SOCIAL)) + assertTrue(strictPolicy.contains(TrackingCategory.TEST)) + assertTrue(strictPolicy.contains(TrackingCategory.FINGERPRINTING)) + assertTrue(strictPolicy.contains(TrackingCategory.CRYPTOMINING)) + assertTrue(strictPolicy.contains(TrackingCategory.CONTENT)) + } } open class DummyEngineSession : EngineSession() { diff --git a/docs/changelog.md b/docs/changelog.md index cdf4cb24493..8af7fe19ec0 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -34,6 +34,9 @@ permalink: /changelog/ * **concept-engine** * Added `WebNotification` data class for the web notifications API. +* **browser-engine-system** + * Fixed issue [4191](https://github.com/mozilla-mobile/android-components/issues/4191) where the `recommended()` tracking category was not getting applied for `SystemEngine`. + # 9.0.0 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v8.0.0...v9.0.0)