Skip to content

Commit 55321ec

Browse files
adinauerclaude
andcommitted
fix(sentry): Unwind MapObjectReader failures fully
Recover MapObjectReader values back to their stack checkpoint so partially consumed nested objects do not leave child entries or end sentinels behind. This keeps later values readable after a deserializer fails mid-object and adds regression coverage for list, map, and map-of-lists paths that partially enter nested values before throwing. Refs GH-5278 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent f823221 commit 55321ec

2 files changed

Lines changed: 75 additions & 8 deletions

File tree

sentry/src/main/java/io/sentry/util/MapObjectReader.java

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,12 @@ public MapObjectReader(final Map<String, Object> root) {
3131
@Override
3232
public void nextUnknown(
3333
final @NotNull ILogger logger, final Map<String, Object> unknown, final String name) {
34+
final int stackSizeBefore = stack.size();
3435
try {
3536
unknown.put(name, nextObjectOrNull());
3637
} catch (Exception exception) {
3738
logger.log(SentryLevel.ERROR, exception, "Error deserializing unknown key: %s", name);
38-
try {
39-
skipValue();
40-
} catch (Exception ignored) {
41-
// stream is unrecoverable
42-
}
39+
recoverValue(stackSizeBefore);
4340
}
4441
}
4542

@@ -57,11 +54,12 @@ public <T> List<T> nextListOrNull(
5754
List<T> list = new ArrayList<>();
5855
if (hasNext()) {
5956
do {
57+
final int stackSizeBefore = stack.size();
6058
try {
6159
list.add(deserializer.deserialize(this, logger));
6260
} catch (Exception e) {
6361
logger.log(SentryLevel.WARNING, "Failed to deserialize object in list.", e);
64-
skipValue();
62+
recoverValue(stackSizeBefore);
6563
}
6664
} while (peek() == JsonToken.BEGIN_OBJECT);
6765
}
@@ -87,11 +85,12 @@ public <T> Map<String, T> nextMapOrNull(
8785
if (hasNext()) {
8886
do {
8987
final String key = nextName();
88+
final int stackSizeBefore = stack.size();
9089
try {
9190
map.put(key, deserializer.deserialize(this, logger));
9291
} catch (Exception e) {
9392
logger.log(SentryLevel.WARNING, "Failed to deserialize object in map.", e);
94-
skipValue();
93+
recoverValue(stackSizeBefore);
9594
}
9695
} while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME);
9796
}
@@ -116,14 +115,15 @@ public <T> Map<String, T> nextMapOrNull(
116115
if (hasNext()) {
117116
do {
118117
final @NotNull String key = nextName();
118+
final int stackSizeBefore = stack.size();
119119
try {
120120
final @Nullable List<T> list = nextListOrNull(logger, deserializer);
121121
if (list != null) {
122122
result.put(key, list);
123123
}
124124
} catch (Exception e) {
125125
logger.log(SentryLevel.WARNING, "Failed to deserialize list in map.", e);
126-
skipValue();
126+
recoverValue(stackSizeBefore);
127127
}
128128
} while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME);
129129
}
@@ -397,6 +397,12 @@ public void skipValue() throws IOException {
397397
}
398398
}
399399

400+
private void recoverValue(final int stackSizeBefore) {
401+
while (!stack.isEmpty() && stack.size() >= stackSizeBefore) {
402+
stack.removeLast();
403+
}
404+
}
405+
400406
@SuppressWarnings("TypeParameterUnusedInFormals")
401407
@Nullable
402408
private <T> T nextValueOrNull() throws IOException {

sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,25 @@ class MapObjectReaderTest {
4949

5050
private fun serializableValue(value: String): Map<String, Any> = linkedMapOf("test" to value)
5151

52+
private fun partialSerializableValue(kind: String, value: String): Map<String, Any> =
53+
linkedMapOf("test" to value, "kind" to kind)
54+
55+
private val partiallyFailingDeserializer =
56+
JsonDeserializer<BasicSerializable> { reader, _ ->
57+
val basicSerializable = BasicSerializable()
58+
reader.beginObject()
59+
if (reader.nextName() == "kind") {
60+
if (reader.nextString() == "fail") {
61+
throw IllegalStateException("intentional")
62+
}
63+
}
64+
if (reader.nextName() == "test") {
65+
basicSerializable.test = reader.nextString()
66+
}
67+
reader.endObject()
68+
basicSerializable
69+
}
70+
5271
@Test
5372
fun `deserializes data correctly`() {
5473
val data = mutableMapOf<String, Any>()
@@ -244,4 +263,46 @@ class MapObjectReaderTest {
244263

245264
assertEquals(mapOf("good" to listOf(BasicSerializable("two"))), actual)
246265
}
266+
267+
@Test(timeout = 1000L)
268+
fun `nextListOrNull keeps elements after a partially consumed failing element`() {
269+
val actual =
270+
getValuesReader(
271+
listOf(partialSerializableValue("fail", "bad"), partialSerializableValue("ok", "two"))
272+
)
273+
.nextListOrNull(logger, partiallyFailingDeserializer)
274+
275+
assertEquals(listOf(BasicSerializable("two")), actual)
276+
}
277+
278+
@Test(timeout = 1000L)
279+
fun `nextMapOrNull keeps values after a partially consumed failing value`() {
280+
val actual =
281+
getValuesReader(
282+
linkedMapOf(
283+
"bad" to partialSerializableValue("fail", "bad"),
284+
"good" to partialSerializableValue("ok", "two"),
285+
)
286+
)
287+
.nextMapOrNull(logger, partiallyFailingDeserializer)
288+
289+
assertEquals(mapOf("good" to BasicSerializable("two")), actual)
290+
}
291+
292+
@Test(timeout = 1000L)
293+
fun `nextMapOfListOrNull keeps values after a partially consumed failing element`() {
294+
val actual =
295+
getValuesReader(
296+
linkedMapOf(
297+
"bad" to listOf(partialSerializableValue("fail", "bad")),
298+
"good" to listOf(partialSerializableValue("ok", "two")),
299+
)
300+
)
301+
.nextMapOfListOrNull(logger, partiallyFailingDeserializer)
302+
303+
assertEquals(
304+
mapOf("bad" to emptyList<BasicSerializable>(), "good" to listOf(BasicSerializable("two"))),
305+
actual,
306+
)
307+
}
247308
}

0 commit comments

Comments
 (0)