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 <noreply@anthropic.com>

Author: adinauer

sentry/src/main/java/io/sentry/JsonObjectReader.java

@@ -30,60 +30,61 @@ 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();
   }
 
   @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<String, Object> unknown, String name
   public <T> @Nullable List<T> nextListOrNull(
       @NotNull ILogger logger, @NotNull JsonDeserializer<T> deserializer) throws IOException {
     if (jsonReader.peek() == JsonToken.NULL) {
-      jsonReader.nextNull();
+      nextNull();
       return null;
     }
     beginArray();
@@ -142,7 +143,7 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
   public <T> @Nullable Map<String, T> nextMapOrNull(
       @NotNull ILogger logger, @NotNull JsonDeserializer<T> deserializer) throws IOException {
     if (jsonReader.peek() == JsonToken.NULL) {
-      jsonReader.nextNull();
+      nextNull();
       return null;
     }
     beginObject();
@@ -215,7 +216,7 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
   public <T> @Nullable T nextOrNull(
       @NotNull ILogger logger, @NotNull JsonDeserializer<T> 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<String, Object> 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<String, Object> 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;

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<String> { 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<String> { 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<Exception> {