Skip to content

Commit ead3653

Browse files
typottertypotter
committed
update safe calls, allow all implementations of maps to apply as default
1 parent afa3316 commit ead3653

File tree

6 files changed

+205
-129
lines changed

6 files changed

+205
-129
lines changed

‎detekt_custom_safe_calls.yml‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,16 +1228,11 @@ datadog:
12281228
- "okio.Buffer.constructor()"
12291229
# endregion
12301230
# region org.json
1231-
- "org.json.JSONArray.get(kotlin.Int)"
12321231
- "org.json.JSONArray.length()"
1233-
- "org.json.JSONArray.put(kotlin.Any?)"
12341232
- "org.json.JSONArray.toJsonArray()"
12351233
- "org.json.JSONObject.constructor()"
1236-
- "org.json.JSONObject.constructor(kotlin.String)"
1237-
- "org.json.JSONObject.get(kotlin.String)"
12381234
- "org.json.JSONObject.keys()"
12391235
- "org.json.JSONObject.optString(kotlin.String?, kotlin.String?)"
1240-
- "org.json.JSONObject.put(kotlin.String, kotlin.Any?)"
12411236
- "org.json.JSONObject.toJsonObject()"
12421237
- "org.json.JSONObject.length()"
12431238
- "org.json.JSONObject.opt(kotlin.String?)"

