Skip to content

Commit d6f8299

Browse files
adinauerclaude
adinauer
and
claude
committed
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 <noreply@anthropic.com>
1 parent 98fb625 commit d6f8299

2 files changed

Lines changed: 113 additions & 27 deletions

File tree

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

Lines changed: 74 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44
import io.sentry.vendor.gson.stream.JsonToken;
55
import java.io.IOException;
66
import java.io.Reader;
7+
import java.util.ArrayDeque;
78
import java.util.ArrayList;
89
import java.util.Date;
10+
import java.util.Deque;
911
import java.util.HashMap;
1012
import java.util.List;
1113
import java.util.Map;
@@ -18,6 +20,7 @@
1820
public final class JsonObjectReader implements ObjectReader {
1921

2022
private final @NotNull JsonReader jsonReader;
23+
private final @NotNull Deque<RecoveryState> recoveryStates = new ArrayDeque<>();
2124
private int depth = 0;
2225

2326
public JsonObjectReader(Reader in) {
@@ -85,18 +88,21 @@ public float nextFloat() throws IOException {
8588

8689
@Override
8790
public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name) {
88-
final int startDepth = depth;
89-
JsonToken startToken = JsonToken.END_DOCUMENT;
91+
RecoveryState recoveryState = null;
9092
try {
91-
startToken = peek();
93+
recoveryState = beginRecovery(peek());
9294
unknown.put(name, nextObjectOrNull());
9395
} catch (Exception exception) {
9496
logger.log(SentryLevel.ERROR, exception, "Error deserializing unknown key: %s", name);
95-
try {
96-
recoverValue(startDepth, startToken);
97-
} catch (Exception ignored) {
98-
// stream is unrecoverable
97+
if (recoveryState != null) {
98+
try {
99+
recoverValue(recoveryState);
100+
} catch (Exception ignored) {
101+
// stream is unrecoverable
102+
}
99103
}
104+
} finally {
105+
endRecovery(recoveryState);
100106
}
101107
}
102108

@@ -111,8 +117,7 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
111117
List<T> list = new ArrayList<>();
112118
if (jsonReader.hasNext()) {
113119
do {
114-
final int startDepth = depth;
115-
final JsonToken startToken = peek();
120+
final RecoveryState recoveryState = beginRecovery(peek());
116121
try {
117122
list.add(deserializer.deserialize(this, logger));
118123
} catch (Exception e) {
@@ -121,10 +126,11 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
121126
e,
122127
"Failed to deserialize object in list.",
123128
"Stream unrecoverable, aborting list deserialization.",
124-
startDepth,
125-
startToken)) {
129+
recoveryState)) {
126130
break;
127131
}
132+
} finally {
133+
endRecovery(recoveryState);
128134
}
129135
} while (jsonReader.peek() == JsonToken.BEGIN_OBJECT);
130136
}
@@ -144,8 +150,7 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
144150
if (jsonReader.hasNext()) {
145151
do {
146152
final String key = jsonReader.nextName();
147-
final int startDepth = depth;
148-
final JsonToken startToken = peek();
153+
final RecoveryState recoveryState = beginRecovery(peek());
149154
try {
150155
map.put(key, deserializer.deserialize(this, logger));
151156
} catch (Exception e) {
@@ -154,10 +159,11 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
154159
e,
155160
"Failed to deserialize object in map.",
156161
"Stream unrecoverable, aborting map deserialization.",
157-
startDepth,
158-
startToken)) {
162+
recoveryState)) {
159163
break;
160164
}
165+
} finally {
166+
endRecovery(recoveryState);
161167
}
162168
} while (jsonReader.peek() == JsonToken.BEGIN_OBJECT || jsonReader.peek() == JsonToken.NAME);
163169
}
@@ -180,8 +186,7 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
180186
if (hasNext()) {
181187
do {
182188
final @NotNull String key = nextName();
183-
final int startDepth = depth;
184-
final JsonToken startToken = peek();
189+
final RecoveryState recoveryState = beginRecovery(peek());
185190
try {
186191
final @Nullable List<T> list = nextListOrNull(logger, deserializer);
187192
if (list != null) {
@@ -193,10 +198,11 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
193198
e,
194199
"Failed to deserialize list in map.",
195200
"Stream unrecoverable, aborting map-of-lists deserialization.",
196-
startDepth,
197-
startToken)) {
201+
recoveryState)) {
198202
break;
199203
}
204+
} finally {
205+
endRecovery(recoveryState);
200206
}
201207
} while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME);
202208
}
@@ -263,6 +269,7 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
263269
public void beginObject() throws IOException {
264270
jsonReader.beginObject();
265271
depth++;
272+
markContainerEntered();
266273
}
267274

