Skip to content

Commit 58eb48b

Browse files
Merge branch 'release/5.233.0'
2 parents defd853 + dc8ae12 commit 58eb48b

File tree

356 files changed

+6568
-2992
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

356 files changed

+6568
-2992
lines changed

.github/workflows/privacy.yml

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,16 @@ name: Privacy Tests
22

33
on:
44
pull_request:
5-
types: [ labeled ]
5+
types:
6+
- opened
7+
- synchronize
8+
- reopened
9+
paths:
10+
- 'node_modules/@duckduckgo/content-scope-scripts/**'
11+
612
schedule:
713
- cron: '0 3 * * *' # run at 3 AM UTC
14+
815
workflow_dispatch:
916

1017
concurrency:
@@ -13,7 +20,7 @@ concurrency:
1320

1421
jobs:
1522
privacy_tests:
16-
if: ${{ contains(github.event.pull_request.labels.*.name, 'content scope scripts') || github.event.schedule == '0 3 * * *' || github.event_name == 'workflow_dispatch' }}
23+
if: ${{ github.event_name == 'workflow_dispatch' || github.event_name == 'schedule' || github.event_name == 'pull_request' }}
1724
name: Privacy Tests
1825
runs-on: ubuntu-latest
1926

anvil/anvil-annotations/src/main/java/com/duckduckgo/anvil/annotations/ContributesRemoteFeature.kt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,6 @@ annotation class ContributesRemoteFeature(
5858
@Deprecated("Not needed anymore. Settings is now supported in top-level and sub-features and Toggle#getSettings returns it")
5959
val settingsStore: KClass<*> = Unit::class,
6060

61-
/** The class that implements the [FeatureExceptions.Store] interface */
62-
val exceptionsStore: KClass<*> = Unit::class,
63-
6461
/** The class that implements the [Toggle.Store] interface */
6562
val toggleStore: KClass<*> = Unit::class,
6663
)

anvil/anvil-compiler/src/main/java/com/duckduckgo/anvil/compiler/ContributesRemoteFeatureCodeGenerator.kt

