Skip to content

Commit b8dc955

Browse files
0xnm0xnm
authored
Merge pull request #1818 from DataDog/nogorodnikov/rum-2459/safe-serialization-of-user-provided-properties
RUM-2459: Safe serialization of user-provided attributes
2 parents cf5bce1 + ef60e47 commit b8dc955

File tree

13 files changed

+586
-21
lines changed

13 files changed

+586
-21
lines changed

‎dd-sdk-android-core/api/apiSurface

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ fun java.util.concurrent.ExecutorService.submitSafe(String, com.datadog.android.
231231
val NULL_MAP_VALUE: Object
232232
object com.datadog.android.core.internal.utils.JsonSerializer
233233
fun toJsonElement(Any?): com.google.gson.JsonElement
234+
fun Map<String, Any?>.safeMapValuesToJson(com.datadog.android.api.InternalLogger): Map<String, com.google.gson.JsonElement>
234235
fun Int.toHexString(): String
235236
fun Long.toHexString(): String
236237
fun java.math.BigInteger.toHexString(): String

‎dd-sdk-android-core/api/dd-sdk-android-core.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,7 @@ public final class com/datadog/android/core/internal/utils/ConcurrencyExtKt {
590590

591591
public final class com/datadog/android/core/internal/utils/JsonSerializer {
592592
public static final field INSTANCE Lcom/datadog/android/core/internal/utils/JsonSerializer;
593+
public final fun safeMapValuesToJson (Ljava/util/Map;Lcom/datadog/android/api/InternalLogger;)Ljava/util/Map;
593594
public final fun toJsonElement (Ljava/lang/Object;)Lcom/google/gson/JsonElement;
594595
}
595596

‎dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/utils/MiscUtils.kt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import com.google.gson.JsonPrimitive
1616
import org.json.JSONArray
1717
import org.json.JSONObject
1818
import java.util.Date
19+
import java.util.Locale
1920

2021
internal fun retryWithDelay(
2122
times: Int,
@@ -65,6 +66,8 @@ object JsonSerializer {
6566
// polluting user-space, we are going to encapsulate it. Maybe later if we have an internal
6667
// package it can be converted back to the extension function.
6768

69+
internal const val ITEM_SERIALIZATION_ERROR = "Error serializing value for key %s, value was dropped."
70+
6871
/**
6972
* Converts arbitrary object to the [JsonElement] with the best effort. [Any.toString] is
7073
* used as a fallback.
@@ -93,6 +96,28 @@ object JsonSerializer {
9396
else -> JsonPrimitive(item.toString())
9497
}
9598
}
99+
100+
/**
101+
* This method will convert all values to JSON in a safe way, meaning if serialization fails
102+
* for the particular value, the process will continue and faulty value will be dropped.
103+
*/
104+
@InternalApi
105+
fun Map<String, Any?>.safeMapValuesToJson(internalLogger: InternalLogger): Map<String, JsonElement> {
106+
val result = mutableMapOf<String, JsonElement>()
107+
forEach {
108+
try {
109+
result += it.key to toJsonElement(it.value)
110+
} catch (@Suppress("TooGenericExceptionCaught") e: Exception) {
111+
internalLogger.log(
112+
InternalLogger.Level.ERROR,
113+
targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY),
114+
messageBuilder = { ITEM_SERIALIZATION_ERROR.format(Locale.US, it.key) },
115+
throwable = e
116+
)
117+
}
118+
}
119+
return result
120+
}
96121
}
97122

98123
internal fun Any?.fromJsonElement(): Any? {

‎dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/utils/MiscUtilsTest.kt

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@
77
package com.datadog.android.core.internal.utils
88

99
import com.datadog.android.api.InternalLogger
10+
import com.datadog.android.core.internal.utils.JsonSerializer.ITEM_SERIALIZATION_ERROR
11+
import com.datadog.android.core.internal.utils.JsonSerializer.safeMapValuesToJson
1012
import com.datadog.android.utils.forge.Configurator
13+
import com.datadog.android.utils.verifyLog
14+
import com.datadog.tools.unit.forge.anException
1115
import com.datadog.tools.unit.forge.exhaustiveAttributes
1216
import com.google.gson.JsonArray
1317
import com.google.gson.JsonElement
@@ -33,6 +37,7 @@ import org.mockito.kotlin.verifyNoInteractions
3337
import org.mockito.kotlin.whenever
3438
import org.mockito.quality.Strictness
3539
import java.util.Date
40+
import java.util.Locale
3641
import java.util.concurrent.TimeUnit
3742
import kotlin.system.measureNanoTime
3843

@@ -127,6 +132,37 @@ internal class MiscUtilsTest {
127132
}
128133
}
129134

135+
@Test
136+
fun `M map values to JSON without throwing W safeMapValuesToJson()`(forge: Forge) {
137+
// GIVEN
138+
val attributes = forge.exhaustiveAttributes().toMutableMap()
139+
val fakeException = forge.anException()
140+
val faultyKey = forge.anAlphabeticalString()
141+
val faultyItem = object {
142+
override fun toString(): String {
143+
throw fakeException
144+
}
145+
}
146+
val mockInternalLogger = mock<InternalLogger>()
147+
148+
// WHEN
149+
val mapped = attributes.apply { this += faultyKey to faultyItem }
150+
.safeMapValuesToJson(mockInternalLogger)
151+
152+
// THEN
153+
assertThat(mapped).hasSize(attributes.size - 1)
154+
assertThat(mapped.values).doesNotContainNull()
155+
assertThat(mapped).doesNotContainKey(faultyKey)
156+
157+
mockInternalLogger
158+
.verifyLog(
159+
level = InternalLogger.Level.ERROR,
160+
targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY),
161+
message = ITEM_SERIALIZATION_ERROR.format(Locale.US, faultyKey),
162+
throwable = fakeException
163+
)
164+
}
165+
130166
// endregion
131167

132168
// region Internal

‎detekt_custom.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -850,6 +850,7 @@ datadog:
850850
- "kotlin.collections.Map.mapNotNull(kotlin.Function1)"
851851
- "kotlin.collections.Map.mapValues(kotlin.Function1)"
852852
- "kotlin.collections.Map.orEmpty()"
853+
- "kotlin.collections.Map.safeMapValuesToJson(com.datadog.android.api.InternalLogger)"
853854
- "kotlin.collections.Map.toJsonObject()"
854855
- "kotlin.collections.Map.toMap()"
855856
- "kotlin.collections.Map.toMap()"
@@ -923,6 +924,7 @@ datadog:
923924
- "kotlin.collections.MutableMap.remove(kotlin.Int)"
924925
- "kotlin.collections.MutableMap.remove(kotlin.Long)"
925926
- "kotlin.collections.MutableMap.remove(kotlin.String)"
927+
- "kotlin.collections.MutableMap.safeMapValuesToJson(com.datadog.android.api.InternalLogger)"
926928
- "kotlin.collections.MutableMap?.forEach(kotlin.Function1)"
927929
- "kotlin.collections.MutableSet.add(com.datadog.android.core.internal.persistence.ConsentAwareStorage.Batch)"
928930
- "kotlin.collections.MutableSet.add(com.datadog.android.telemetry.internal.TelemetryEventId)"

‎features/dd-sdk-android-logs/src/main/kotlin/com/datadog/android/log/internal/domain/event/LogEventSerializer.kt

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@ package com.datadog.android.log.internal.domain.event
99
import com.datadog.android.api.InternalLogger
1010
import com.datadog.android.core.constraints.DataConstraints
1111
import com.datadog.android.core.constraints.DatadogDataConstraints
12+
import com.datadog.android.core.internal.utils.JsonSerializer.safeMapValuesToJson
1213
import com.datadog.android.core.persistence.Serializer
1314
import com.datadog.android.log.LogAttributes
1415
import com.datadog.android.log.model.LogEvent
1516

1617
internal class LogEventSerializer(
17-
internalLogger: InternalLogger,
18+
private val internalLogger: InternalLogger,
1819
private val dataConstraints: DataConstraints = DatadogDataConstraints(internalLogger)
1920
) : Serializer<LogEvent> {
2021

@@ -35,11 +36,17 @@ internal class LogEventSerializer(
3536
keyPrefix = LogAttributes.USR_ATTRIBUTES_GROUP,
3637
attributesGroupName = USER_EXTRA_GROUP_VERBOSE_NAME
3738
)
38-
it.copy(additionalProperties = sanitizedUserAttributes)
39+
it.copy(
40+
additionalProperties = sanitizedUserAttributes
41+
.safeMapValuesToJson(internalLogger)
42+
.toMutableMap()
43+
)
3944
}
4045
return log.copy(
4146
ddtags = sanitizedTags,
42-
additionalProperties = sanitizedAttributes.toMutableMap(),
47+
additionalProperties = sanitizedAttributes
48+
.safeMapValuesToJson(internalLogger)
49+
.toMutableMap(),
4350
usr = usr
4451
)
4552
}