268275
@Override
@@ -275,6 +282,7 @@ public void endObject() throws IOException {
275282
public void beginArray() throws IOException {
276283
jsonReader.beginArray();
277284
depth++;
285+
markContainerEntered();
278286
}
279287

280288
@Override
@@ -333,22 +341,46 @@ private boolean recoverAfterValueFailure(
333341
final @NotNull Exception error,
334342
final @NotNull String warningMessage,
335343
final @NotNull String unrecoverableMessage,
336-
final int startDepth,
337-
final @NotNull JsonToken startToken) {
344+
final @NotNull RecoveryState recoveryState) {
338345
logger.log(SentryLevel.WARNING, warningMessage, error);
339346
try {
340-
recoverValue(startDepth, startToken);
347+
recoverValue(recoveryState);
341348
return true;
342349
} catch (Exception recoveryException) {
343350
logger.log(SentryLevel.ERROR, unrecoverableMessage, recoveryException);
344351
return false;
345352
}
346353
}
347354

348-
private void recoverValue(final int startDepth, final @NotNull JsonToken startToken)
349-
throws IOException {
350-
final boolean enteredNestedValue = depth > startDepth;
351-
while (depth > startDepth) {
355+
private @NotNull RecoveryState beginRecovery(final @NotNull JsonToken startToken) {
356+
final RecoveryState recoveryState = new RecoveryState(depth, startToken);
357+
recoveryStates.addLast(recoveryState);
358+
return recoveryState;
359+
}
360+
361+
private void endRecovery(final @Nullable RecoveryState recoveryState) {
362+
if (recoveryState == null) {
363+
return;
364+
}
365+
if (!recoveryStates.isEmpty() && recoveryStates.peekLast() == recoveryState) {
366+
recoveryStates.removeLast();
367+
} else {
368+
recoveryStates.remove(recoveryState);
369+
}
370+
}
371+
372+
private void markContainerEntered() {
373+
for (RecoveryState recoveryState : recoveryStates) {
374+
if (!recoveryState.enteredContainer
375+
&& isContainerStartToken(recoveryState.startToken)
376+
&& depth > recoveryState.startDepth) {
377+
recoveryState.enteredContainer = true;
378+
}
379+
}
380+
}
381+
382+
private void recoverValue(final @NotNull RecoveryState recoveryState) throws IOException {
383+
while (depth > recoveryState.startDepth) {
352384
final JsonToken token = peek();
353385
if (token == JsonToken.END_OBJECT) {
354386
endObject();
@@ -359,13 +391,28 @@ private void recoverValue(final int startDepth, final @NotNull JsonToken startTo
359391
}
360392
}
361393

362-
if (!enteredNestedValue && peek() == startToken) {
394+
if (!recoveryState.enteredContainer && peek() == recoveryState.startToken) {
363395
skipValue();
364396
}
365397
}
366398

399+
private static boolean isContainerStartToken(final @NotNull JsonToken token) {
400+
return token == JsonToken.BEGIN_OBJECT || token == JsonToken.BEGIN_ARRAY;
401+
}
402+
367403
@Override
368404
public void close() throws IOException {
369405
jsonReader.close();
370406
}
407+
408+
private static final class RecoveryState {
409+
private final int startDepth;
410+
private final @NotNull JsonToken startToken;
411+
private boolean enteredContainer;
412+
413+
private RecoveryState(final int startDepth, final @NotNull JsonToken startToken) {
414+
this.startDepth = startDepth;
415+
this.startToken = startToken;
416+
}
417+
}
371418
}

‎sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,18 @@ class JsonObjectReaderTest {
3232
value
3333
}
3434

35+
private val postParseThrowingValueDeserializer =
36+
JsonDeserializer<String> { reader, _ ->
37+
reader.beginObject()
38+
reader.nextName()
39+
val value = reader.nextString()
40+
reader.endObject()
41+
if (value == "fail") {
42+
throw IllegalStateException("intentional")
43+
}
44+
value
45+
}
46+
3547
private fun getValuesReader(jsonValue: String): JsonObjectReader =
3648
fixture.getSut("{\"values\": $jsonValue}").apply {
3749
beginObject()
@@ -227,6 +239,24 @@ class JsonObjectReaderTest {
227239
assertEquals(emptyList(), actual)
228240
}
229241

242+
@Test(timeout = 1000L)
243+
fun `nextListOrNull skips an unconsumed failing element`() {
244+
var callCount = 0
245+
val deserializer =
246+
JsonDeserializer<String> { reader, logger ->
247+
if (callCount++ == 0) {
248+
throw IllegalStateException("intentional")
249+
}
250+
throwingValueDeserializer.deserialize(reader, logger)
251+
}
252+
253+
val actual =
254+
getValuesReader("[{\"value\": \"ignored\"}, {\"value\": \"two\"}]")
255+
.nextListOrNull(fixture.logger, deserializer)
256+
257+
assertEquals(listOf("two"), actual)
258+
}
259+
230260
@Test(timeout = 1000L)
231261
fun `nextListOrNull keeps elements before a failing element`() {
232262
val actual =
@@ -245,6 +275,15 @@ class JsonObjectReaderTest {
245275
assertEquals(listOf("two"), actual)
246276
}
247277

278+
@Test(timeout = 1000L)
279+
fun `nextListOrNull keeps elements after a fully consumed failing element`() {
280+
val actual =
281+
getValuesReader("[{\"value\": \"fail\"}, {\"value\": \"two\"}]")
282+
.nextListOrNull(fixture.logger, postParseThrowingValueDeserializer)
283+
284+
assertEquals(listOf("two"), actual)
285+
}
286+
248287
@Test(timeout = 1000L)
249288
fun `nextMapOrNull skips a failing value`() {
250289
val actual =

0 commit comments

Comments
 (0)