Skip to content

Commit ffdc501

Browse files
joshliebejoshliebe
authored
Refactor tracking parameter removal (#4879)
Task/Issue URL: https://app.asana.com/0/1200204095367872/1208009677259118/f ### Description As mentioned in #4871, this adds some unit tests and cleans up the code in this class to make it a bit more readable. ### Steps to test this PR - [x] Visit http://privacy-test-pages.site/ - [x] Go to "Privacy Protections Tests” > “Query parameters" - [x] Tap on the links - [x] Verify that the parameters are stripped as expected
1 parent 01bf909 commit ffdc501

File tree

2 files changed

+128
-19
lines changed

2 files changed

+128
-19
lines changed

‎privacy-config/privacy-config-impl/src/main/java/com/duckduckgo/privacy/config/impl/features/trackingparameters/RealTrackingParameters.kt‎

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -62,42 +62,66 @@ class RealTrackingParameters @Inject constructor(
6262
if (!featureToggle.isFeatureEnabled(PrivacyFeatureName.TrackingParametersFeatureName.value)) return null
6363
if (isAnException(initiatingUrl, url)) return null
6464

65-
val trackingParameters = trackingParametersRepository.parameters
66-
6765
val parsedUri = Uri.parse(url)
66+
6867
// In some instances, particularly with ads, the query may represent a different URL (without encoding),
6968
// making it difficult to detect accurately.
7069
val query = parsedUri.query
7170
val queryUri = query?.toUri()
72-
val uri = if (queryUri?.isValid() == true) queryUri else parsedUri
7371

74-
try {
75-
val queryParameters = uri.queryParameterNames
72+
return if (queryUri?.isValid() == true) {
73+
cleanQueryUriParameters(url, query, queryUri)
74+
} else {
75+
cleanParsedUriParameters(parsedUri)
76+
}
77+
}
78+
79+
private fun cleanParsedUriParameters(uri: Uri): String? {
80+
return cleanUri(uri) { cleanedUrl ->
81+
cleanedUrl
82+
}
83+
}
7684

77-
if (queryParameters.isEmpty()) {
78-
return null
79-
}
80-
val preservedParameters = getPreservedParameters(queryParameters, trackingParameters)
81-
if (preservedParameters.size == queryParameters.size) {
82-
return null
83-
}
85+
private fun cleanQueryUriParameters(url: String, query: String, queryUri: Uri): String? {
86+
return cleanUri(queryUri) { interimCleanedUrl ->
87+
url.replace(query, interimCleanedUrl)
88+
}
89+
}
90+
91+
private fun cleanUri(uri: Uri, buildCleanedUrl: (String) -> String): String? {
92+
val trackingParameters = trackingParametersRepository.parameters
93+
94+
return try {
95+
val preservedParameters = getPreservedParameters(uri, trackingParameters) ?: return null
8496
val interimCleanedUrl = uri.replaceQueryParameters(preservedParameters).toString()
85-
val cleanedUrl = if (queryUri?.isValid() == true) url.replace(query, interimCleanedUrl) else interimCleanedUrl
97+
val cleanedUrl = buildCleanedUrl(interimCleanedUrl)
8698

8799
lastCleanedUrl = cleanedUrl
88-
89-
return cleanedUrl
100+
cleanedUrl
90101
} catch (exception: UnsupportedOperationException) {
91102
Timber.e("Tracking Parameter Removal: ${exception.message}")
103+
null
104+
}
105+
}
106+
107+
private fun getPreservedParameters(uri: Uri, trackingParameters: List<String>): List<String>? {
108+
val queryParameters = uri.queryParameterNames
109+
if (queryParameters.isEmpty()) {
92110
return null
93111
}
112+
val preservedParameters = filterNonTrackingParameters(queryParameters, trackingParameters)
113+
return if (preservedParameters.size == queryParameters.size) {
114+
null
115+
} else {
116+
preservedParameters
117+
}
94118
}
95119

96120
private fun Uri?.isValid(): Boolean {
97121
return this?.isAbsolute == true && this.isHierarchical
98122
}
99123

100-
private fun getPreservedParameters(
124+
private fun filterNonTrackingParameters(
101125
queryParameters: MutableSet<String>,
102126
trackingParameters: List<String>,
103127
) =

‎privacy-config/privacy-config-impl/src/test/java/com/duckduckgo/privacy/config/impl/features/trackingparameters/RealTrackingParametersTest.kt‎

Lines changed: 88 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,25 @@
1616

1717
package com.duckduckgo.privacy.config.impl.features.trackingparameters
1818

19+
import androidx.test.ext.junit.runners.AndroidJUnit4
1920
import com.duckduckgo.app.privacy.db.UserAllowListRepository
21+
import com.duckduckgo.feature.toggles.api.FeatureExceptions.FeatureException
2022
import com.duckduckgo.feature.toggles.api.FeatureToggle
23+
import com.duckduckgo.privacy.config.api.PrivacyFeatureName
2124
import com.duckduckgo.privacy.config.api.UnprotectedTemporary
2225
import com.duckduckgo.privacy.config.store.features.trackingparameters.TrackingParametersRepository
23-
import junit.framework.TestCase.assertTrue
26+
import junit.framework.TestCase.assertEquals
27+
import junit.framework.TestCase.assertNull
2428
import org.junit.Before
2529
import org.junit.Test
30+
import org.junit.runner.RunWith
2631
import org.mockito.ArgumentMatchers.anyString
32+
import org.mockito.kotlin.any
33+
import org.mockito.kotlin.eq
2734
import org.mockito.kotlin.mock
2835
import org.mockito.kotlin.whenever
2936

37+
@RunWith(AndroidJUnit4::class)
3038
class RealTrackingParametersTest {
3139

3240
private lateinit var testee: RealTrackingParameters
@@ -37,12 +45,89 @@ class RealTrackingParametersTest {
3745

3846
@Before
3947
fun setup() {
48+
givenFeatureEnabled(true)
49+
whenever(mockTrackingParametersRepository.exceptions).thenReturn(emptyList())
50+
whenever(mockTrackingParametersRepository.parameters).thenReturn(listOf(TRACKING_PARAMETER))
51+
whenever(mockUserAllowListRepository.isUrlInUserAllowList(anyString())).thenReturn(false)
52+
whenever(mockUnprotectedTemporary.isAnException(anyString())).thenReturn(false)
4053
testee = RealTrackingParameters(mockTrackingParametersRepository, mockFeatureToggle, mockUnprotectedTemporary, mockUserAllowListRepository)
4154
}
4255

4356
@Test
44-
fun whenIsExceptionCalledAndDomainIsInUserAllowListThenReturnTrue() {
57+
fun whenCleanTrackingParametersAndFeatureIsDisabledThenReturnNull() {
58+
givenFeatureEnabled(false)
59+
assertNull(testee.cleanTrackingParameters(null, URL))
60+
}
61+
62+
@Test
63+
fun whenCleanTrackingParametersAndUrlIsInAllowListThenReturnNull() {
4564
whenever(mockUserAllowListRepository.isUrlInUserAllowList(anyString())).thenReturn(true)
46-
assertTrue(testee.isAnException("foo.com", "test.com"))
65+
assertNull(testee.cleanTrackingParameters(null, URL))
66+
}
67+
68+
@Test
69+
fun whenCleanTrackingParametersAndUrlIsUnprotectedTemporaryExceptionThenReturnNull() {
70+
whenever(mockUnprotectedTemporary.isAnException(anyString())).thenReturn(true)
71+
assertNull(testee.cleanTrackingParameters(null, URL))
72+
}
73+
74+
@Test
75+
fun whenCleanTrackingParametersAndUrlDomainIsExceptionThenReturnNull() {
76+
whenever(mockTrackingParametersRepository.exceptions).thenReturn(listOf(FeatureException(domain = "example.com", reason = "reason")))
77+
assertNull(testee.cleanTrackingParameters(null, URL))
78+
}
79+
80+
@Test
81+
fun whenCleanTrackingParametersAndUrlSubdomainIsExceptionThenReturnNull() {
82+
whenever(mockTrackingParametersRepository.exceptions).thenReturn(listOf(FeatureException(domain = "example.com", reason = "reason")))
83+
assertNull(testee.cleanTrackingParameters(null, "https://sub.example.com?tracking_param=value&other=value"))
84+
}
85+
86+
@Test
87+
fun whenCleanTrackingParametersAndInitiatingUrlDomainIsExceptionThenReturnNull() {
88+
whenever(mockTrackingParametersRepository.exceptions).thenReturn(listOf(FeatureException(domain = "foo.com", reason = "reason")))
89+
assertNull(testee.cleanTrackingParameters("https://foo.com", URL))
90+
}
91+
92+
@Test
93+
fun whenCleanTrackingParametersAndInitiatingUrlSubdomainIsExceptionThenReturnNull() {
94+
whenever(mockTrackingParametersRepository.exceptions).thenReturn(listOf(FeatureException(domain = "foo.com", reason = "reason")))
95+
assertNull(testee.cleanTrackingParameters("https://sub.foo.com", URL))
96+
}
97+
98+
@Test
99+
fun whenCleanTrackingParametersAndUrlHasTrackingParametersThenReturnCleanedUrl() {
100+
val result = testee.cleanTrackingParameters(null, URL)
101+
assertEquals(CLEANED_URL, result)
102+
}
103+
104+
@Test
105+
fun whenCleanTrackingParametersAndUrlDoesNotHaveTrackingParametersThenReturnNull() {
106+
val result = testee.cleanTrackingParameters(null, CLEANED_URL)
107+
assertNull(result)
108+
}
109+
110+
@Test
111+
fun whenCleanTrackingParametersAndQueryIsValidUrlThenReturnCleanedUrl() {
112+
val expectedCleanedUrl = "https://example.com?$CLEANED_URL"
113+
val result = testee.cleanTrackingParameters(null, "https://example.com?$URL")
114+
assertEquals(expectedCleanedUrl, result)
115+
}
116+
117+
@Test
118+
fun whenCleanTrackingParametersThenSetLastCleanedUrl() {
119+
assertNull(testee.lastCleanedUrl)
120+
testee.cleanTrackingParameters(null, URL)
121+
assertEquals(CLEANED_URL, testee.lastCleanedUrl)
122+
}
123+
124+
private fun givenFeatureEnabled(enabled: Boolean) {
125+
whenever(mockFeatureToggle.isFeatureEnabled(eq(PrivacyFeatureName.TrackingParametersFeatureName.value), any())).thenReturn(enabled)
126+
}
127+
128+
companion object {
129+
private const val URL = "https://example.com?tracking_param=value&other=value"
130+
private const val CLEANED_URL = "https://example.com?other=value"
131+
private const val TRACKING_PARAMETER = "tracking_param"
47132
}
48133
}

0 commit comments

Comments
 (0)