From 5ab0155d89139dea2e01d60b440b6a9fe895fefd Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 14 Apr 2026 13:44:22 +0200 Subject: [PATCH 01/17] fix(sentry): Prevent object readers from hanging on bad values Recover from deserializer failures by advancing past the broken value instead of retrying the same token forever. This keeps list and map helpers progressing and preserves later valid entries. Track nesting in JsonObjectReader so recovery also works after a partially consumed object or array. Implement skipValue in MapObjectReader and avoid consuming stack entries before type checks so failed reads do not corrupt the remaining input. Fixes GH-5278 Co-Authored-By: Claude --- .../main/java/io/sentry/JsonObjectReader.java | 61 +++++++++-- .../java/io/sentry/util/MapObjectReader.java | 32 ++++-- .../java/io/sentry/JsonObjectReaderTest.kt | 99 +++++++++++++++++ .../io/sentry/util/MapObjectReaderTest.kt | 103 +++++++++++++++++- 4 files changed, 278 insertions(+), 17 deletions(-) diff --git a/sentry/src/main/java/io/sentry/JsonObjectReader.java b/sentry/src/main/java/io/sentry/JsonObjectReader.java index f9fe1841847..4a8134ede1a 100644 --- a/sentry/src/main/java/io/sentry/JsonObjectReader.java +++ b/sentry/src/main/java/io/sentry/JsonObjectReader.java @@ -18,6 +18,7 @@ public final class JsonObjectReader implements ObjectReader { private final @NotNull JsonReader jsonReader; + private int depth = 0; public JsonObjectReader(Reader in) { this.jsonReader = new JsonReader(in); @@ -84,10 +85,18 @@ public float nextFloat() throws IOException { @Override public void nextUnknown(ILogger logger, Map unknown, String name) { + final int startDepth = depth; + JsonToken startToken = JsonToken.END_DOCUMENT; try { + startToken = peek(); unknown.put(name, nextObjectOrNull()); } catch (Exception exception) { logger.log(SentryLevel.ERROR, exception, "Error deserializing unknown key: %s", name); + try { + recoverValue(startDepth, startToken); + } catch (Exception ignored) { + // stream is unrecoverable + } } } @@ -98,18 +107,21 @@ public void nextUnknown(ILogger logger, Map unknown, String name jsonReader.nextNull(); return null; } - jsonReader.beginArray(); + beginArray(); List list = new ArrayList<>(); if (jsonReader.hasNext()) { do { + final int startDepth = depth; + final JsonToken startToken = peek(); try { list.add(deserializer.deserialize(this, logger)); } catch (Exception e) { logger.log(SentryLevel.WARNING, "Failed to deserialize object in list.", e); + recoverValue(startDepth, startToken); } } while (jsonReader.peek() == JsonToken.BEGIN_OBJECT); } - jsonReader.endArray(); + endArray(); return list; } @@ -120,20 +132,23 @@ public void nextUnknown(ILogger logger, Map unknown, String name jsonReader.nextNull(); return null; } - jsonReader.beginObject(); + beginObject(); Map map = new HashMap<>(); if (jsonReader.hasNext()) { do { + final String key = jsonReader.nextName(); + final int startDepth = depth; + final JsonToken startToken = peek(); try { - String key = jsonReader.nextName(); map.put(key, deserializer.deserialize(this, logger)); } catch (Exception e) { logger.log(SentryLevel.WARNING, "Failed to deserialize object in map.", e); + recoverValue(startDepth, startToken); } } while (jsonReader.peek() == JsonToken.BEGIN_OBJECT || jsonReader.peek() == JsonToken.NAME); } - jsonReader.endObject(); + endObject(); return map; } @@ -151,9 +166,16 @@ public void nextUnknown(ILogger logger, Map unknown, String name if (hasNext()) { do { final @NotNull String key = nextName(); - final @Nullable List list = nextListOrNull(logger, deserializer); - if (list != null) { - result.put(key, list); + final int startDepth = depth; + final JsonToken startToken = peek(); + try { + final @Nullable List list = nextListOrNull(logger, deserializer); + if (list != null) { + result.put(key, list); + } + } catch (Exception e) { + logger.log(SentryLevel.WARNING, "Failed to deserialize list in map.", e); + recoverValue(startDepth, startToken); } } while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME); } @@ -219,21 +241,25 @@ public void nextUnknown(ILogger logger, Map unknown, String name @Override public void beginObject() throws IOException { jsonReader.beginObject(); + depth++; } @Override public void endObject() throws IOException { jsonReader.endObject(); + depth--; } @Override public void beginArray() throws IOException { jsonReader.beginArray(); + depth++; } @Override public void endArray() throws IOException { jsonReader.endArray(); + depth--; } @Override @@ -281,6 +307,25 @@ public void skipValue() throws IOException { jsonReader.skipValue(); } + private void recoverValue(final int startDepth, final @NotNull JsonToken startToken) + throws IOException { + final boolean enteredNestedValue = depth > startDepth; + while (depth > startDepth) { + final JsonToken token = peek(); + if (token == JsonToken.END_OBJECT) { + endObject(); + } else if (token == JsonToken.END_ARRAY) { + endArray(); + } else { + skipValue(); + } + } + + if (!enteredNestedValue && peek() == startToken) { + skipValue(); + } + } + @Override public void close() throws IOException { jsonReader.close(); diff --git a/sentry/src/main/java/io/sentry/util/MapObjectReader.java b/sentry/src/main/java/io/sentry/util/MapObjectReader.java index b04fbb96751..166ef9bdab9 100644 --- a/sentry/src/main/java/io/sentry/util/MapObjectReader.java +++ b/sentry/src/main/java/io/sentry/util/MapObjectReader.java @@ -35,6 +35,11 @@ public void nextUnknown( unknown.put(name, nextObjectOrNull()); } catch (Exception exception) { logger.log(SentryLevel.ERROR, exception, "Error deserializing unknown key: %s", name); + try { + skipValue(); + } catch (Exception ignored) { + // stream is unrecoverable + } } } @@ -56,6 +61,7 @@ public List nextListOrNull( list.add(deserializer.deserialize(this, logger)); } catch (Exception e) { logger.log(SentryLevel.WARNING, "Failed to deserialize object in list.", e); + skipValue(); } } while (peek() == JsonToken.BEGIN_OBJECT); } @@ -80,11 +86,12 @@ public Map nextMapOrNull( Map map = new HashMap<>(); if (hasNext()) { do { + final String key = nextName(); try { - String key = nextName(); map.put(key, deserializer.deserialize(this, logger)); } catch (Exception e) { logger.log(SentryLevel.WARNING, "Failed to deserialize object in map.", e); + skipValue(); } } while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME); } @@ -109,9 +116,14 @@ public Map nextMapOrNull( if (hasNext()) { do { final @NotNull String key = nextName(); - final @Nullable List list = nextListOrNull(logger, deserializer); - if (list != null) { - result.put(key, list); + try { + final @Nullable List list = nextListOrNull(logger, deserializer); + if (list != null) { + result.put(key, list); + } + } catch (Exception e) { + logger.log(SentryLevel.WARNING, "Failed to deserialize list in map.", e); + skipValue(); } } while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME); } @@ -197,12 +209,13 @@ public String nextName() throws IOException { @Override public void beginObject() throws IOException { - final Map.Entry currentEntry = stack.removeLast(); + final Map.Entry currentEntry = stack.peekLast(); if (currentEntry == null) { throw new IOException("No more entries"); } final Object value = currentEntry.getValue(); if (value instanceof Map) { + stack.removeLast(); // insert a dummy entry to indicate end of an object stack.addLast(new AbstractMap.SimpleEntry<>(null, JsonToken.END_OBJECT)); // extract map entries onto the stack @@ -223,12 +236,13 @@ public void endObject() throws IOException { @Override public void beginArray() throws IOException { - final Map.Entry currentEntry = stack.removeLast(); + final Map.Entry currentEntry = stack.peekLast(); if (currentEntry == null) { throw new IOException("No more entries"); } final Object value = currentEntry.getValue(); if (value instanceof List) { + stack.removeLast(); // insert a dummy entry to indicate end of an object stack.addLast(new AbstractMap.SimpleEntry<>(null, JsonToken.END_ARRAY)); // extract map entries onto the stack @@ -377,7 +391,11 @@ public void nextNull() throws IOException { public void setLenient(final boolean lenient) {} @Override - public void skipValue() throws IOException {} + public void skipValue() throws IOException { + if (!stack.isEmpty()) { + stack.removeLast(); + } + } @SuppressWarnings("TypeParameterUnusedInFormals") @Nullable diff --git a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt index 45c97122ee3..89ef84247f4 100644 --- a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt +++ b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt @@ -18,6 +18,24 @@ class JsonObjectReaderTest { val fixture = Fixture() + private val throwingValueDeserializer = + JsonDeserializer { reader, _ -> + reader.beginObject() + reader.nextName() + val value = reader.nextString() + if (value == "fail") { + throw IllegalStateException("intentional") + } + reader.endObject() + value + } + + private fun getValuesReader(jsonValue: String): JsonObjectReader = + fixture.getSut("{\"values\": $jsonValue}").apply { + beginObject() + nextName() + } + // nextStringOrNull @Test @@ -198,6 +216,87 @@ class JsonObjectReaderTest { verify(fixture.logger, never()).log(any(), any(), any()) } + @Test(timeout = 1000L) + fun `nextListOrNull skips a failing element`() { + val actual = + getValuesReader("[{\"value\": \"fail\"}]") + .nextListOrNull(fixture.logger, throwingValueDeserializer) + + assertEquals(emptyList(), actual) + } + + @Test(timeout = 1000L) + fun `nextListOrNull keeps elements before a failing element`() { + val actual = + getValuesReader("[{\"value\": \"one\"}, {\"value\": \"fail\"}]") + .nextListOrNull(fixture.logger, throwingValueDeserializer) + + assertEquals(listOf("one"), actual) + } + + @Test(timeout = 1000L) + fun `nextListOrNull keeps elements after a failing element`() { + val actual = + getValuesReader("[{\"value\": \"fail\"}, {\"value\": \"two\"}]") + .nextListOrNull(fixture.logger, throwingValueDeserializer) + + assertEquals(listOf("two"), actual) + } + + @Test(timeout = 1000L) + fun `nextMapOrNull skips a failing value`() { + val actual = + getValuesReader("{\"bad\": {\"value\": \"fail\"}}") + .nextMapOrNull(fixture.logger, throwingValueDeserializer) + + assertEquals(emptyMap(), actual) + } + + @Test(timeout = 1000L) + fun `nextMapOrNull keeps values before a failing value`() { + val actual = + getValuesReader("{\"good\": {\"value\": \"one\"}, \"bad\": {\"value\": \"fail\"}}") + .nextMapOrNull(fixture.logger, throwingValueDeserializer) + + assertEquals(mapOf("good" to "one"), actual) + } + + @Test(timeout = 1000L) + fun `nextMapOrNull keeps values after a failing value`() { + val actual = + getValuesReader("{\"bad\": {\"value\": \"fail\"}, \"good\": {\"value\": \"two\"}}") + .nextMapOrNull(fixture.logger, throwingValueDeserializer) + + assertEquals(mapOf("good" to "two"), actual) + } + + @Test(timeout = 1000L) + fun `nextMapOfListOrNull skips a failing value`() { + val actual = + getValuesReader("{\"bad\": {\"value\": \"fail\"}}") + .nextMapOfListOrNull(fixture.logger, throwingValueDeserializer) + + assertEquals(emptyMap(), actual) + } + + @Test(timeout = 1000L) + fun `nextMapOfListOrNull keeps values before a failing value`() { + val actual = + getValuesReader("{\"good\": [{\"value\": \"one\"}], \"bad\": {\"value\": \"fail\"}}") + .nextMapOfListOrNull(fixture.logger, throwingValueDeserializer) + + assertEquals(mapOf("good" to listOf("one")), actual) + } + + @Test(timeout = 1000L) + fun `nextMapOfListOrNull keeps values after a failing value`() { + val actual = + getValuesReader("{\"bad\": {\"value\": \"fail\"}, \"good\": [{\"value\": \"two\"}]}") + .nextMapOfListOrNull(fixture.logger, throwingValueDeserializer) + + assertEquals(mapOf("good" to listOf("two")), actual) + } + // nextDateOrNull @Test diff --git a/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt b/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt index 26991bd4937..4ac28c90226 100644 --- a/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt +++ b/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt @@ -13,8 +13,8 @@ import java.util.Currency import java.util.Date import java.util.Locale import java.util.TimeZone -import kotlin.test.Test import kotlin.test.assertEquals +import org.junit.Test class MapObjectReaderTest { enum class BasicEnum { @@ -39,9 +39,18 @@ class MapObjectReaderTest { } } + private val logger = NoOpLogger.getInstance() + + private fun getValuesReader(value: Any): MapObjectReader = + MapObjectReader(linkedMapOf("values" to value)).apply { + beginObject() + nextName() + } + + private fun serializableValue(value: String): Map = linkedMapOf("test" to value) + @Test fun `deserializes data correctly`() { - val logger = NoOpLogger.getInstance() val data = mutableMapOf() val writer = MapObjectWriter(data) @@ -145,4 +154,94 @@ class MapObjectReaderTest { reader.nextNull() reader.endObject() } + + @Test(timeout = 1000L) + fun `nextListOrNull skips a failing element`() { + val actual = + getValuesReader(listOf("fail")).nextListOrNull(logger, BasicSerializable.Deserializer()) + + assertEquals(emptyList(), actual) + } + + @Test(timeout = 1000L) + fun `nextListOrNull keeps elements before a failing element`() { + val actual = + getValuesReader(listOf(serializableValue("one"), "fail")) + .nextListOrNull(logger, BasicSerializable.Deserializer()) + + assertEquals(listOf(BasicSerializable("one")), actual) + } + + @Test(timeout = 1000L) + fun `nextListOrNull keeps elements after a failing element`() { + val actual = + getValuesReader(listOf("fail", serializableValue("two"))) + .nextListOrNull(logger, BasicSerializable.Deserializer()) + + assertEquals(listOf(BasicSerializable("two")), actual) + } + + @Test(timeout = 1000L) + fun `nextMapOrNull skips a failing value`() { + val actual = + getValuesReader(linkedMapOf("bad" to "fail")) + .nextMapOrNull(logger, BasicSerializable.Deserializer()) + + assertEquals(emptyMap(), actual) + } + + @Test(timeout = 1000L) + fun `nextMapOrNull keeps values before a failing value`() { + val actual = + getValuesReader(linkedMapOf("bad" to "fail", "good" to serializableValue("one"))) + .nextMapOrNull(logger, BasicSerializable.Deserializer()) + + assertEquals(mapOf("good" to BasicSerializable("one")), actual) + } + + @Test(timeout = 1000L) + fun `nextMapOrNull keeps values after a failing value`() { + val actual = + getValuesReader(linkedMapOf("good" to serializableValue("two"), "bad" to "fail")) + .nextMapOrNull(logger, BasicSerializable.Deserializer()) + + assertEquals(mapOf("good" to BasicSerializable("two")), actual) + } + + @Test(timeout = 1000L) + fun `nextMapOfListOrNull skips a failing value`() { + val actual = + getValuesReader(linkedMapOf("bad" to serializableValue("fail"))) + .nextMapOfListOrNull(logger, BasicSerializable.Deserializer()) + + assertEquals(emptyMap(), actual) + } + + @Test(timeout = 1000L) + fun `nextMapOfListOrNull keeps values before a failing value`() { + val actual = + getValuesReader( + linkedMapOf( + "bad" to serializableValue("fail"), + "good" to listOf(serializableValue("one")), + ) + ) + .nextMapOfListOrNull(logger, BasicSerializable.Deserializer()) + + assertEquals(mapOf("good" to listOf(BasicSerializable("one"))), actual) + } + + @Test(timeout = 1000L) + fun `nextMapOfListOrNull keeps values after a failing value`() { + val actual = + getValuesReader( + linkedMapOf( + "good" to listOf(serializableValue("two")), + "bad" to serializableValue("fail"), + ) + ) + .nextMapOfListOrNull(logger, BasicSerializable.Deserializer()) + + assertEquals(mapOf("good" to listOf(BasicSerializable("two"))), actual) + } } From 1f8edab75e5e2342d75263c6cc7f9080c1fffe65 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 14 Apr 2026 13:52:29 +0200 Subject: [PATCH 02/17] fix(sentry): Guard JsonObjectReader recovery failures Abort collection recovery gracefully when the stream is already unrecoverable instead of letting recoverValue fail from inside the original error handler. Log an explicit error and stop iterating so malformed or truncated JSON does not leave the reader in a worse state while still allowing the container to close cleanly. Refs GH-5278 Co-Authored-By: Claude --- .../main/java/io/sentry/JsonObjectReader.java | 30 +++++++++-- .../java/io/sentry/JsonObjectReaderTest.kt | 50 +++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/sentry/src/main/java/io/sentry/JsonObjectReader.java b/sentry/src/main/java/io/sentry/JsonObjectReader.java index 4a8134ede1a..c5048d2eed7 100644 --- a/sentry/src/main/java/io/sentry/JsonObjectReader.java +++ b/sentry/src/main/java/io/sentry/JsonObjectReader.java @@ -117,7 +117,15 @@ public void nextUnknown(ILogger logger, Map unknown, String name list.add(deserializer.deserialize(this, logger)); } catch (Exception e) { logger.log(SentryLevel.WARNING, "Failed to deserialize object in list.", e); - recoverValue(startDepth, startToken); + try { + recoverValue(startDepth, startToken); + } catch (Exception recoveryException) { + logger.log( + SentryLevel.ERROR, + "Stream unrecoverable, aborting list deserialization.", + recoveryException); + break; + } } } while (jsonReader.peek() == JsonToken.BEGIN_OBJECT); } @@ -143,7 +151,15 @@ public void nextUnknown(ILogger logger, Map unknown, String name map.put(key, deserializer.deserialize(this, logger)); } catch (Exception e) { logger.log(SentryLevel.WARNING, "Failed to deserialize object in map.", e); - recoverValue(startDepth, startToken); + try { + recoverValue(startDepth, startToken); + } catch (Exception recoveryException) { + logger.log( + SentryLevel.ERROR, + "Stream unrecoverable, aborting map deserialization.", + recoveryException); + break; + } } } while (jsonReader.peek() == JsonToken.BEGIN_OBJECT || jsonReader.peek() == JsonToken.NAME); } @@ -175,7 +191,15 @@ public void nextUnknown(ILogger logger, Map unknown, String name } } catch (Exception e) { logger.log(SentryLevel.WARNING, "Failed to deserialize list in map.", e); - recoverValue(startDepth, startToken); + try { + recoverValue(startDepth, startToken); + } catch (Exception recoveryException) { + logger.log( + SentryLevel.ERROR, + "Stream unrecoverable, aborting map-of-lists deserialization.", + recoveryException); + break; + } } } while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME); } diff --git a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt index 89ef84247f4..5eb360340a6 100644 --- a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt +++ b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt @@ -2,9 +2,11 @@ package io.sentry import java.io.StringReader import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertNull import org.junit.Test import org.mockito.kotlin.any +import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify @@ -297,6 +299,54 @@ class JsonObjectReaderTest { assertEquals(mapOf("good" to listOf("two")), actual) } + @Test(timeout = 1000L) + fun `nextListOrNull logs and aborts when recovery fails`() { + assertFailsWith { + fixture + .getSut("[{\"value\": \"fail\"") + .nextListOrNull(fixture.logger, throwingValueDeserializer) + } + + verify(fixture.logger) + .log( + eq(SentryLevel.ERROR), + eq("Stream unrecoverable, aborting list deserialization."), + any(), + ) + } + + @Test(timeout = 1000L) + fun `nextMapOrNull logs and aborts when recovery fails`() { + assertFailsWith { + fixture + .getSut("{\"bad\": {\"value\": \"fail\"") + .nextMapOrNull(fixture.logger, throwingValueDeserializer) + } + + verify(fixture.logger) + .log( + eq(SentryLevel.ERROR), + eq("Stream unrecoverable, aborting map deserialization."), + any(), + ) + } + + @Test(timeout = 1000L) + fun `nextMapOfListOrNull logs and aborts when recovery fails`() { + assertFailsWith { + fixture + .getSut("{\"bad\": [{\"value\": \"fail\"") + .nextMapOfListOrNull(fixture.logger, throwingValueDeserializer) + } + + verify(fixture.logger) + .log( + eq(SentryLevel.ERROR), + eq("Stream unrecoverable, aborting map-of-lists deserialization."), + any(), + ) + } + // nextDateOrNull @Test From f823221dadfda2c7fcf355844ac2b70ba435b9f3 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 14 Apr 2026 14:05:42 +0200 Subject: [PATCH 03/17] ref(sentry): Extract JsonObjectReader recovery helper Move the repeated recovery logging flow behind a dedicated helper so the collection readers stay focused on parsing and keep the guarded recovery behavior aligned across all three paths. This does not change behavior. It only removes duplicated error handling around failed value recovery. Refs GH-5278 Co-Authored-By: Claude --- .../main/java/io/sentry/JsonObjectReader.java | 62 ++++++++++++------- 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/sentry/src/main/java/io/sentry/JsonObjectReader.java b/sentry/src/main/java/io/sentry/JsonObjectReader.java index c5048d2eed7..32651260db4 100644 --- a/sentry/src/main/java/io/sentry/JsonObjectReader.java +++ b/sentry/src/main/java/io/sentry/JsonObjectReader.java @@ -116,14 +116,13 @@ public void nextUnknown(ILogger logger, Map unknown, String name try { list.add(deserializer.deserialize(this, logger)); } catch (Exception e) { - logger.log(SentryLevel.WARNING, "Failed to deserialize object in list.", e); - try { - recoverValue(startDepth, startToken); - } catch (Exception recoveryException) { - logger.log( - SentryLevel.ERROR, - "Stream unrecoverable, aborting list deserialization.", - recoveryException); + if (!recoverAfterValueFailure( + logger, + e, + "Failed to deserialize object in list.", + "Stream unrecoverable, aborting list deserialization.", + startDepth, + startToken)) { break; } } @@ -150,14 +149,13 @@ public void nextUnknown(ILogger logger, Map unknown, String name try { map.put(key, deserializer.deserialize(this, logger)); } catch (Exception e) { - logger.log(SentryLevel.WARNING, "Failed to deserialize object in map.", e); - try { - recoverValue(startDepth, startToken); - } catch (Exception recoveryException) { - logger.log( - SentryLevel.ERROR, - "Stream unrecoverable, aborting map deserialization.", - recoveryException); + if (!recoverAfterValueFailure( + logger, + e, + "Failed to deserialize object in map.", + "Stream unrecoverable, aborting map deserialization.", + startDepth, + startToken)) { break; } } @@ -190,14 +188,13 @@ public void nextUnknown(ILogger logger, Map unknown, String name result.put(key, list); } } catch (Exception e) { - logger.log(SentryLevel.WARNING, "Failed to deserialize list in map.", e); - try { - recoverValue(startDepth, startToken); - } catch (Exception recoveryException) { - logger.log( - SentryLevel.ERROR, - "Stream unrecoverable, aborting map-of-lists deserialization.", - recoveryException); + if (!recoverAfterValueFailure( + logger, + e, + "Failed to deserialize list in map.", + "Stream unrecoverable, aborting map-of-lists deserialization.", + startDepth, + startToken)) { break; } } @@ -331,6 +328,23 @@ public void skipValue() throws IOException { jsonReader.skipValue(); } + private boolean recoverAfterValueFailure( + final @NotNull ILogger logger, + final @NotNull Exception error, + final @NotNull String warningMessage, + final @NotNull String unrecoverableMessage, + final int startDepth, + final @NotNull JsonToken startToken) { + logger.log(SentryLevel.WARNING, warningMessage, error); + try { + recoverValue(startDepth, startToken); + return true; + } catch (Exception recoveryException) { + logger.log(SentryLevel.ERROR, unrecoverableMessage, recoveryException); + return false; + } + } + private void recoverValue(final int startDepth, final @NotNull JsonToken startToken) throws IOException { final boolean enteredNestedValue = depth > startDepth; From 55321ec0e6208aa2dbb51e57830ffd97ce51cc83 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 14 Apr 2026 14:12:57 +0200 Subject: [PATCH 04/17] 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 --- .../java/io/sentry/util/MapObjectReader.java | 22 ++++--- .../io/sentry/util/MapObjectReaderTest.kt | 61 +++++++++++++++++++ 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/sentry/src/main/java/io/sentry/util/MapObjectReader.java b/sentry/src/main/java/io/sentry/util/MapObjectReader.java index 166ef9bdab9..ab7afb54b6b 100644 --- a/sentry/src/main/java/io/sentry/util/MapObjectReader.java +++ b/sentry/src/main/java/io/sentry/util/MapObjectReader.java @@ -31,15 +31,12 @@ public MapObjectReader(final Map root) { @Override public void nextUnknown( final @NotNull ILogger logger, final Map unknown, final String name) { + final int stackSizeBefore = stack.size(); try { unknown.put(name, nextObjectOrNull()); } catch (Exception exception) { logger.log(SentryLevel.ERROR, exception, "Error deserializing unknown key: %s", name); - try { - skipValue(); - } catch (Exception ignored) { - // stream is unrecoverable - } + recoverValue(stackSizeBefore); } } @@ -57,11 +54,12 @@ public List nextListOrNull( List list = new ArrayList<>(); if (hasNext()) { do { + final int stackSizeBefore = stack.size(); try { list.add(deserializer.deserialize(this, logger)); } catch (Exception e) { logger.log(SentryLevel.WARNING, "Failed to deserialize object in list.", e); - skipValue(); + recoverValue(stackSizeBefore); } } while (peek() == JsonToken.BEGIN_OBJECT); } @@ -87,11 +85,12 @@ public Map nextMapOrNull( if (hasNext()) { do { final String key = nextName(); + final int stackSizeBefore = stack.size(); try { map.put(key, deserializer.deserialize(this, logger)); } catch (Exception e) { logger.log(SentryLevel.WARNING, "Failed to deserialize object in map.", e); - skipValue(); + recoverValue(stackSizeBefore); } } while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME); } @@ -116,6 +115,7 @@ public Map nextMapOrNull( if (hasNext()) { do { final @NotNull String key = nextName(); + final int stackSizeBefore = stack.size(); try { final @Nullable List list = nextListOrNull(logger, deserializer); if (list != null) { @@ -123,7 +123,7 @@ public Map nextMapOrNull( } } catch (Exception e) { logger.log(SentryLevel.WARNING, "Failed to deserialize list in map.", e); - skipValue(); + recoverValue(stackSizeBefore); } } while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME); } @@ -397,6 +397,12 @@ public void skipValue() throws IOException { } } + private void recoverValue(final int stackSizeBefore) { + while (!stack.isEmpty() && stack.size() >= stackSizeBefore) { + stack.removeLast(); + } + } + @SuppressWarnings("TypeParameterUnusedInFormals") @Nullable private T nextValueOrNull() throws IOException { diff --git a/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt b/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt index 4ac28c90226..06270ffcced 100644 --- a/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt +++ b/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt @@ -49,6 +49,25 @@ class MapObjectReaderTest { private fun serializableValue(value: String): Map = linkedMapOf("test" to value) + private fun partialSerializableValue(kind: String, value: String): Map = + linkedMapOf("test" to value, "kind" to kind) + + private val partiallyFailingDeserializer = + JsonDeserializer { reader, _ -> + val basicSerializable = BasicSerializable() + reader.beginObject() + if (reader.nextName() == "kind") { + if (reader.nextString() == "fail") { + throw IllegalStateException("intentional") + } + } + if (reader.nextName() == "test") { + basicSerializable.test = reader.nextString() + } + reader.endObject() + basicSerializable + } + @Test fun `deserializes data correctly`() { val data = mutableMapOf() @@ -244,4 +263,46 @@ class MapObjectReaderTest { assertEquals(mapOf("good" to listOf(BasicSerializable("two"))), actual) } + + @Test(timeout = 1000L) + fun `nextListOrNull keeps elements after a partially consumed failing element`() { + val actual = + getValuesReader( + listOf(partialSerializableValue("fail", "bad"), partialSerializableValue("ok", "two")) + ) + .nextListOrNull(logger, partiallyFailingDeserializer) + + assertEquals(listOf(BasicSerializable("two")), actual) + } + + @Test(timeout = 1000L) + fun `nextMapOrNull keeps values after a partially consumed failing value`() { + val actual = + getValuesReader( + linkedMapOf( + "bad" to partialSerializableValue("fail", "bad"), + "good" to partialSerializableValue("ok", "two"), + ) + ) + .nextMapOrNull(logger, partiallyFailingDeserializer) + + assertEquals(mapOf("good" to BasicSerializable("two")), actual) + } + + @Test(timeout = 1000L) + fun `nextMapOfListOrNull keeps values after a partially consumed failing element`() { + val actual = + getValuesReader( + linkedMapOf( + "bad" to listOf(partialSerializableValue("fail", "bad")), + "good" to listOf(partialSerializableValue("ok", "two")), + ) + ) + .nextMapOfListOrNull(logger, partiallyFailingDeserializer) + + assertEquals( + mapOf("bad" to emptyList(), "good" to listOf(BasicSerializable("two"))), + actual, + ) + } } From 98fb62596c2127943cc0d21f652d08133447bc04 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 14 Apr 2026 14:15:53 +0200 Subject: [PATCH 05/17] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4a9266f261..5073c5297f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Fix `JsonObjectReader` and `MapObjectReader` hanging indefinitely when deserialization errors leave the reader in an inconsistent state; failed values are now skipped, parsing continues, and a WARNING-level log entry is emitted for each skipped value ([#5293](https://github.com/getsentry/sentry-java/pull/5293)) + ### Dependencies - Bump Native SDK from v0.13.3 to v0.13.6 ([#5277](https://github.com/getsentry/sentry-java/pull/5277)) From d6f8299d1faffec9664febb11853ed08a2aa2a37 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 15 Apr 2026 12:18:00 +0200 Subject: [PATCH 06/17] fix(sentry): Track container entry during JSON recovery Track recovery state per collection element so post-parse validation failures do not cause the next sibling container to be skipped. This keeps the livelock fix while preserving later valid values after a failed object deserializer. Refs GH-5278 Co-Authored-By: Claude --- .../main/java/io/sentry/JsonObjectReader.java | 101 +++++++++++++----- .../java/io/sentry/JsonObjectReaderTest.kt | 39 +++++++ 2 files changed, 113 insertions(+), 27 deletions(-) diff --git a/sentry/src/main/java/io/sentry/JsonObjectReader.java b/sentry/src/main/java/io/sentry/JsonObjectReader.java index 32651260db4..0a447c77b62 100644 --- a/sentry/src/main/java/io/sentry/JsonObjectReader.java +++ b/sentry/src/main/java/io/sentry/JsonObjectReader.java @@ -4,8 +4,10 @@ import io.sentry.vendor.gson.stream.JsonToken; import java.io.IOException; import java.io.Reader; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Date; +import java.util.Deque; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -18,6 +20,7 @@ public final class JsonObjectReader implements ObjectReader { private final @NotNull JsonReader jsonReader; + private final @NotNull Deque recoveryStates = new ArrayDeque<>(); private int depth = 0; public JsonObjectReader(Reader in) { @@ -85,18 +88,21 @@ public float nextFloat() throws IOException { @Override public void nextUnknown(ILogger logger, Map unknown, String name) { - final int startDepth = depth; - JsonToken startToken = JsonToken.END_DOCUMENT; + RecoveryState recoveryState = null; try { - startToken = peek(); + recoveryState = beginRecovery(peek()); unknown.put(name, nextObjectOrNull()); } catch (Exception exception) { logger.log(SentryLevel.ERROR, exception, "Error deserializing unknown key: %s", name); - try { - recoverValue(startDepth, startToken); - } catch (Exception ignored) { - // stream is unrecoverable + if (recoveryState != null) { + try { + recoverValue(recoveryState); + } catch (Exception ignored) { + // stream is unrecoverable + } } + } finally { + endRecovery(recoveryState); } } @@ -111,8 +117,7 @@ public void nextUnknown(ILogger logger, Map unknown, String name List list = new ArrayList<>(); if (jsonReader.hasNext()) { do { - final int startDepth = depth; - final JsonToken startToken = peek(); + final RecoveryState recoveryState = beginRecovery(peek()); try { list.add(deserializer.deserialize(this, logger)); } catch (Exception e) { @@ -121,10 +126,11 @@ public void nextUnknown(ILogger logger, Map unknown, String name e, "Failed to deserialize object in list.", "Stream unrecoverable, aborting list deserialization.", - startDepth, - startToken)) { + recoveryState)) { break; } + } finally { + endRecovery(recoveryState); } } while (jsonReader.peek() == JsonToken.BEGIN_OBJECT); } @@ -144,8 +150,7 @@ public void nextUnknown(ILogger logger, Map unknown, String name if (jsonReader.hasNext()) { do { final String key = jsonReader.nextName(); - final int startDepth = depth; - final JsonToken startToken = peek(); + final RecoveryState recoveryState = beginRecovery(peek()); try { map.put(key, deserializer.deserialize(this, logger)); } catch (Exception e) { @@ -154,10 +159,11 @@ public void nextUnknown(ILogger logger, Map unknown, String name e, "Failed to deserialize object in map.", "Stream unrecoverable, aborting map deserialization.", - startDepth, - startToken)) { + recoveryState)) { break; } + } finally { + endRecovery(recoveryState); } } while (jsonReader.peek() == JsonToken.BEGIN_OBJECT || jsonReader.peek() == JsonToken.NAME); } @@ -180,8 +186,7 @@ public void nextUnknown(ILogger logger, Map unknown, String name if (hasNext()) { do { final @NotNull String key = nextName(); - final int startDepth = depth; - final JsonToken startToken = peek(); + final RecoveryState recoveryState = beginRecovery(peek()); try { final @Nullable List list = nextListOrNull(logger, deserializer); if (list != null) { @@ -193,10 +198,11 @@ public void nextUnknown(ILogger logger, Map unknown, String name e, "Failed to deserialize list in map.", "Stream unrecoverable, aborting map-of-lists deserialization.", - startDepth, - startToken)) { + recoveryState)) { break; } + } finally { + endRecovery(recoveryState); } } while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME); } @@ -263,6 +269,7 @@ public void nextUnknown(ILogger logger, Map unknown, String name public void beginObject() throws IOException { jsonReader.beginObject(); depth++; + markContainerEntered(); } @Override @@ -275,6 +282,7 @@ public void endObject() throws IOException { public void beginArray() throws IOException { jsonReader.beginArray(); depth++; + markContainerEntered(); } @Override @@ -333,11 +341,10 @@ private boolean recoverAfterValueFailure( final @NotNull Exception error, final @NotNull String warningMessage, final @NotNull String unrecoverableMessage, - final int startDepth, - final @NotNull JsonToken startToken) { + final @NotNull RecoveryState recoveryState) { logger.log(SentryLevel.WARNING, warningMessage, error); try { - recoverValue(startDepth, startToken); + recoverValue(recoveryState); return true; } catch (Exception recoveryException) { logger.log(SentryLevel.ERROR, unrecoverableMessage, recoveryException); @@ -345,10 +352,35 @@ private boolean recoverAfterValueFailure( } } - private void recoverValue(final int startDepth, final @NotNull JsonToken startToken) - throws IOException { - final boolean enteredNestedValue = depth > startDepth; - while (depth > startDepth) { + private @NotNull RecoveryState beginRecovery(final @NotNull JsonToken startToken) { + final RecoveryState recoveryState = new RecoveryState(depth, startToken); + recoveryStates.addLast(recoveryState); + return recoveryState; + } + + private void endRecovery(final @Nullable RecoveryState recoveryState) { + if (recoveryState == null) { + return; + } + if (!recoveryStates.isEmpty() && recoveryStates.peekLast() == recoveryState) { + recoveryStates.removeLast(); + } else { + recoveryStates.remove(recoveryState); + } + } + + private void markContainerEntered() { + for (RecoveryState recoveryState : recoveryStates) { + if (!recoveryState.enteredContainer + && isContainerStartToken(recoveryState.startToken) + && depth > recoveryState.startDepth) { + recoveryState.enteredContainer = true; + } + } + } + + private void recoverValue(final @NotNull RecoveryState recoveryState) throws IOException { + while (depth > recoveryState.startDepth) { final JsonToken token = peek(); if (token == JsonToken.END_OBJECT) { endObject(); @@ -359,13 +391,28 @@ private void recoverValue(final int startDepth, final @NotNull JsonToken startTo } } - if (!enteredNestedValue && peek() == startToken) { + if (!recoveryState.enteredContainer && peek() == recoveryState.startToken) { skipValue(); } } + private static boolean isContainerStartToken(final @NotNull JsonToken token) { + return token == JsonToken.BEGIN_OBJECT || token == JsonToken.BEGIN_ARRAY; + } + @Override public void close() throws IOException { jsonReader.close(); } + + private static final class RecoveryState { + private final int startDepth; + private final @NotNull JsonToken startToken; + private boolean enteredContainer; + + private RecoveryState(final int startDepth, final @NotNull JsonToken startToken) { + this.startDepth = startDepth; + this.startToken = startToken; + } + } } diff --git a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt index 5eb360340a6..e0a5eb422f7 100644 --- a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt +++ b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt @@ -32,6 +32,18 @@ class JsonObjectReaderTest { value } + private val postParseThrowingValueDeserializer = + JsonDeserializer { reader, _ -> + reader.beginObject() + reader.nextName() + val value = reader.nextString() + reader.endObject() + if (value == "fail") { + throw IllegalStateException("intentional") + } + value + } + private fun getValuesReader(jsonValue: String): JsonObjectReader = fixture.getSut("{\"values\": $jsonValue}").apply { beginObject() @@ -227,6 +239,24 @@ class JsonObjectReaderTest { assertEquals(emptyList(), actual) } + @Test(timeout = 1000L) + fun `nextListOrNull skips an unconsumed failing element`() { + var callCount = 0 + val deserializer = + JsonDeserializer { reader, logger -> + if (callCount++ == 0) { + throw IllegalStateException("intentional") + } + throwingValueDeserializer.deserialize(reader, logger) + } + + val actual = + getValuesReader("[{\"value\": \"ignored\"}, {\"value\": \"two\"}]") + .nextListOrNull(fixture.logger, deserializer) + + assertEquals(listOf("two"), actual) + } + @Test(timeout = 1000L) fun `nextListOrNull keeps elements before a failing element`() { val actual = @@ -245,6 +275,15 @@ class JsonObjectReaderTest { assertEquals(listOf("two"), actual) } + @Test(timeout = 1000L) + fun `nextListOrNull keeps elements after a fully consumed failing element`() { + val actual = + getValuesReader("[{\"value\": \"fail\"}, {\"value\": \"two\"}]") + .nextListOrNull(fixture.logger, postParseThrowingValueDeserializer) + + assertEquals(listOf("two"), actual) + } + @Test(timeout = 1000L) fun `nextMapOrNull skips a failing value`() { val actual = From cc962a6c56a319f47797a0c82c4ef9047075ceb3 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 15 Apr 2026 12:52:39 +0200 Subject: [PATCH 07/17] fix(sentry): Track consumed values during JSON recovery Treat recovery state as consumed when the current value has already been read, including skipValue()-driven failures. This prevents the fallback recovery step from skipping the next valid sibling after a deserializer consumes a value and then throws. Add regressions for direct list recovery and nested map-of-list recovery so consumed-value failures keep later valid entries. Refs GH-5278 Co-Authored-By: Claude --- .../main/java/io/sentry/JsonObjectReader.java | 67 ++++++++++--------- .../java/io/sentry/JsonObjectReaderTest.kt | 38 +++++++++++ 2 files changed, 72 insertions(+), 33 deletions(-) diff --git a/sentry/src/main/java/io/sentry/JsonObjectReader.java b/sentry/src/main/java/io/sentry/JsonObjectReader.java index 0a447c77b62..37ce6c95adc 100644 --- a/sentry/src/main/java/io/sentry/JsonObjectReader.java +++ b/sentry/src/main/java/io/sentry/JsonObjectReader.java @@ -30,25 +30,25 @@ public JsonObjectReader(Reader in) { @Override public @Nullable String nextStringOrNull() throws IOException { if (jsonReader.peek() == JsonToken.NULL) { - jsonReader.nextNull(); + nextNull(); return null; } - return jsonReader.nextString(); + return nextString(); } @Override public @Nullable Double nextDoubleOrNull() throws IOException { if (jsonReader.peek() == JsonToken.NULL) { - jsonReader.nextNull(); + nextNull(); return null; } - return jsonReader.nextDouble(); + return nextDouble(); } @Override public @Nullable Float nextFloatOrNull() throws IOException { if (jsonReader.peek() == JsonToken.NULL) { - jsonReader.nextNull(); + nextNull(); return null; } return nextFloat(); @@ -56,34 +56,35 @@ public JsonObjectReader(Reader in) { @Override public float nextFloat() throws IOException { + markValueConsumed(); return (float) jsonReader.nextDouble(); } @Override public @Nullable Long nextLongOrNull() throws IOException { if (jsonReader.peek() == JsonToken.NULL) { - jsonReader.nextNull(); + nextNull(); return null; } - return jsonReader.nextLong(); + return nextLong(); } @Override public @Nullable Integer nextIntegerOrNull() throws IOException { if (jsonReader.peek() == JsonToken.NULL) { - jsonReader.nextNull(); + nextNull(); return null; } - return jsonReader.nextInt(); + return nextInt(); } @Override public @Nullable Boolean nextBooleanOrNull() throws IOException { if (jsonReader.peek() == JsonToken.NULL) { - jsonReader.nextNull(); + nextNull(); return null; } - return jsonReader.nextBoolean(); + return nextBoolean(); } @Override @@ -110,7 +111,7 @@ public void nextUnknown(ILogger logger, Map unknown, String name public @Nullable List nextListOrNull( @NotNull ILogger logger, @NotNull JsonDeserializer deserializer) throws IOException { if (jsonReader.peek() == JsonToken.NULL) { - jsonReader.nextNull(); + nextNull(); return null; } beginArray(); @@ -142,7 +143,7 @@ public void nextUnknown(ILogger logger, Map unknown, String name public @Nullable Map nextMapOrNull( @NotNull ILogger logger, @NotNull JsonDeserializer deserializer) throws IOException { if (jsonReader.peek() == JsonToken.NULL) { - jsonReader.nextNull(); + nextNull(); return null; } beginObject(); @@ -215,7 +216,7 @@ public void nextUnknown(ILogger logger, Map unknown, String name public @Nullable T nextOrNull( @NotNull ILogger logger, @NotNull JsonDeserializer deserializer) throws Exception { if (jsonReader.peek() == JsonToken.NULL) { - jsonReader.nextNull(); + nextNull(); return null; } return deserializer.deserialize(this, logger); @@ -224,20 +225,20 @@ public void nextUnknown(ILogger logger, Map unknown, String name @Override public @Nullable Date nextDateOrNull(ILogger logger) throws IOException { if (jsonReader.peek() == JsonToken.NULL) { - jsonReader.nextNull(); + nextNull(); return null; } - return ObjectReader.dateOrNull(jsonReader.nextString(), logger); + return ObjectReader.dateOrNull(nextString(), logger); } @Override public @Nullable TimeZone nextTimeZoneOrNull(ILogger logger) throws IOException { if (jsonReader.peek() == JsonToken.NULL) { - jsonReader.nextNull(); + nextNull(); return null; } try { - return TimeZone.getTimeZone(jsonReader.nextString()); + return TimeZone.getTimeZone(nextString()); } catch (Exception e) { logger.log(SentryLevel.ERROR, "Error when deserializing TimeZone", e); } @@ -268,8 +269,8 @@ public void nextUnknown(ILogger logger, Map unknown, String name @Override public void beginObject() throws IOException { jsonReader.beginObject(); + markValueConsumed(); depth++; - markContainerEntered(); } @Override @@ -281,8 +282,8 @@ public void endObject() throws IOException { @Override public void beginArray() throws IOException { jsonReader.beginArray(); + markValueConsumed(); depth++; - markContainerEntered(); } @Override @@ -298,31 +299,37 @@ public boolean hasNext() throws IOException { @Override public int nextInt() throws IOException { + markValueConsumed(); return jsonReader.nextInt(); } @Override public long nextLong() throws IOException { + markValueConsumed(); return jsonReader.nextLong(); } @Override public String nextString() throws IOException { + markValueConsumed(); return jsonReader.nextString(); } @Override public boolean nextBoolean() throws IOException { + markValueConsumed(); return jsonReader.nextBoolean(); } @Override public double nextDouble() throws IOException { + markValueConsumed(); return jsonReader.nextDouble(); } @Override public void nextNull() throws IOException { + markValueConsumed(); jsonReader.nextNull(); } @@ -333,6 +340,7 @@ public void setLenient(boolean lenient) { @Override public void skipValue() throws IOException { + markValueConsumed(); jsonReader.skipValue(); } @@ -369,13 +377,10 @@ private void endRecovery(final @Nullable RecoveryState recoveryState) { } } - private void markContainerEntered() { - for (RecoveryState recoveryState : recoveryStates) { - if (!recoveryState.enteredContainer - && isContainerStartToken(recoveryState.startToken) - && depth > recoveryState.startDepth) { - recoveryState.enteredContainer = true; - } + private void markValueConsumed() { + final @Nullable RecoveryState recoveryState = recoveryStates.peekLast(); + if (recoveryState != null) { + recoveryState.valueConsumed = true; } } @@ -391,15 +396,11 @@ private void recoverValue(final @NotNull RecoveryState recoveryState) throws IOE } } - if (!recoveryState.enteredContainer && peek() == recoveryState.startToken) { + if (!recoveryState.valueConsumed && peek() == recoveryState.startToken) { skipValue(); } } - private static boolean isContainerStartToken(final @NotNull JsonToken token) { - return token == JsonToken.BEGIN_OBJECT || token == JsonToken.BEGIN_ARRAY; - } - @Override public void close() throws IOException { jsonReader.close(); @@ -408,7 +409,7 @@ public void close() throws IOException { private static final class RecoveryState { private final int startDepth; private final @NotNull JsonToken startToken; - private boolean enteredContainer; + private boolean valueConsumed; private RecoveryState(final int startDepth, final @NotNull JsonToken startToken) { this.startDepth = startDepth; diff --git a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt index e0a5eb422f7..a5f2b464ab1 100644 --- a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt +++ b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt @@ -284,6 +284,25 @@ class JsonObjectReaderTest { assertEquals(listOf("two"), actual) } + @Test(timeout = 1000L) + fun `nextListOrNull keeps elements after skipValue consumes a failing element`() { + var callCount = 0 + val deserializer = + JsonDeserializer { reader, logger -> + if (callCount++ == 0) { + reader.skipValue() + throw IllegalStateException("intentional") + } + throwingValueDeserializer.deserialize(reader, logger) + } + + val actual = + getValuesReader("[{\"value\": \"ignored\"}, {\"value\": \"two\"}]") + .nextListOrNull(fixture.logger, deserializer) + + assertEquals(listOf("two"), actual) + } + @Test(timeout = 1000L) fun `nextMapOrNull skips a failing value`() { val actual = @@ -338,6 +357,25 @@ class JsonObjectReaderTest { assertEquals(mapOf("good" to listOf("two")), actual) } + @Test(timeout = 1000L) + fun `nextMapOfListOrNull keeps nested values after skipValue consumes a failing element`() { + var callCount = 0 + val deserializer = + JsonDeserializer { reader, logger -> + if (callCount++ == 0) { + reader.skipValue() + throw IllegalStateException("intentional") + } + throwingValueDeserializer.deserialize(reader, logger) + } + + val actual = + getValuesReader("{\"good\": [{\"value\": \"ignored\"}, {\"value\": \"two\"}]}") + .nextMapOfListOrNull(fixture.logger, deserializer) + + assertEquals(mapOf("good" to listOf("two")), actual) + } + @Test(timeout = 1000L) fun `nextListOrNull logs and aborts when recovery fails`() { assertFailsWith { From 50e4f4671563ef39159b8c001ec89acd0b4f3c41 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 15 Apr 2026 13:48:14 +0200 Subject: [PATCH 08/17] fix(sentry): Log unknown key recovery failures Log a second error when unknown-field recovery itself fails. This makes nextUnknown consistent with the list and map recovery paths and makes truncated payload failures easier to diagnose. Add regression coverage for unknown-key deserialization failures that also leave the stream unrecoverable. Co-Authored-By: Claude --- .../main/java/io/sentry/JsonObjectReader.java | 7 +++-- .../java/io/sentry/JsonObjectReaderTest.kt | 27 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/sentry/src/main/java/io/sentry/JsonObjectReader.java b/sentry/src/main/java/io/sentry/JsonObjectReader.java index 37ce6c95adc..82bf27e8211 100644 --- a/sentry/src/main/java/io/sentry/JsonObjectReader.java +++ b/sentry/src/main/java/io/sentry/JsonObjectReader.java @@ -98,8 +98,11 @@ public void nextUnknown(ILogger logger, Map unknown, String name if (recoveryState != null) { try { recoverValue(recoveryState); - } catch (Exception ignored) { - // stream is unrecoverable + } catch (Exception recoveryException) { + logger.log( + SentryLevel.ERROR, + "Stream unrecoverable after unknown key deserialization failure.", + recoveryException); } } } finally { diff --git a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt index a5f2b464ab1..895f3016c62 100644 --- a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt +++ b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt @@ -10,6 +10,7 @@ import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoMoreInteractions class JsonObjectReaderTest { class Fixture { @@ -424,6 +425,32 @@ class JsonObjectReaderTest { ) } + @Test(timeout = 1000L) + fun `nextUnknown logs when recovery fails`() { + val unknown = mutableMapOf() + val reader = fixture.getSut("{\"key\": {\"value\": \"fail\"") + reader.beginObject() + val name = reader.nextName() + + reader.nextUnknown(fixture.logger, unknown, name) + + assertEquals(emptyMap(), unknown) + verify(fixture.logger) + .log( + eq(SentryLevel.ERROR), + any(), + eq("Error deserializing unknown key: %s"), + eq("key"), + ) + verify(fixture.logger) + .log( + eq(SentryLevel.ERROR), + eq("Stream unrecoverable after unknown key deserialization failure."), + any(), + ) + verifyNoMoreInteractions(fixture.logger) + } + // nextDateOrNull @Test From 298720a750e6935ce1f226f892967335771ab8b8 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 15 Apr 2026 14:36:52 +0200 Subject: [PATCH 09/17] docs(changelog): Clarify JSON recovery log levels Clarify that collection recovery emits warning logs for skipped values, while unknown-key failures and unrecoverable recovery paths emit errors. This keeps the release note aligned with the actual logging behavior from the recovery fixes. Co-Authored-By: Claude --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5073c5297f1..b3639a36675 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,11 @@ ### Fixes -- Fix `JsonObjectReader` and `MapObjectReader` hanging indefinitely when deserialization errors leave the reader in an inconsistent state; failed values are now skipped, parsing continues, and a WARNING-level log entry is emitted for each skipped value ([#5293](https://github.com/getsentry/sentry-java/pull/5293)) +- Fix `JsonObjectReader` and `MapObjectReader` hanging indefinitely when deserialization + errors leave the reader in an inconsistent state ([#5293](https://github.com/getsentry/sentry-java/pull/5293)) + - Failed collection values are now skipped so parsing can continue + - Skipped collection values emit `WARNING` logs + - Unknown-key failures and unrecoverable recovery failures emit `ERROR` logs ### Dependencies From d710cca9a02cee7320e733d86e63904e1a72b8b3 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 15 Apr 2026 16:11:45 +0200 Subject: [PATCH 10/17] merge changelog lines --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3639a36675..9e160844531 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,7 @@ ### Fixes -- Fix `JsonObjectReader` and `MapObjectReader` hanging indefinitely when deserialization - errors leave the reader in an inconsistent state ([#5293](https://github.com/getsentry/sentry-java/pull/5293)) +- Fix `JsonObjectReader` and `MapObjectReader` hanging indefinitely when deserialization errors leave the reader in an inconsistent state ([#5293](https://github.com/getsentry/sentry-java/pull/5293)) - Failed collection values are now skipped so parsing can continue - Skipped collection values emit `WARNING` logs - Unknown-key failures and unrecoverable recovery failures emit `ERROR` logs From 356ddda836f6809de1823942a3820a87197407b3 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 16 Apr 2026 05:48:08 +0200 Subject: [PATCH 11/17] mark consumed afterwards instead of before --- .../main/java/io/sentry/JsonObjectReader.java | 20 ++++--- .../java/io/sentry/JsonObjectReaderTest.kt | 59 +++++++++++++++++++ 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/sentry/src/main/java/io/sentry/JsonObjectReader.java b/sentry/src/main/java/io/sentry/JsonObjectReader.java index 82bf27e8211..176227bba93 100644 --- a/sentry/src/main/java/io/sentry/JsonObjectReader.java +++ b/sentry/src/main/java/io/sentry/JsonObjectReader.java @@ -56,8 +56,9 @@ public JsonObjectReader(Reader in) { @Override public float nextFloat() throws IOException { + final double value = jsonReader.nextDouble(); markValueConsumed(); - return (float) jsonReader.nextDouble(); + return (float) value; } @Override @@ -302,38 +303,43 @@ public boolean hasNext() throws IOException { @Override public int nextInt() throws IOException { + final int value = jsonReader.nextInt(); markValueConsumed(); - return jsonReader.nextInt(); + return value; } @Override public long nextLong() throws IOException { + final long value = jsonReader.nextLong(); markValueConsumed(); - return jsonReader.nextLong(); + return value; } @Override public String nextString() throws IOException { + final String value = jsonReader.nextString(); markValueConsumed(); - return jsonReader.nextString(); + return value; } @Override public boolean nextBoolean() throws IOException { + final boolean value = jsonReader.nextBoolean(); markValueConsumed(); - return jsonReader.nextBoolean(); + return value; } @Override public double nextDouble() throws IOException { + final double value = jsonReader.nextDouble(); markValueConsumed(); - return jsonReader.nextDouble(); + return value; } @Override public void nextNull() throws IOException { - markValueConsumed(); jsonReader.nextNull(); + markValueConsumed(); } @Override diff --git a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt index 895f3016c62..865d996933f 100644 --- a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt +++ b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt @@ -51,6 +51,19 @@ class JsonObjectReaderTest { nextName() } + private fun assertNextMapOrNullRecoversAfterFailedPrimitiveRead( + badValue: String, + goodValue: String, + expectedValue: T, + deserializer: JsonDeserializer, + ) { + val actual = + getValuesReader("{\"bad\": $badValue, \"good\": $goodValue}") + .nextMapOrNull(fixture.logger, deserializer) + + assertEquals(mapOf("good" to expectedValue), actual) + } + // nextStringOrNull @Test @@ -313,6 +326,52 @@ class JsonObjectReaderTest { assertEquals(emptyMap(), actual) } + @Test(timeout = 1000L) + fun `nextMapOrNull recovers after failed primitive reads`() { + assertNextMapOrNullRecoversAfterFailedPrimitiveRead( + badValue = "true", + goodValue = "2", + expectedValue = 2, + deserializer = JsonDeserializer { reader, _ -> reader.nextInt() }, + ) + assertNextMapOrNullRecoversAfterFailedPrimitiveRead( + badValue = "true", + goodValue = "2", + expectedValue = 2L, + deserializer = JsonDeserializer { reader, _ -> reader.nextLong() }, + ) + assertNextMapOrNullRecoversAfterFailedPrimitiveRead( + badValue = "true", + goodValue = "\"two\"", + expectedValue = "two", + deserializer = JsonDeserializer { reader, _ -> reader.nextString() }, + ) + assertNextMapOrNullRecoversAfterFailedPrimitiveRead( + badValue = "1", + goodValue = "false", + expectedValue = false, + deserializer = JsonDeserializer { reader, _ -> reader.nextBoolean() }, + ) + assertNextMapOrNullRecoversAfterFailedPrimitiveRead( + badValue = "true", + goodValue = "2.5", + expectedValue = 2.5, + deserializer = JsonDeserializer { reader, _ -> reader.nextDouble() }, + ) + assertNextMapOrNullRecoversAfterFailedPrimitiveRead( + badValue = "true", + goodValue = "null", + expectedValue = Unit, + deserializer = JsonDeserializer { reader, _ -> reader.nextNull() }, + ) + assertNextMapOrNullRecoversAfterFailedPrimitiveRead( + badValue = "true", + goodValue = "2.5", + expectedValue = 2.5f, + deserializer = JsonDeserializer { reader, _ -> reader.nextFloat() }, + ) + } + @Test(timeout = 1000L) fun `nextMapOrNull keeps values before a failing value`() { val actual = From 8f2002106b5904eead8416f6b2fc1723c2aaf5a6 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 16 Apr 2026 10:39:31 +0200 Subject: [PATCH 12/17] test(sentry): Remove explicit timeouts from reader recovery tests Remove the explicit JUnit timeout annotations from the new JsonObjectReader and MapObjectReader recovery tests. These tests already fail deterministically and do not need per-test timeouts. Dropping the annotations keeps the new coverage aligned with the rest of the suite and avoids extra noise in the test definitions. Co-Authored-By: Claude --- .../java/io/sentry/JsonObjectReaderTest.kt | 36 +++++++++---------- .../io/sentry/util/MapObjectReaderTest.kt | 24 ++++++------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt index 865d996933f..71a4ad4f059 100644 --- a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt +++ b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt @@ -244,7 +244,7 @@ class JsonObjectReaderTest { verify(fixture.logger, never()).log(any(), any(), any()) } - @Test(timeout = 1000L) + @Test fun `nextListOrNull skips a failing element`() { val actual = getValuesReader("[{\"value\": \"fail\"}]") @@ -253,7 +253,7 @@ class JsonObjectReaderTest { assertEquals(emptyList(), actual) } - @Test(timeout = 1000L) + @Test fun `nextListOrNull skips an unconsumed failing element`() { var callCount = 0 val deserializer = @@ -271,7 +271,7 @@ class JsonObjectReaderTest { assertEquals(listOf("two"), actual) } - @Test(timeout = 1000L) + @Test fun `nextListOrNull keeps elements before a failing element`() { val actual = getValuesReader("[{\"value\": \"one\"}, {\"value\": \"fail\"}]") @@ -280,7 +280,7 @@ class JsonObjectReaderTest { assertEquals(listOf("one"), actual) } - @Test(timeout = 1000L) + @Test fun `nextListOrNull keeps elements after a failing element`() { val actual = getValuesReader("[{\"value\": \"fail\"}, {\"value\": \"two\"}]") @@ -289,7 +289,7 @@ class JsonObjectReaderTest { assertEquals(listOf("two"), actual) } - @Test(timeout = 1000L) + @Test fun `nextListOrNull keeps elements after a fully consumed failing element`() { val actual = getValuesReader("[{\"value\": \"fail\"}, {\"value\": \"two\"}]") @@ -298,7 +298,7 @@ class JsonObjectReaderTest { assertEquals(listOf("two"), actual) } - @Test(timeout = 1000L) + @Test fun `nextListOrNull keeps elements after skipValue consumes a failing element`() { var callCount = 0 val deserializer = @@ -317,7 +317,7 @@ class JsonObjectReaderTest { assertEquals(listOf("two"), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOrNull skips a failing value`() { val actual = getValuesReader("{\"bad\": {\"value\": \"fail\"}}") @@ -326,7 +326,7 @@ class JsonObjectReaderTest { assertEquals(emptyMap(), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOrNull recovers after failed primitive reads`() { assertNextMapOrNullRecoversAfterFailedPrimitiveRead( badValue = "true", @@ -372,7 +372,7 @@ class JsonObjectReaderTest { ) } - @Test(timeout = 1000L) + @Test fun `nextMapOrNull keeps values before a failing value`() { val actual = getValuesReader("{\"good\": {\"value\": \"one\"}, \"bad\": {\"value\": \"fail\"}}") @@ -381,7 +381,7 @@ class JsonObjectReaderTest { assertEquals(mapOf("good" to "one"), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOrNull keeps values after a failing value`() { val actual = getValuesReader("{\"bad\": {\"value\": \"fail\"}, \"good\": {\"value\": \"two\"}}") @@ -390,7 +390,7 @@ class JsonObjectReaderTest { assertEquals(mapOf("good" to "two"), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOfListOrNull skips a failing value`() { val actual = getValuesReader("{\"bad\": {\"value\": \"fail\"}}") @@ -399,7 +399,7 @@ class JsonObjectReaderTest { assertEquals(emptyMap(), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOfListOrNull keeps values before a failing value`() { val actual = getValuesReader("{\"good\": [{\"value\": \"one\"}], \"bad\": {\"value\": \"fail\"}}") @@ -408,7 +408,7 @@ class JsonObjectReaderTest { assertEquals(mapOf("good" to listOf("one")), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOfListOrNull keeps values after a failing value`() { val actual = getValuesReader("{\"bad\": {\"value\": \"fail\"}, \"good\": [{\"value\": \"two\"}]}") @@ -417,7 +417,7 @@ class JsonObjectReaderTest { assertEquals(mapOf("good" to listOf("two")), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOfListOrNull keeps nested values after skipValue consumes a failing element`() { var callCount = 0 val deserializer = @@ -436,7 +436,7 @@ class JsonObjectReaderTest { assertEquals(mapOf("good" to listOf("two")), actual) } - @Test(timeout = 1000L) + @Test fun `nextListOrNull logs and aborts when recovery fails`() { assertFailsWith { fixture @@ -452,7 +452,7 @@ class JsonObjectReaderTest { ) } - @Test(timeout = 1000L) + @Test fun `nextMapOrNull logs and aborts when recovery fails`() { assertFailsWith { fixture @@ -468,7 +468,7 @@ class JsonObjectReaderTest { ) } - @Test(timeout = 1000L) + @Test fun `nextMapOfListOrNull logs and aborts when recovery fails`() { assertFailsWith { fixture @@ -484,7 +484,7 @@ class JsonObjectReaderTest { ) } - @Test(timeout = 1000L) + @Test fun `nextUnknown logs when recovery fails`() { val unknown = mutableMapOf() val reader = fixture.getSut("{\"key\": {\"value\": \"fail\"") diff --git a/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt b/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt index 06270ffcced..932c98f4105 100644 --- a/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt +++ b/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt @@ -174,7 +174,7 @@ class MapObjectReaderTest { reader.endObject() } - @Test(timeout = 1000L) + @Test fun `nextListOrNull skips a failing element`() { val actual = getValuesReader(listOf("fail")).nextListOrNull(logger, BasicSerializable.Deserializer()) @@ -182,7 +182,7 @@ class MapObjectReaderTest { assertEquals(emptyList(), actual) } - @Test(timeout = 1000L) + @Test fun `nextListOrNull keeps elements before a failing element`() { val actual = getValuesReader(listOf(serializableValue("one"), "fail")) @@ -191,7 +191,7 @@ class MapObjectReaderTest { assertEquals(listOf(BasicSerializable("one")), actual) } - @Test(timeout = 1000L) + @Test fun `nextListOrNull keeps elements after a failing element`() { val actual = getValuesReader(listOf("fail", serializableValue("two"))) @@ -200,7 +200,7 @@ class MapObjectReaderTest { assertEquals(listOf(BasicSerializable("two")), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOrNull skips a failing value`() { val actual = getValuesReader(linkedMapOf("bad" to "fail")) @@ -209,7 +209,7 @@ class MapObjectReaderTest { assertEquals(emptyMap(), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOrNull keeps values before a failing value`() { val actual = getValuesReader(linkedMapOf("bad" to "fail", "good" to serializableValue("one"))) @@ -218,7 +218,7 @@ class MapObjectReaderTest { assertEquals(mapOf("good" to BasicSerializable("one")), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOrNull keeps values after a failing value`() { val actual = getValuesReader(linkedMapOf("good" to serializableValue("two"), "bad" to "fail")) @@ -227,7 +227,7 @@ class MapObjectReaderTest { assertEquals(mapOf("good" to BasicSerializable("two")), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOfListOrNull skips a failing value`() { val actual = getValuesReader(linkedMapOf("bad" to serializableValue("fail"))) @@ -236,7 +236,7 @@ class MapObjectReaderTest { assertEquals(emptyMap(), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOfListOrNull keeps values before a failing value`() { val actual = getValuesReader( @@ -250,7 +250,7 @@ class MapObjectReaderTest { assertEquals(mapOf("good" to listOf(BasicSerializable("one"))), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOfListOrNull keeps values after a failing value`() { val actual = getValuesReader( @@ -264,7 +264,7 @@ class MapObjectReaderTest { assertEquals(mapOf("good" to listOf(BasicSerializable("two"))), actual) } - @Test(timeout = 1000L) + @Test fun `nextListOrNull keeps elements after a partially consumed failing element`() { val actual = getValuesReader( @@ -275,7 +275,7 @@ class MapObjectReaderTest { assertEquals(listOf(BasicSerializable("two")), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOrNull keeps values after a partially consumed failing value`() { val actual = getValuesReader( @@ -289,7 +289,7 @@ class MapObjectReaderTest { assertEquals(mapOf("good" to BasicSerializable("two")), actual) } - @Test(timeout = 1000L) + @Test fun `nextMapOfListOrNull keeps values after a partially consumed failing element`() { val actual = getValuesReader( From 886612d370aacaf1303f1d783066a7c2b8ef474d Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 16 Apr 2026 13:10:09 +0200 Subject: [PATCH 13/17] test(sentry): Rename mismatched MapObjectReader recovery tests Rename the partially consumed MapObjectReader recovery tests so their names match the map iteration order. MapObjectReader reads map entries in reverse insertion order from its stack. The previous names described the surviving values as coming after the failure even though they are read before it. Co-Authored-By: Claude --- sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt b/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt index 932c98f4105..40e6a47cf08 100644 --- a/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt +++ b/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt @@ -276,7 +276,7 @@ class MapObjectReaderTest { } @Test - fun `nextMapOrNull keeps values after a partially consumed failing value`() { + fun `nextMapOrNull keeps values before a partially consumed failing value`() { val actual = getValuesReader( linkedMapOf( @@ -290,7 +290,7 @@ class MapObjectReaderTest { } @Test - fun `nextMapOfListOrNull keeps values after a partially consumed failing element`() { + fun `nextMapOfListOrNull keeps values before a partially consumed failing element`() { val actual = getValuesReader( linkedMapOf( From 1bcb316302e5a9da0cf6b34e8eaa064c6b280272 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 16 Apr 2026 15:02:45 +0200 Subject: [PATCH 14/17] fix(sentry): Mark skipped values consumed after success Move skipValue consumption tracking after the underlying Gson skip succeeds. This keeps valueConsumed aligned with the primitive reader methods and avoids marking a value as consumed before JsonReader has actually advanced past it. Co-Authored-By: Claude --- sentry/src/main/java/io/sentry/JsonObjectReader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/JsonObjectReader.java b/sentry/src/main/java/io/sentry/JsonObjectReader.java index 176227bba93..0d0c0e72ec6 100644 --- a/sentry/src/main/java/io/sentry/JsonObjectReader.java +++ b/sentry/src/main/java/io/sentry/JsonObjectReader.java @@ -349,8 +349,8 @@ public void setLenient(boolean lenient) { @Override public void skipValue() throws IOException { - markValueConsumed(); jsonReader.skipValue(); + markValueConsumed(); } private boolean recoverAfterValueFailure( From 27f2d213f0975973c3de3c6d82ac57474876f5c3 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 16 Apr 2026 15:33:06 +0200 Subject: [PATCH 15/17] fix(sentry): Handle empty collections in MapObjectReader Gate the recovery loops on the current token instead of hasNext when reading maps and lists. MapObjectReader keeps END_OBJECT and END_ARRAY sentinels on its stack, so hasNext stays true for empty collections. Checking the token preserves empty map and empty list handling after moving nextName outside the inner recovery try-catch, and the new tests cover those cases. Co-Authored-By: Claude --- .../java/io/sentry/util/MapObjectReader.java | 6 ++--- .../io/sentry/util/MapObjectReaderTest.kt | 25 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/sentry/src/main/java/io/sentry/util/MapObjectReader.java b/sentry/src/main/java/io/sentry/util/MapObjectReader.java index ab7afb54b6b..6eca9e14875 100644 --- a/sentry/src/main/java/io/sentry/util/MapObjectReader.java +++ b/sentry/src/main/java/io/sentry/util/MapObjectReader.java @@ -52,7 +52,7 @@ public List nextListOrNull( try { beginArray(); List list = new ArrayList<>(); - if (hasNext()) { + if (peek() != JsonToken.END_ARRAY) { do { final int stackSizeBefore = stack.size(); try { @@ -82,7 +82,7 @@ public Map nextMapOrNull( try { beginObject(); Map map = new HashMap<>(); - if (hasNext()) { + if (peek() == JsonToken.NAME) { do { final String key = nextName(); final int stackSizeBefore = stack.size(); @@ -112,7 +112,7 @@ public Map nextMapOrNull( try { beginObject(); - if (hasNext()) { + if (peek() == JsonToken.NAME) { do { final @NotNull String key = nextName(); final int stackSizeBefore = stack.size(); diff --git a/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt b/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt index 40e6a47cf08..757a0bf5e61 100644 --- a/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt +++ b/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt @@ -174,6 +174,13 @@ class MapObjectReaderTest { reader.endObject() } + @Test + fun `nextListOrNull returns empty list for empty list`() { + val actual = getValuesReader(emptyList()).nextListOrNull(logger, BasicSerializable.Deserializer()) + + assertEquals(emptyList(), actual) + } + @Test fun `nextListOrNull skips a failing element`() { val actual = @@ -200,6 +207,15 @@ class MapObjectReaderTest { assertEquals(listOf(BasicSerializable("two")), actual) } + @Test + fun `nextMapOrNull returns empty map for empty map`() { + val actual = + getValuesReader(linkedMapOf()) + .nextMapOrNull(logger, BasicSerializable.Deserializer()) + + assertEquals(emptyMap(), actual) + } + @Test fun `nextMapOrNull skips a failing value`() { val actual = @@ -227,6 +243,15 @@ class MapObjectReaderTest { assertEquals(mapOf("good" to BasicSerializable("two")), actual) } + @Test + fun `nextMapOfListOrNull returns empty map for empty map`() { + val actual = + getValuesReader(linkedMapOf>()) + .nextMapOfListOrNull(logger, BasicSerializable.Deserializer()) + + assertEquals(emptyMap(), actual) + } + @Test fun `nextMapOfListOrNull skips a failing value`() { val actual = From 37e9595295d6630c39cc8c0ca6bdc48786439e9a Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 16 Apr 2026 15:41:31 +0200 Subject: [PATCH 16/17] fix(sentry): Preserve list recovery in MapObjectReader Continue list recovery until the END_ARRAY sentinel instead of stopping at the next non-object token. A failed object element could be followed by an invalid primitive and then a valid object. The old loop stopped at the primitive, letting endArray pop the wrong stack entry and leaving the reader in an inconsistent state. The updated loop keeps recovering until the array is fully consumed, and the new tests cover empty collections and mixed invalid list contents. Co-Authored-By: Claude --- .../java/io/sentry/util/MapObjectReader.java | 18 ++++++++--------- .../io/sentry/util/MapObjectReaderTest.kt | 20 ++++++++++++++++++- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/sentry/src/main/java/io/sentry/util/MapObjectReader.java b/sentry/src/main/java/io/sentry/util/MapObjectReader.java index 6eca9e14875..edd91190c82 100644 --- a/sentry/src/main/java/io/sentry/util/MapObjectReader.java +++ b/sentry/src/main/java/io/sentry/util/MapObjectReader.java @@ -52,16 +52,14 @@ public List nextListOrNull( try { beginArray(); List list = new ArrayList<>(); - if (peek() != JsonToken.END_ARRAY) { - do { - final int stackSizeBefore = stack.size(); - try { - list.add(deserializer.deserialize(this, logger)); - } catch (Exception e) { - logger.log(SentryLevel.WARNING, "Failed to deserialize object in list.", e); - recoverValue(stackSizeBefore); - } - } while (peek() == JsonToken.BEGIN_OBJECT); + while (peek() != JsonToken.END_ARRAY) { + final int stackSizeBefore = stack.size(); + try { + list.add(deserializer.deserialize(this, logger)); + } catch (Exception e) { + logger.log(SentryLevel.WARNING, "Failed to deserialize object in list.", e); + recoverValue(stackSizeBefore); + } } endArray(); return list; diff --git a/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt b/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt index 757a0bf5e61..e7ad9bc61e3 100644 --- a/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt +++ b/sentry/src/test/java/io/sentry/util/MapObjectReaderTest.kt @@ -176,7 +176,8 @@ class MapObjectReaderTest { @Test fun `nextListOrNull returns empty list for empty list`() { - val actual = getValuesReader(emptyList()).nextListOrNull(logger, BasicSerializable.Deserializer()) + val actual = + getValuesReader(emptyList()).nextListOrNull(logger, BasicSerializable.Deserializer()) assertEquals(emptyList(), actual) } @@ -207,6 +208,23 @@ class MapObjectReaderTest { assertEquals(listOf(BasicSerializable("two")), actual) } + @Test + fun `nextListOrNull keeps elements after a failing object followed by a primitive`() { + val reader = + getValuesReader( + listOf( + partialSerializableValue("fail", "bad"), + "oops", + partialSerializableValue("ok", "two"), + ) + ) + + val actual = reader.nextListOrNull(logger, partiallyFailingDeserializer) + + assertEquals(listOf(BasicSerializable("two")), actual) + assertEquals(JsonToken.END_OBJECT, reader.peek()) + } + @Test fun `nextMapOrNull returns empty map for empty map`() { val actual = From 0f38c5fefa9157749cb7e9e2d40904dbcd7b70a4 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 16 Apr 2026 16:13:55 +0200 Subject: [PATCH 17/17] fix(sentry): Continue list recovery until array end Continue JsonObjectReader list deserialization until the array boundary instead of stopping when the next token is not BEGIN_OBJECT. After recovering from a failed object element, the next unread token can be a primitive. The previous loop exited early in that case and then aborted when endArray() encountered unread content. Iterating with hasNext() keeps recovery aligned with the actual array boundary. Add a regression test covering a failing object followed by a primitive and a later valid object. Fixes GH-5278 Co-Authored-By: Claude --- .../main/java/io/sentry/JsonObjectReader.java | 32 +++++++++---------- .../java/io/sentry/JsonObjectReaderTest.kt | 25 +++++++++++++++ 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/sentry/src/main/java/io/sentry/JsonObjectReader.java b/sentry/src/main/java/io/sentry/JsonObjectReader.java index 0d0c0e72ec6..fef9bfc71ff 100644 --- a/sentry/src/main/java/io/sentry/JsonObjectReader.java +++ b/sentry/src/main/java/io/sentry/JsonObjectReader.java @@ -120,24 +120,22 @@ public void nextUnknown(ILogger logger, Map unknown, String name } beginArray(); List list = new ArrayList<>(); - if (jsonReader.hasNext()) { - do { - final RecoveryState recoveryState = beginRecovery(peek()); - try { - list.add(deserializer.deserialize(this, logger)); - } catch (Exception e) { - if (!recoverAfterValueFailure( - logger, - e, - "Failed to deserialize object in list.", - "Stream unrecoverable, aborting list deserialization.", - recoveryState)) { - break; - } - } finally { - endRecovery(recoveryState); + while (jsonReader.hasNext()) { + final RecoveryState recoveryState = beginRecovery(peek()); + try { + list.add(deserializer.deserialize(this, logger)); + } catch (Exception e) { + if (!recoverAfterValueFailure( + logger, + e, + "Failed to deserialize object in list.", + "Stream unrecoverable, aborting list deserialization.", + recoveryState)) { + break; } - } while (jsonReader.peek() == JsonToken.BEGIN_OBJECT); + } finally { + endRecovery(recoveryState); + } } endArray(); return list; diff --git a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt index 71a4ad4f059..d4fd525a87b 100644 --- a/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt +++ b/sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt @@ -298,6 +298,31 @@ class JsonObjectReaderTest { assertEquals(listOf("two"), actual) } + @Test + fun `nextListOrNull keeps elements after a failing object followed by a primitive`() { + val reader = + getValuesReader( + "[{\"kind\": \"fail\", \"value\": \"bad\"}, \"oops\", {\"kind\": \"ok\", \"value\": \"two\"}]" + ) + val deserializer = + JsonDeserializer { objectReader, _ -> + objectReader.beginObject() + objectReader.nextName() + if (objectReader.nextString() == "fail") { + throw IllegalStateException("intentional") + } + objectReader.nextName() + val value = objectReader.nextString() + objectReader.endObject() + value + } + + val actual = reader.nextListOrNull(fixture.logger, deserializer) + + assertEquals(listOf("two"), actual) + assertEquals(io.sentry.vendor.gson.stream.JsonToken.END_OBJECT, reader.peek()) + } + @Test fun `nextListOrNull keeps elements after skipValue consumes a failing element`() { var callCount = 0