Skip to content

Commit 60d89ff

Browse files
committed
perf(nav3): Bound navigation argument payload size
Limit the total argument characters sanitized from one back-stack update so large strings and stringified values cannot dominate navigation telemetry payloads or processing time. Preserve the existing entry, depth, and value budgets and verify the new limit with a regression test.\n\nCo-Authored-By: Codex <noreply@openai.com>
1 parent 1a324af commit 60d89ff

2 files changed

Lines changed: 37 additions & 5 deletions

File tree

‎sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackConverter.kt‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ internal class BackStackConverter<T : Any>(
124124

125125
private val activeContainers = IdentityHashMap<Any, Unit>()
126126
private var remainingValues = MAX_ARGUMENT_COUNT
127+
private var remainingCharacters = MAX_ARGUMENT_CHARACTERS
127128
private var budgetExhausted = false
128129

129130
/**
@@ -158,7 +159,9 @@ internal class BackStackConverter<T : Any>(
158159
try {
159160
val sanitized = mutableMapOf<String, Any?>()
160161
for ((key, childValue) in value) {
161-
sanitized[key.toString()] = sanitizeValue(childValue, depth + 1)
162+
val keyString = key.toString()
163+
consumeCharacters(keyString.length)
164+
sanitized[keyString] = sanitizeValue(childValue, depth + 1)
162165
}
163166
return sanitized
164167
} finally {
@@ -185,14 +188,16 @@ internal class BackStackConverter<T : Any>(
185188
val collection = value?.asSanitizableCollectionOrNull()
186189

187190
return when {
188-
value == null || value is String || value is Number || value is Boolean -> value
189-
value is CharSequence || value is Char -> value.toString()
190-
value is Enum<*> -> value.name
191+
value == null || value is Number || value is Boolean -> value
192+
value is String -> value.also { consumeCharacters(it.length) }
193+
value is CharSequence || value is Char ->
194+
value.toString().also { consumeCharacters(it.length) }
195+
value is Enum<*> -> value.name.also { consumeCharacters(it.length) }
191196
value is Map<*, *> -> sanitizeMap(value, depth)
192197
collection != null -> sanitizeCollection(collection, depth)
193198
else -> {
194199
warningState.logUnsupportedValueWarning(value::class.simpleName, logger)
195-
value.toString()
200+
value.toString().also { consumeCharacters(it.length) }
196201
}
197202
}
198203
}
@@ -225,6 +230,13 @@ internal class BackStackConverter<T : Any>(
225230
}
226231
}
227232

233+
private fun consumeCharacters(count: Int) {
234+
if (count > remainingCharacters) {
235+
throw DropSubtree(CHARACTER_BUDGET_WARNING, exhaustsBudget = true)
236+
}
237+
remainingCharacters -= count
238+
}
239+
228240
private fun enter(container: Any) {
229241
if (activeContainers.put(container, Unit) != null) {
230242
throw DropSubtree(STRUCTURE_WARNING, exhaustsBudget = false)
@@ -281,10 +293,22 @@ internal class BackStackConverter<T : Any>(
281293
*/
282294
private const val MAX_ARGUMENT_COUNT = 200
283295

296+
// TODO ADAM: Add visual indicator when we truncate args.
297+
/**
298+
* Max number of characters visited while sanitizing all entries in a given back stack update.
299+
*
300+
* Caps payload size in the presence of large individual arguments.
301+
*/
302+
private const val MAX_ARGUMENT_CHARACTERS = 4_096
303+
284304
private const val BUDGET_WARNING =
285305
"Nav3 arguments exceeded the maximum total value count for one backstack update. Skipping arguments " +
286306
"for this and older captured entries."
287307

308+
private const val CHARACTER_BUDGET_WARNING =
309+
"Nav3 arguments exceeded the maximum total character count for one backstack update. Skipping " +
310+
"arguments for this and older captured entries."
311+
288312
private const val STRUCTURE_WARNING =
289313
"Nav3 argument sanitization failed (possibly a cyclic or deeply nested structure). Skipping arguments."
290314
}

‎sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,14 @@ class BackStackConverterTest {
503503
assertThat(sut.convert(HomeScreen()).arguments).isEmpty()
504504
}
505505

506+
@Test
507+
fun `convert drops arguments when character budget is exceeded`() {
508+
val sut =
509+
getSut(entryMapper = { entry -> entryInfo(entry, mapOf("value" to "x".repeat(4_097))) })
510+
511+
assertThat(sut.convert(HomeScreen()).arguments).isEmpty()
512+
}
513+
506514
@Test
507515
fun `arguments do not use caller collection size for allocation`() {
508516
val values =

0 commit comments

Comments
 (0)