Skip to content

Commit 1f8edab

Browse files
adinauerclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 5ab0155 commit 1f8edab

2 files changed

Lines changed: 77 additions & 3 deletions

File tree

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

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,15 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
117117
list.add(deserializer.deserialize(this, logger));
118118
} catch (Exception e) {
119119
logger.log(SentryLevel.WARNING, "Failed to deserialize object in list.", e);
120-
recoverValue(startDepth, startToken);
120+
try {
121+
recoverValue(startDepth, startToken);
122+
} catch (Exception recoveryException) {
123+
logger.log(
124+
SentryLevel.ERROR,
125+
"Stream unrecoverable, aborting list deserialization.",
126+
recoveryException);
127+
break;
128+
}
121129
}
122130
} while (jsonReader.peek() == JsonToken.BEGIN_OBJECT);
123131
}
@@ -143,7 +151,15 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
143151
map.put(key, deserializer.deserialize(this, logger));
144152
} catch (Exception e) {
145153
logger.log(SentryLevel.WARNING, "Failed to deserialize object in map.", e);
146-
recoverValue(startDepth, startToken);
154+
try {
155+
recoverValue(startDepth, startToken);
156+
} catch (Exception recoveryException) {
157+
logger.log(
158+
SentryLevel.ERROR,
159+
"Stream unrecoverable, aborting map deserialization.",
160+
recoveryException);
161+
break;
162+
}
147163
}
148164
} while (jsonReader.peek() == JsonToken.BEGIN_OBJECT || jsonReader.peek() == JsonToken.NAME);
149165
}
@@ -175,7 +191,15 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
175191
}
176192
} catch (Exception e) {
177193
logger.log(SentryLevel.WARNING, "Failed to deserialize list in map.", e);
178-
recoverValue(startDepth, startToken);
194+
try {
195+
recoverValue(startDepth, startToken);
196+
} catch (Exception recoveryException) {
197+
logger.log(
198+
SentryLevel.ERROR,
199+
"Stream unrecoverable, aborting map-of-lists deserialization.",
200+
recoveryException);
201+
break;
202+
}
179203
}
180204
} while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME);
181205
}

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@ package io.sentry
22

33
import java.io.StringReader
44
import kotlin.test.assertEquals
5+
import kotlin.test.assertFailsWith
56
import kotlin.test.assertNull
67
import org.junit.Test
78
import org.mockito.kotlin.any
9+
import org.mockito.kotlin.eq
810
import org.mockito.kotlin.mock
911
import org.mockito.kotlin.never
1012
import org.mockito.kotlin.verify
@@ -297,6 +299,54 @@ class JsonObjectReaderTest {
297299
assertEquals(mapOf("good" to listOf("two")), actual)
298300
}
299301

302+
@Test(timeout = 1000L)
303+
fun `nextListOrNull logs and aborts when recovery fails`() {
304+
assertFailsWith<Exception> {
305+
fixture
306+
.getSut("[{\"value\": \"fail\"")
307+
.nextListOrNull(fixture.logger, throwingValueDeserializer)
308+
}
309+
310+
verify(fixture.logger)
311+
.log(
312+
eq(SentryLevel.ERROR),
313+
eq("Stream unrecoverable, aborting list deserialization."),
314+
any<Throwable>(),
315+
)
316+
}
317+
318+
@Test(timeout = 1000L)
319+
fun `nextMapOrNull logs and aborts when recovery fails`() {
320+
assertFailsWith<Exception> {
321+
fixture
322+
.getSut("{\"bad\": {\"value\": \"fail\"")
323+
.nextMapOrNull(fixture.logger, throwingValueDeserializer)
324+
}
325+
326+
verify(fixture.logger)
327+
.log(
328+
eq(SentryLevel.ERROR),
329+
eq("Stream unrecoverable, aborting map deserialization."),
330+
any<Throwable>(),
331+
)
332+
}
333+
334+
@Test(timeout = 1000L)
335+
fun `nextMapOfListOrNull logs and aborts when recovery fails`() {
336+
assertFailsWith<Exception> {
337+
fixture
338+
.getSut("{\"bad\": [{\"value\": \"fail\"")
339+
.nextMapOfListOrNull(fixture.logger, throwingValueDeserializer)
340+
}
341+
342+
verify(fixture.logger)
343+
.log(
344+
eq(SentryLevel.ERROR),
345+
eq("Stream unrecoverable, aborting map-of-lists deserialization."),
346+
any<Throwable>(),
347+
)
348+
}
349+
300350
// nextDateOrNull
301351

302352
@Test

0 commit comments

Comments
 (0)