‎features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/DatadogFlagsClient.kt‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -141,15 +141,8 @@ internal class DatadogFlagsClient(
141141
* @return The map value of the flag, or the default value if unavailable.
142142
*/
143143
override fun resolveStructureValue(flagKey: String, defaultValue: Map<String, Any?>): Map<String, Any?> {
144-
val jsonDefault = defaultValue.toJSONObject()
145-
val jsonResult = resolveValue(flagKey, jsonDefault)
146-
147-
// Preserve original default value on failure (avoids round-trip conversion)
148-
return if (jsonResult === jsonDefault) {
149-
defaultValue
150-
} else {
151-
jsonResult.toMap()
152-
}
144+
val result = resolveValue(flagKey, defaultValue)
145+
return result
153146
}
154147

155148
/**

‎features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/FlagValueConverter.kt‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,14 @@ internal object FlagValueConverter {
5757
Double::class -> variationValue.toDoubleOrNull() as? T
5858
Map::class -> variationValue.toMap() as? T
5959
JSONObject::class -> JSONObject(variationValue) as? T
60-
else -> null
60+
else -> {
61+
// Check if targetType is a Map implementation
62+
if (Map::class.java.isAssignableFrom(targetType.java)) {
63+
variationValue.toMap() as? T
64+
} else {
65+
null
66+
}
67+
}
6168
}
6269

6370
result ?: throw IllegalArgumentException("Failed to parse value '$variationValue'")
@@ -80,7 +87,14 @@ internal object FlagValueConverter {
8087
variationType == VariationType.INTEGER.value
8188
JSONObject::class -> variationType == VariationType.OBJECT.value
8289
Map::class -> variationType == VariationType.OBJECT.value
83-
else -> false
90+
else -> {
91+
// Check if targetType is a Map implementation (e.g., LinkedHashMap, HashMap, etc.)
92+
if (Map::class.java.isAssignableFrom(targetType.java)) {
93+
variationType == VariationType.OBJECT.value
94+
} else {
95+
false
96+
}
97+
}
8498
}
8599

86100
fun getTypeName(targetType: KClass<*>): String = when (targetType) {

‎features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/JsonExtensions.kt‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import org.json.JSONObject
2121
* @throws JSONException if the string is not valid JSON
2222
*/
2323
internal fun String.toMap(): Map<String, Any?> {
24+
// Safe: Input validation happens at call site; JSONException is expected to propagate
25+
@Suppress("UnsafeThirdPartyFunctionCall")
2426
val jsonObject = JSONObject(this)
2527
return jsonObject.toMap()
2628
}
@@ -45,6 +47,8 @@ internal fun JSONObject.toMap(): Map<String, Any?> {
4547
while (keys.hasNext()) {
4648
@Suppress("UnsafeThirdPartyFunctionCall")
4749
val key = keys.next()
50+
// Safe: Key exists because it came from keys() iterator
51+
@Suppress("UnsafeThirdPartyFunctionCall")
4852
result[key] = convertJsonValue(this.get(key))
4953
}
5054

@@ -68,6 +72,7 @@ internal fun JSONArray.toList(): List<Any?> {
6872
// Safe: Iterating within bounds (0 until length)
6973
@Suppress("UnsafeThirdPartyFunctionCall")
7074
for (i in 0 until this.length()) {
75+
// Safe: Index is within bounds (0 until length)
7176
@Suppress("UnsafeThirdPartyFunctionCall")
7277
result.add(convertJsonValue(this.get(i)))
7378
}
@@ -87,7 +92,7 @@ internal fun Map<String, Any?>.toJSONObject(): JSONObject {
8792
val jsonObject = JSONObject()
8893

8994
forEach { (key, value) ->
90-
// Safe: put() on JSONObject doesn't throw for valid key/value pairs
95+
// Safe: convertToJsonValue ensures valid types (primitives, JSONObject.NULL, JSONObject, JSONArray)
9196
@Suppress("UnsafeThirdPartyFunctionCall")
9297
jsonObject.put(key, convertToJsonValue(value))
9398
}
@@ -107,7 +112,7 @@ internal fun List<Any?>.toJSONArray(): JSONArray {
107112
val jsonArray = JSONArray()
108113

109114
forEach { value ->
110-
// Safe: put() on JSONArray doesn't throw for valid values
115+
// Safe: convertToJsonValue ensures valid types (primitives, JSONObject.NULL, JSONObject, JSONArray)
111116
@Suppress("UnsafeThirdPartyFunctionCall")
112117
jsonArray.put(convertToJsonValue(value))
113118
}

‎features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/DatadogFlagsClientTest.kt‎

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,14 @@ internal class DatadogFlagsClientTest {
8383
@StringForgery
8484
lateinit var fakeJsonKey: String
8585

86+
private lateinit var fakeDefaultMapValue: Map<String, Any?>
87+
8688
@BeforeEach
8789
fun `set up`(forge: Forge) {
90+
fakeDefaultMapValue = mapOf(
91+
"name" to forge.anAlphabeticalString(),
92+
"age" to forge.anInt()
93+
)
8894
whenever(mockFeatureSdkCore.internalLogger) doReturn mockInternalLogger
8995
whenever(mockFeatureSdkCore.getFeature(RUM_FEATURE_NAME)) doReturn mock()
9096

@@ -497,9 +503,11 @@ internal class DatadogFlagsClientTest {
497503
val fakeDefaultValue = JSONObject().apply {
498504
put(fakeJsonKey, forge.anAlphabeticalString())
499505
}
506+
val fakeKey1Value = forge.anAlphabeticalString()
507+
val fakeKey2Value = forge.anInt()
500508
val fakeFlagValue = JSONObject().apply {
501-
put("key1", forge.anAlphabeticalString())
502-
put("key2", forge.anInt())
509+
put("key1", fakeKey1Value)
510+
put("key2", fakeKey2Value)
503511
}
504512
val fakeFlag = forge.getForgery<PrecomputedFlag>().copy(
505513
variationType = VariationType.OBJECT.value,
@@ -633,14 +641,23 @@ internal class DatadogFlagsClientTest {
633641
fun `M return flag value as map W resolveStructureValue() {map default, flag exists}`(forge: Forge) {
634642
// Given
635643
val fakeFlagKey = forge.anAlphabeticalString()
636-
val fakeDefaultValue = mapOf("default" to "value")
644+
val fakeName = forge.anAlphabeticalString()
645+
val fakeAge = forge.anInt()
646+
val fakeCity = forge.anAlphabeticalString()
647+
val expectedMap = mapOf(
648+
"name" to fakeName,
649+
"age" to fakeAge,
650+
"nested" to mapOf(
651+
"city" to fakeCity
652+
)
653+
)
637654
val fakeFlagValue = JSONObject().apply {
638-
put("name", "Alice")
639-
put("age", 30)
655+
put("name", fakeName)
656+
put("age", fakeAge)
640657
put(
641658
"nested",
642659
JSONObject().apply {
643-
put("city", "NYC")
660+
put("city", fakeCity)
644661
}
645662
)
646663
}
@@ -656,15 +673,10 @@ internal class DatadogFlagsClientTest {
656673
whenever(mockFlagsRepository.getPrecomputedFlagWithContext(fakeFlagKey)) doReturn (fakeFlag to fakeContext)
657674

658675
// When
659-
val result = testedClient.resolveStructureValue(fakeFlagKey, fakeDefaultValue)
676+
val result = testedClient.resolveStructureValue(fakeFlagKey, fakeDefaultMapValue)
660677

661678
// Then - returns Map with no JSON types
662-
assertThat(result).isInstanceOf(Map::class.java)
663-
assertThat(result["name"]).isEqualTo("Alice")
664-
assertThat(result["age"]).isEqualTo(30)
665-
assertThat(result["nested"]).isInstanceOf(Map::class.java)
666-
val nested = result["nested"] as Map<*, *>
667-
assertThat(nested["city"]).isEqualTo("NYC")
679+
assertThat(result).isEqualTo(expectedMap)
668680

669681
// Verify exposure tracked
670682
verify(mockProcessor).processEvent(
@@ -678,32 +690,27 @@ internal class DatadogFlagsClientTest {
678690
fun `M return default map W resolveStructureValue() {map default, flag does not exist}`(forge: Forge) {
679691
// Given
680692
val fakeFlagKey = forge.anAlphabeticalString()
681-
val fakeDefaultValue = mapOf(
682-
"key" to "value",
683-
"number" to 42
684-
)
685693
whenever(mockFlagsRepository.getPrecomputedFlagWithContext(fakeFlagKey)) doReturn null
686694

687695
// When
688-
val result = testedClient.resolveStructureValue(fakeFlagKey, fakeDefaultValue)
696+
val result = testedClient.resolveStructureValue(fakeFlagKey, fakeDefaultMapValue)
689697

690698
// Then
691-
assertThat(result).isEqualTo(fakeDefaultValue)
699+
assertThat(result).isSameAs(fakeDefaultMapValue)
692700
verifyNoInteractions(mockProcessor)
693701
}
694702

695703
@Test
696704
fun `M return default map W resolveStructureValue() {map default, provider not ready}`(forge: Forge) {
697705
// Given
698706
val fakeFlagKey = forge.anAlphabeticalString()
699-
val fakeDefaultValue = mapOf("default" to true)
700707
whenever(mockFlagsRepository.getEvaluationContext()) doReturn null
701708

702709
// When
703-
val result = testedClient.resolveStructureValue(fakeFlagKey, fakeDefaultValue)
710+
val result = testedClient.resolveStructureValue(fakeFlagKey, fakeDefaultMapValue)
704711

705712
// Then
706-
assertThat(result).isEqualTo(fakeDefaultValue)
713+
assertThat(result).isSameAs(fakeDefaultMapValue)
707714
verifyNoInteractions(mockProcessor)
708715
verifyNoInteractions(mockRumEvaluationLogger)
709716
}
@@ -728,10 +735,9 @@ internal class DatadogFlagsClientTest {
728735
fun `M return default map W resolveStructureValue() {map default, type mismatch}`(forge: Forge) {
729736
// Given
730737
val fakeFlagKey = forge.anAlphabeticalString()
731-
val fakeDefaultValue = mapOf("key" to "value")
732738
val fakeFlag = forge.getForgery<PrecomputedFlag>().copy(
733739
variationType = VariationType.STRING.value,
734-
variationValue = "not-an-object"
740+
variationValue = forge.anAlphabeticalString()
735741
)
736742
val fakeContext = EvaluationContext(
737743
targetingKey = forge.anAlphabeticalString(),
@@ -740,10 +746,10 @@ internal class DatadogFlagsClientTest {
740746
whenever(mockFlagsRepository.getPrecomputedFlagWithContext(fakeFlagKey)) doReturn (fakeFlag to fakeContext)
741747

742748
// When
743-
val result = testedClient.resolveStructureValue(fakeFlagKey, fakeDefaultValue)
749+
val result = testedClient.resolveStructureValue(fakeFlagKey, fakeDefaultMapValue)
744750

745751
// Then
746-
assertThat(result).isEqualTo(fakeDefaultValue)
752+
assertThat(result).isSameAs(fakeDefaultMapValue)
747753
verifyNoInteractions(mockProcessor) // No exposure tracked for type mismatch
748754
}
749755

@@ -1284,7 +1290,9 @@ internal class DatadogFlagsClientTest {
12841290
) {
12851291
// Given
12861292
val fakeFlagKey = forge.anAlphabeticalString()
1287-
val fakeDefaultValue = mapOf("default" to "value")
1293+
val fakeDefaultValue = JSONObject().apply {
1294+
put("default", "value")
1295+
}
12881296
val fakeFlag = forge.getForgery<PrecomputedFlag>().copy(
12891297
variationType = VariationType.STRING.value,
12901298
variationValue = "just a string"

0 commit comments

Comments
 (0)