‎features/dd-sdk-android-logs/src/test/kotlin/com/datadog/android/log/internal/domain/event/LogEventSerializerTest.kt

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import com.datadog.android.log.model.LogEvent
1212
import com.datadog.android.utils.forge.Configurator
1313
import com.datadog.tools.unit.assertj.JsonObjectAssert
1414
import com.datadog.tools.unit.assertj.JsonObjectAssert.Companion.assertThat
15+
import com.datadog.tools.unit.forge.anException
1516
import com.google.gson.JsonParser
1617
import fr.xgouchet.elmyr.Forge
1718
import fr.xgouchet.elmyr.annotation.Forgery
@@ -35,7 +36,7 @@ import org.mockito.quality.Strictness
3536
@ForgeConfiguration(Configurator::class)
3637
internal class LogEventSerializerTest {
3738

38-
lateinit var testedSerializer: LogEventSerializer
39+
private lateinit var testedSerializer: LogEventSerializer
3940

4041
@Mock
4142
lateinit var mockInternalLogger: InternalLogger
@@ -118,6 +119,60 @@ internal class LogEventSerializerTest {
118119
}
119120
}
120121

122+
@Test
123+
fun `M not throw W serialize() { usr#additionalProperties serialization throws }`(
124+
@Forgery fakeLog: LogEvent,
125+
forge: Forge
126+
) {
127+
// Given
128+
val faultyKey = forge.anAlphabeticalString()
129+
val faultyObject = object {
130+
override fun toString(): String {
131+
throw forge.anException()
132+
}
133+
}
134+
val faultyLogEvent = fakeLog.copy(
135+
usr = fakeLog.usr?.copy(
136+
additionalProperties = fakeLog.usr?.additionalProperties
137+
?.toMutableMap()
138+
?.apply { put(faultyKey, faultyObject) }
139+
.orEmpty()
140+
.toMutableMap()
141+
)
142+
)
143+
144+
// When
145+
val serialized = testedSerializer.serialize(faultyLogEvent)
146+
147+
// Then
148+
assertSerializedLogMatchesInputLog(serialized, fakeLog)
149+
}
150+
151+
@Test
152+
fun `M not throw W serialize() { additionalProperties serialization throws }`(
153+
@Forgery fakeLog: LogEvent,
154+
forge: Forge
155+
) {
156+
// Given
157+
val faultyKey = forge.anAlphabeticalString()
158+
val faultyObject = object {
159+
override fun toString(): String {
160+
throw forge.anException()
161+
}
162+
}
163+
val faultyLogEvent = fakeLog.copy(
164+
additionalProperties = fakeLog.additionalProperties
165+
.toMutableMap()
166+
.apply { put(faultyKey, faultyObject) }
167+
)
168+
169+
// When
170+
val serialized = testedSerializer.serialize(faultyLogEvent)
171+
172+
// Then
173+
assertSerializedLogMatchesInputLog(serialized, fakeLog)
174+
}
175+
121176
// region Internal
122177