Lines changed: 15 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -152,27 +152,6 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
152152
.build(),
153153
)
154154
}
155-
if (!customStorePresence.exceptionStorePresent) {
156-
addFunction(
157-
FunSpec.builder("providesNoopExceptionsStore")
158-
.addAnnotation(Provides::class.asClassName())
159-
.addAnnotation(
160-
AnnotationSpec.builder(RemoteFeatureStoreNamed::class.asClassName())
161-
.addMember("value = %T::class", boundType.asClassName())
162-
.build(),
163-
)
164-
.addCode(
165-
CodeBlock.of(
166-
"""
167-
return %T.EMPTY_STORE
168-
""".trimIndent(),
169-
FeatureExceptions::class.asClassName(),
170-
),
171-
)
172-
.returns(FeatureExceptions.Store::class.asClassName())
173-
.build(),
174-
)
175-
}
176155
addFunction(
177156
FunSpec.builder("provides${boundType.shortName}Inventory")
178157
.addAnnotation(Provides::class.asClassName())
@@ -264,20 +243,6 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
264243
.primaryConstructor(
265244
FunSpec.constructorBuilder()
266245
.addAnnotation(AnnotationSpec.builder(Inject::class).build())
267-
.addParameter(
268-
ParameterSpec
269-
.builder(
270-
"exceptionStore",
271-
FeatureExceptions.Store::class,
272-
)
273-
.addAnnotation(
274-
AnnotationSpec
275-
.builder(RemoteFeatureStoreNamed::class).addMember("value = %T::class", boundType.asClassName())
276-
.build(),
277-
278-
)
279-
.build(),
280-
)
281246
.addParameter(
282247
ParameterSpec
283248
.builder(
@@ -298,16 +263,6 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
298263
.addParameter("context", context.asClassName(module))
299264
.build(),
300265
)
301-
.addProperty(
302-
PropertySpec
303-
.builder(
304-
"exceptionStore",
305-
FeatureExceptions.Store::class,
306-
KModifier.PRIVATE,
307-
)
308-
.initializer("exceptionStore")
309-
.build(),
310-
)
311266
.addProperty(
312267
PropertySpec
313268
.builder(
@@ -400,18 +355,6 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
400355
.returns(FeatureSettings.Store::class.java.asClassName())
401356
.build(),
402357
)
403-
.addFunction(
404-
FunSpec.builder("bindOptionalExceptionsStore")
405-
.addModifiers(KModifier.ABSTRACT)
406-
.addAnnotation(BindsOptionalOf::class.asClassName())
407-
.addAnnotation(
408-
AnnotationSpec.builder(RemoteFeatureStoreNamed::class.asClassName())
409-
.addMember("value = %T::class", boundType.asClassName())
410-
.build(),
411-
)
412-
.returns(FeatureExceptions.Store::class.java.asClassName())
413-
.build(),
414-
)
415358
.build(),
416359
)
417360
}
@@ -487,8 +430,6 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
487430
}
488431
489432
val exceptions = parseExceptions(feature.exceptions)
490-
// TODO: Remove once migrating everything to getExceptions()
491-
exceptionStore.insertAll(exceptions)
492433
493434
val isEnabled = (feature.state == "enabled") || (appBuildConfig.flavor == %T && feature.state == "internal")
494435
this.feature.get().invokeMethod("self").setRawStoredState(
@@ -656,11 +597,11 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
656597
}
657598
return featureExceptions.toList()
658599
""".trimIndent(),
659-
FeatureExceptions.FeatureException::class.fqName.asClassName(module),
660-
FeatureExceptions.FeatureException::class.fqName.asClassName(module),
600+
FeatureException::class.fqName.asClassName(module),
601+
FeatureException::class.fqName.asClassName(module),
661602
).build(),
662603
)
663-
.returns(List::class.asClassName().parameterizedBy(FeatureExceptions.FeatureException::class.asClassName()))
604+
.returns(List::class.asClassName().parameterizedBy(FeatureException::class.asClassName()))
664605
.build()
665606
}
666607

@@ -1025,7 +966,6 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
1025966
}
1026967
}
1027968

1028-
var exceptionStore = false
1029969
var settingsStore = false
1030970
var toggleStore = false
1031971

@@ -1080,20 +1020,6 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
10801020
requireFeatureAndStoreCrossReference(vmClass, this)
10811021
}
10821022
}
1083-
with(annotation.exceptionsStoreOrNull()) {
1084-
exceptionStore = this != null
1085-
if (this != null) {
1086-
if (this.directSuperTypeReferences()
1087-
.none { it.asClassReferenceOrNull()?.fqName == FeatureExceptions.Store::class.fqName }
1088-
) {
1089-
throw AnvilCompilationException(
1090-
"${vmClass.fqName} [exceptionsStore] must extend [FeatureExceptions.Store]",
1091-
element = vmClass.clazz.identifyingElement,
1092-
)
1093-
}
1094-
requireFeatureAndStoreCrossReference(vmClass, this)
1095-
}
1096-
}
10971023
with(annotation.toggleStoreOrNull()) {
10981024
toggleStore = this != null
10991025
if (this != null) {
@@ -1152,30 +1078,33 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
11521078
}
11531079

11541080
return CustomStorePresence(
1155-
exceptionStorePresent = exceptionStore,
11561081
toggleStorePresent = toggleStore,
11571082
settingsStorePresent = settingsStore,
11581083
)
11591084
}
11601085

1086+
private enum class ContributesRemoteFeatureValues {
1087+
SCOPE,
1088+
BOUND_TYPE,
1089+
FEATURE_NAME,
1090+
SETTINGS_STORE,
1091+
TOGGLE_STORE,
1092+
}
1093+
11611094
private fun AnnotationReference.remoteFeatureStoreValueOrNull(): ClassReference? {
1162-
return argumentAt("value", 0)?.value()
1095+
return argumentAt("value", ContributesRemoteFeatureValues.SCOPE.ordinal)?.value()
11631096
}
11641097

11651098
private fun AnnotationReference.featureNameOrNull(): String? {
1166-
return argumentAt("featureName", 2)?.value()
1099+
return argumentAt("featureName", ContributesRemoteFeatureValues.FEATURE_NAME.ordinal)?.value()
11671100
}
11681101

11691102
private fun AnnotationReference.settingsStoreOrNull(): ClassReference? {
1170-
return argumentAt("settingsStore", 3)?.value()
1171-
}
1172-
1173-
private fun AnnotationReference.exceptionsStoreOrNull(): ClassReference? {
1174-
return argumentAt("exceptionsStore", 4)?.value()
1103+
return argumentAt("settingsStore", ContributesRemoteFeatureValues.SETTINGS_STORE.ordinal)?.value()
11751104
}
11761105

11771106
private fun AnnotationReference.toggleStoreOrNull(): ClassReference? {
1178-
return argumentAt("toggleStore", 5)?.value()
1107+
return argumentAt("toggleStore", ContributesRemoteFeatureValues.TOGGLE_STORE.ordinal)?.value()
11791108
}
11801109

11811110
private fun ClassReference.declaredFunctions(): List<MemberFunctionReference> {
@@ -1208,6 +1137,5 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
12081137

12091138
private data class CustomStorePresence(
12101139
val settingsStorePresent: Boolean = false,
1211-
val exceptionStorePresent: Boolean = false,
12121140
val toggleStorePresent: Boolean = false,
12131141
)

app-tracking-protection/vpn-impl/src/main/java/com/duckduckgo/mobile/android/vpn/blocklist/AppTrackerListUpdateWorker.kt

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,7 @@ class AppTrackerListUpdateWorker(context: Context, workerParameters: WorkerParam
7373
vpnDatabase.vpnAppTrackerBlockingDao().getTrackerBlocklistMetadata()?.eTag
7474
val updatedEtag = blocklist.etag.value
7575

76-
if (updatedEtag == currentEtag) {
77-
logcat { "Downloaded blocklist has same eTag, noop" }
78-
return Result.success()
79-
}
80-
81-
logcat { "Updating the app tracker blocklist, eTag: ${blocklist.etag.value}" }
76+
logcat { "Updating the app tracker blocklist, previous/new eTag: $currentEtag / $updatedEtag" }
8277

8378
vpnDatabase
8479
.vpnAppTrackerBlockingDao()

app-tracking-protection/vpn-impl/src/main/java/com/duckduckgo/mobile/android/vpn/feature/settings/ExceptionListsSettingStore.kt

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,7 @@ class ExceptionListsSettingStore @Inject constructor(
5050
override fun store(jsonString: String) {
5151
logcat { "Received configuration: $jsonString" }
5252
runCatching {
53-
jsonAdapter.fromJson(jsonString)?.let { model ->
54-
55-
val exceptionLists = model.exceptionLists
53+
jsonAdapter.fromJson(jsonString)?.exceptionLists?.let { exceptionLists ->
5654

5755
val appTrackerExceptionRuleList = exceptionLists.appTrackerAllowList.map { appTrackerAllowRule ->
5856
AppTrackerExceptionRule(
@@ -78,7 +76,7 @@ class ExceptionListsSettingStore @Inject constructor(
7876
}
7977

8078
private data class JsonConfigModel(
81-
val exceptionLists: JsonExceptionListsModel,
79+
val exceptionLists: JsonExceptionListsModel?,
8280
)
8381

8482
private data class JsonExceptionListsModel(

app-tracking-protection/vpn-impl/src/main/java/com/duckduckgo/mobile/android/vpn/integration/NgVpnNetworkStack.kt

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,9 +234,15 @@ class NgVpnNetworkStack @Inject constructor(
234234

235235
tunnelThread = Thread {
236236
logcat { "Running tunnel in context $jniContext" }
237-
vpnNetwork.run(jniContext, tunfd)
238-
logcat(LogPriority.WARN) { "Tunnel exited" }
239-
tunnelThread = null
237+
try {
238+
vpnNetwork.run(jniContext, tunfd)
239+
} catch (t: Throwable) {
240+
logcat(LogPriority.ERROR) { "Tunnel thread crashed: ${t.asLog()}" }
241+
deviceShieldPixels.reportTunnelThreadAbnormalCrash()
242+
} finally {
243+
tunnelThread = null
244+
logcat(LogPriority.WARN) { "Tunnel exited" }
245+
}
240246
}.also { it.start() }
241247

242248
logcat { "Started tunnel thread" }

app-tracking-protection/vpn-impl/src/main/java/com/duckduckgo/mobile/android/vpn/pixels/DeviceShieldPixelNames.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,9 @@ enum class DeviceShieldPixelNames(override val pixelName: String, val enqueue: B
212212
ATP_REPORT_TUNNEL_THREAD_STOP_TIMEOUT("m_atp_ev_apptp_tunnel_thread_stop_timeout_c", enqueue = true),
213213
ATP_REPORT_TUNNEL_THREAD_STOP_TIMEOUT_DAILY("m_atp_ev_apptp_tunnel_thread_stop_timeout_d", enqueue = true),
214214

215+
ATP_REPORT_TUNNEL_THREAD_STOP_CRASH("m_atp_ev_apptp_tunnel_thread_crash_c", enqueue = true),
216+
ATP_REPORT_TUNNEL_THREAD_CRASH_DAILY("m_atp_ev_apptp_tunnel_thread_crash_d", enqueue = true),
217+
215218
REPORT_VPN_ALWAYS_ON_TRIGGERED("m_vpn_ev_always_on_triggered_c"),
216219
REPORT_VPN_ALWAYS_ON_TRIGGERED_DAILY("m_vpn_ev_always_on_triggered_d"),
217220

app-tracking-protection/vpn-impl/src/main/java/com/duckduckgo/mobile/android/vpn/pixels/DeviceShieldPixels.kt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,8 @@ interface DeviceShieldPixels {
364364

365365
fun reportTunnelThreadStopTimeout()
366366

367+
fun reportTunnelThreadAbnormalCrash()
368+
367369
fun reportVpnAlwaysOnTriggered()
368370

369371
fun notifyStartFailed()
@@ -883,6 +885,11 @@ class RealDeviceShieldPixels @Inject constructor(
883885
firePixel(DeviceShieldPixelNames.ATP_REPORT_TUNNEL_THREAD_STOP_TIMEOUT)
884886
}
885887

888+
override fun reportTunnelThreadAbnormalCrash() {
889+
tryToFireDailyPixel(DeviceShieldPixelNames.ATP_REPORT_TUNNEL_THREAD_CRASH_DAILY)
890+
firePixel(DeviceShieldPixelNames.ATP_REPORT_TUNNEL_THREAD_STOP_CRASH)
891+
}
892+
886893
override fun reportVpnAlwaysOnTriggered() {
887894
tryToFireDailyPixel(DeviceShieldPixelNames.REPORT_VPN_ALWAYS_ON_TRIGGERED_DAILY)
888895
firePixel(DeviceShieldPixelNames.REPORT_VPN_ALWAYS_ON_TRIGGERED)

app-tracking-protection/vpn-impl/src/main/java/com/duckduckgo/mobile/android/vpn/service/TrackerBlockingVpnService.kt

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ import kotlinx.coroutines.CoroutineScope
7575
import kotlinx.coroutines.MainScope
7676
import kotlinx.coroutines.asCoroutineDispatcher
7777
import kotlinx.coroutines.async
78+
import kotlinx.coroutines.delay
7879
import kotlinx.coroutines.launch
7980
import kotlinx.coroutines.runBlocking
8081
import kotlinx.coroutines.withContext
@@ -83,6 +84,8 @@ import logcat.LogPriority.WARN
8384
import logcat.asLog
8485
import logcat.logcat
8586

87+
private const val DDG_VPN_SESSION = "DuckDuckGo"
88+
8689
@InjectWith(
8790
scope = VpnScope::class,
8891
delayGeneration = true,
@@ -237,6 +240,10 @@ class TrackerBlockingVpnService : VpnService(), CoroutineScope by MainScope(), V
237240
if (notifyVpnStart()) {
238241
synchronized(this) {
239242
launch(serviceDispatcher) {
243+
// Give Android a moment to complete foreground transition
244+
// You might think this is a hack (and it kind of is)
245+
// but it’s a workaround to avoid early establish() binding failures
246+
delay(100)
240247
async {
241248
startVpn(intent.alwaysOnTriggered())
242249
}.await()
@@ -422,16 +429,33 @@ class TrackerBlockingVpnService : VpnService(), CoroutineScope by MainScope(), V
422429
return runCatching {
423430
Builder().run {
424431
allowFamily(AF_INET6)
425-
addAddress(InetAddress.getByName("10.0.100.100"), 32)
426-
addAddress(InetAddress.getByName("fd00:1:fd00:1:fd00:1:fd00:1"), 128)
432+
try {
433+
addAddress(InetAddress.getByName("10.0.100.100"), 32)
434+
} catch (e: Exception) {
435+
throw IllegalStateException("Failed to add IPv4 address 10.0.100.100/32", e)
436+
}
437+
try {
438+
addAddress(InetAddress.getByName("fd00::1"), 128)
439+
} catch (e: Exception) {
440+
throw IllegalStateException("Failed to add IPv6 address fd00::1/128", e)
441+
}
427442
// nobody will be listening here we just want to make sure no app has connection
428-
addDnsServer("10.0.100.1")
443+
try {
444+
addDnsServer("10.0.100.1")
445+
} catch (e: Exception) {
446+
throw IllegalStateException("Failed to add DNS server 10.0.100.1", e)
447+
}
429448
// just so that we can connect to our BE
430449
// TODO should we protect all comms with our controller BE? other VPNs do that
431450
safelyAddDisallowedApps(listOf(this@TrackerBlockingVpnService.packageName))
432451
setBlocking(true)
452+
setSession(DDG_VPN_SESSION)
433453
setMtu(1280)
434-
prepare(this@TrackerBlockingVpnService)
454+
try {
455+
prepare(this@TrackerBlockingVpnService)
456+
} catch (e: Exception) {
457+
throw IllegalStateException("VPN service not prepared", e)
458+
}
435459
establish()
436460
}.also {
437461
logcat { "VPN log: Hole TUN created ${it?.fd}" }
@@ -506,6 +530,9 @@ class TrackerBlockingVpnService : VpnService(), CoroutineScope by MainScope(), V
506530
}
507531

508532
setBlocking(true)
533+
// optional in docs but apparently some OEMs may expect to have a session
534+
setSession(DDG_VPN_SESSION)
535+
509536
// Cap the max MTU value to avoid backpressure issues in the socket
510537
// This is effectively capping the max segment size too
511538
setMtu(tunnelConfig.mtu)

app-tracking-protection/vpn-impl/src/test/java/com/duckduckgo/mobile/android/vpn/feature/settings/ExceptionListsSettingStoreTest.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,12 @@ class ExceptionListsSettingStoreTest {
9696

9797
@Test
9898
fun whenEmptyJsonStoreNothing() {
99+
exceptionListsSettingStore.store("{}")
100+
verifyNoInteractions(mockVpnDatabase)
101+
}
102+
103+
@Test
104+
fun whenInvalidJsonStoreNothingAndDoNotCrash() {
99105
exceptionListsSettingStore.store("")
100106
verifyNoInteractions(mockVpnDatabase)
101107
}

0 commit comments

Comments
 (0)