123178
private fun assertSerializedLogMatchesInputLog(

‎features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/event/RumEventSerializer.kt

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ package com.datadog.android.rum.internal.domain.event
99
import com.datadog.android.api.InternalLogger
1010
import com.datadog.android.core.constraints.DataConstraints
1111
import com.datadog.android.core.constraints.DatadogDataConstraints
12+
import com.datadog.android.core.internal.utils.JsonSerializer.safeMapValuesToJson
1213
import com.datadog.android.core.persistence.Serializer
1314
import com.datadog.android.rum.RumAttributes
1415
import com.datadog.android.rum.model.ActionEvent
@@ -22,7 +23,7 @@ import com.datadog.android.telemetry.model.TelemetryErrorEvent
2223
import com.google.gson.JsonObject
2324

2425
internal class RumEventSerializer(
25-
internalLogger: InternalLogger,
26+
private val internalLogger: InternalLogger,
2627
private val dataConstraints: DataConstraints = DatadogDataConstraints(internalLogger)
2728
) : Serializer<Any> {
2829

@@ -70,9 +71,13 @@ internal class RumEventSerializer(
7071
private fun serializeViewEvent(model: ViewEvent): String {
7172
val sanitizedUser = model.usr?.copy(
7273
additionalProperties = validateUserAttributes(model.usr.additionalProperties)
74+
.safeMapValuesToJson(internalLogger)
75+
.toMutableMap()
7376
)
7477
val sanitizedContext = model.context?.copy(
7578
additionalProperties = validateContextAttributes(model.context.additionalProperties)
79+
.safeMapValuesToJson(internalLogger)
80+
.toMutableMap()
7681
)
7782
val sanitizedView = model.view.copy(
7883
customTimings = model.view.customTimings?.copy(
@@ -93,9 +98,13 @@ internal class RumEventSerializer(
9398
private fun serializeErrorEvent(model: ErrorEvent): String {
9499
val sanitizedUser = model.usr?.copy(
95100
additionalProperties = validateUserAttributes(model.usr.additionalProperties)
101+
.safeMapValuesToJson(internalLogger)
102+
.toMutableMap()
96103
)
97104
val sanitizedContext = model.context?.copy(
98105
additionalProperties = validateContextAttributes(model.context.additionalProperties)
106+
.safeMapValuesToJson(internalLogger)
107+
.toMutableMap()
99108
)
100109
val sanitizedModel = model.copy(
101110
usr = sanitizedUser,
@@ -107,9 +116,13 @@ internal class RumEventSerializer(
107116
private fun serializeResourceEvent(model: ResourceEvent): String {
108117
val sanitizedUser = model.usr?.copy(
109118
additionalProperties = validateUserAttributes(model.usr.additionalProperties)
119+
.safeMapValuesToJson(internalLogger)
120+
.toMutableMap()
110121
)
111122
val sanitizedContext = model.context?.copy(
112123
additionalProperties = validateContextAttributes(model.context.additionalProperties)
124+
.safeMapValuesToJson(internalLogger)
125+
.toMutableMap()
113126
)
114127
val sanitizedModel = model.copy(
115128
usr = sanitizedUser,
@@ -121,9 +134,13 @@ internal class RumEventSerializer(
121134
private fun serializeActionEvent(model: ActionEvent): String {
122135
val sanitizedUser = model.usr?.copy(
123136
additionalProperties = validateUserAttributes(model.usr.additionalProperties)
137+
.safeMapValuesToJson(internalLogger)
138+
.toMutableMap()
124139
)
125140
val sanitizedContext = model.context?.copy(
126141
additionalProperties = validateContextAttributes(model.context.additionalProperties)
142+
.safeMapValuesToJson(internalLogger)
143+
.toMutableMap()
127144
)
128145
val sanitizedModel = model.copy(
129146
usr = sanitizedUser,
@@ -135,9 +152,13 @@ internal class RumEventSerializer(
135152
private fun serializeLongTaskEvent(model: LongTaskEvent): String {
136153
val sanitizedUser = model.usr?.copy(
137154
additionalProperties = validateUserAttributes(model.usr.additionalProperties)
155+
.safeMapValuesToJson(internalLogger)
156+
.toMutableMap()
138157
)
139158
val sanitizedContext = model.context?.copy(
140159
additionalProperties = validateContextAttributes(model.context.additionalProperties)
160+
.safeMapValuesToJson(internalLogger)
161+
.toMutableMap()
141162
)
142163
val sanitizedModel = model.copy(
143164
usr = sanitizedUser,

0 commit comments

Comments
 (0)