Skip to content

Commit 5ab0155

Browse files
adinauerclaude
adinauer
and
claude
committed
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 <noreply@anthropic.com>
1 parent cd0981b commit 5ab0155

4 files changed

Lines changed: 278 additions & 17 deletions

File tree

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

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
public final class JsonObjectReader implements ObjectReader {
1919

2020
private final @NotNull JsonReader jsonReader;
21+
private int depth = 0;
2122

2223
public JsonObjectReader(Reader in) {
2324
this.jsonReader = new JsonReader(in);
@@ -84,10 +85,18 @@ public float nextFloat() throws IOException {
8485

8586
@Override
8687
public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name) {
88+
final int startDepth = depth;
89+
JsonToken startToken = JsonToken.END_DOCUMENT;
8790
try {
91+
startToken = peek();
8892
unknown.put(name, nextObjectOrNull());
8993
} catch (Exception exception) {
9094
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
99+
}
91100
}
92101
}
93102

@@ -98,18 +107,21 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
98107
jsonReader.nextNull();
99108
return null;
100109
}
101-
jsonReader.beginArray();
110+
beginArray();
102111
List<T> list = new ArrayList<>();
103112
if (jsonReader.hasNext()) {
104113
do {
114+
final int startDepth = depth;
115+
final JsonToken startToken = peek();
105116
try {
106117
list.add(deserializer.deserialize(this, logger));
107118
} catch (Exception e) {
108119
logger.log(SentryLevel.WARNING, "Failed to deserialize object in list.", e);
120+
recoverValue(startDepth, startToken);
109121
}
110122
} while (jsonReader.peek() == JsonToken.BEGIN_OBJECT);
111123
}
112-
jsonReader.endArray();
124+
endArray();
113125
return list;
114126
}
115127

@@ -120,20 +132,23 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
120132
jsonReader.nextNull();
121133
return null;
122134
}
123-
jsonReader.beginObject();
135+
beginObject();
124136
Map<String, T> map = new HashMap<>();
125137
if (jsonReader.hasNext()) {
126138
do {
139+
final String key = jsonReader.nextName();
140+
final int startDepth = depth;
141+
final JsonToken startToken = peek();
127142
try {
128-
String key = jsonReader.nextName();
129143
map.put(key, deserializer.deserialize(this, logger));
130144
} catch (Exception e) {
131145
logger.log(SentryLevel.WARNING, "Failed to deserialize object in map.", e);
146+
recoverValue(startDepth, startToken);
132147
}
133148
} while (jsonReader.peek() == JsonToken.BEGIN_OBJECT || jsonReader.peek() == JsonToken.NAME);
134149
}
135150

136-
jsonReader.endObject();
151+
endObject();
137152
return map;
138153
}
139154

@@ -151,9 +166,16 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
151166
if (hasNext()) {
152167
do {
153168
final @NotNull String key = nextName();
154-
final @Nullable List<T> list = nextListOrNull(logger, deserializer);
155-
if (list != null) {
156-
result.put(key, list);
169+
final int startDepth = depth;
170+
final JsonToken startToken = peek();
171+
try {
172+
final @Nullable List<T> list = nextListOrNull(logger, deserializer);
173+
if (list != null) {
174+
result.put(key, list);
175+
}
176+
} catch (Exception e) {
177+
logger.log(SentryLevel.WARNING, "Failed to deserialize list in map.", e);
178+
recoverValue(startDepth, startToken);
157179
}
158180
} while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME);
159181
}
@@ -219,21 +241,25 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
219241
@Override
220242
public void beginObject() throws IOException {
221243
jsonReader.beginObject();
244+
depth++;
222245
}
223246

224247
@Override
225248
public void endObject() throws IOException {
226249
jsonReader.endObject();
250+
depth--;
227251
}
228252

229253
@Override
230254
public void beginArray() throws IOException {
231255
jsonReader.beginArray();
256+
depth++;
232257
}
233258

234259
@Override
235260
public void endArray() throws IOException {
236261
jsonReader.endArray();
262+
depth--;
237263
}
238264

239265
@Override
@@ -281,6 +307,25 @@ public void skipValue() throws IOException {
281307
jsonReader.skipValue();
282308
}
283309

310+
private void recoverValue(final int startDepth, final @NotNull JsonToken startToken)
311+
throws IOException {
312+
final boolean enteredNestedValue = depth > startDepth;
313+
while (depth > startDepth) {
314+
final JsonToken token = peek();
315+
if (token == JsonToken.END_OBJECT) {
316+
endObject();
317+
} else if (token == JsonToken.END_ARRAY) {
318+
endArray();
319+
} else {
320+
skipValue();
321+
}
322+
}
323+
324+
if (!enteredNestedValue && peek() == startToken) {
325+
skipValue();
326+
}
327+
}
328+
284329
@Override
285330
public void close() throws IOException {
286331
jsonReader.close();

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

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ public void nextUnknown(
3535
unknown.put(name, nextObjectOrNull());
3636
} catch (Exception exception) {
3737
logger.log(SentryLevel.ERROR, exception, "Error deserializing unknown key: %s", name);
38+
try {
39+
skipValue();
40+
} catch (Exception ignored) {
41+
// stream is unrecoverable
42+
}
3843
}
3944
}
4045

@@ -56,6 +61,7 @@ public <T> List<T> nextListOrNull(
5661
list.add(deserializer.deserialize(this, logger));
5762
} catch (Exception e) {
5863
logger.log(SentryLevel.WARNING, "Failed to deserialize object in list.", e);
64+
skipValue();
5965
}
6066
} while (peek() == JsonToken.BEGIN_OBJECT);
6167
}
@@ -80,11 +86,12 @@ public <T> Map<String, T> nextMapOrNull(
8086
Map<String, T> map = new HashMap<>();
8187
if (hasNext()) {
8288
do {
89+
final String key = nextName();
8390
try {
84-
String key = nextName();
8591
map.put(key, deserializer.deserialize(this, logger));
8692
} catch (Exception e) {
8793
logger.log(SentryLevel.WARNING, "Failed to deserialize object in map.", e);
94+
skipValue();
8895
}
8996
} while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME);
9097
}
@@ -109,9 +116,14 @@ public <T> Map<String, T> nextMapOrNull(
109116
if (hasNext()) {
110117
do {
111118
final @NotNull String key = nextName();
112-
final @Nullable List<T> list = nextListOrNull(logger, deserializer);
113-
if (list != null) {
114-
result.put(key, list);
119+
try {
120+
final @Nullable List<T> list = nextListOrNull(logger, deserializer);
121+
if (list != null) {
122+
result.put(key, list);
123+
}
124+
} catch (Exception e) {
125+
logger.log(SentryLevel.WARNING, "Failed to deserialize list in map.", e);
126+
skipValue();
115127
}
116128
} while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME);
117129
}
@@ -197,12 +209,13 @@ public String nextName() throws IOException {
197209

198210
@Override
199211
public void beginObject() throws IOException {
200-
final Map.Entry<String, Object> currentEntry = stack.removeLast();
212+
final Map.Entry<String, Object> currentEntry = stack.peekLast();
201213
if (currentEntry == null) {
202214
throw new IOException("No more entries");
203215
}
204216
final Object value = currentEntry.getValue();
205217
if (value instanceof Map) {
218+
stack.removeLast();
206219
// insert a dummy entry to indicate end of an object
207220
stack.addLast(new AbstractMap.SimpleEntry<>(null, JsonToken.END_OBJECT));
208221
// extract map entries onto the stack
@@ -223,12 +236,13 @@ public void endObject() throws IOException {
223236

224237
@Override
225238
public void beginArray() throws IOException {
226-
final Map.Entry<String, Object> currentEntry = stack.removeLast();
239+
final Map.Entry<String, Object> currentEntry = stack.peekLast();
227240
if (currentEntry == null) {
228241
throw new IOException("No more entries");
229242
}
230243
final Object value = currentEntry.getValue();
231244
if (value instanceof List) {
245+
stack.removeLast();
232246
// insert a dummy entry to indicate end of an object
233247
stack.addLast(new AbstractMap.SimpleEntry<>(null, JsonToken.END_ARRAY));
234248
// extract map entries onto the stack
@@ -377,7 +391,11 @@ public void nextNull() throws IOException {
377391
public void setLenient(final boolean lenient) {}
378392

379393
@Override
380-
public void skipValue() throws IOException {}
394+
public void skipValue() throws IOException {
395+
if (!stack.isEmpty()) {
396+
stack.removeLast();
397+
}
398+
}
381399

382400
@SuppressWarnings("TypeParameterUnusedInFormals")
383401
@Nullable

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,24 @@ class JsonObjectReaderTest {
1818

1919
val fixture = Fixture()
2020

21+
private val throwingValueDeserializer =
22+
JsonDeserializer<String> { reader, _ ->
23+
reader.beginObject()
24+
reader.nextName()
25+
val value = reader.nextString()
26+
if (value == "fail") {
27+
throw IllegalStateException("intentional")
28+
}
29+
reader.endObject()
30+
value
31+
}
32+
33+
private fun getValuesReader(jsonValue: String): JsonObjectReader =
34+
fixture.getSut("{\"values\": $jsonValue}").apply {
35+
beginObject()
36+
nextName()
37+
}
38+
2139
// nextStringOrNull
2240

2341
@Test
@@ -198,6 +216,87 @@ class JsonObjectReaderTest {
198216
verify(fixture.logger, never()).log(any(), any(), any<Throwable>())
199217
}
200218

219+
@Test(timeout = 1000L)
220+
fun `nextListOrNull skips a failing element`() {
221+
val actual =
222+
getValuesReader("[{\"value\": \"fail\"}]")
223+
.nextListOrNull(fixture.logger, throwingValueDeserializer)
224+
225+
assertEquals(emptyList(), actual)
226+
}
227+
228+
@Test(timeout = 1000L)
229+
fun `nextListOrNull keeps elements before a failing element`() {
230+
val actual =
231+
getValuesReader("[{\"value\": \"one\"}, {\"value\": \"fail\"}]")
232+
.nextListOrNull(fixture.logger, throwingValueDeserializer)
233+
234+
assertEquals(listOf("one"), actual)
235+
}
236+
237+
@Test(timeout = 1000L)
238+
fun `nextListOrNull keeps elements after a failing element`() {
239+
val actual =
240+
getValuesReader("[{\"value\": \"fail\"}, {\"value\": \"two\"}]")
241+
.nextListOrNull(fixture.logger, throwingValueDeserializer)
242+
243+
assertEquals(listOf("two"), actual)
244+
}
245+
246+
@Test(timeout = 1000L)
247+
fun `nextMapOrNull skips a failing value`() {
248+
val actual =
249+
getValuesReader("{\"bad\": {\"value\": \"fail\"}}")
250+
.nextMapOrNull(fixture.logger, throwingValueDeserializer)
251+
252+
assertEquals(emptyMap(), actual)
253+
}
254+
255+
@Test(timeout = 1000L)
256+
fun `nextMapOrNull keeps values before a failing value`() {
257+
val actual =
258+
getValuesReader("{\"good\": {\"value\": \"one\"}, \"bad\": {\"value\": \"fail\"}}")
259+
.nextMapOrNull(fixture.logger, throwingValueDeserializer)
260+
261+
assertEquals(mapOf("good" to "one"), actual)
262+
}
263+
264+
@Test(timeout = 1000L)
265+
fun `nextMapOrNull keeps values after a failing value`() {
266+
val actual =
267+
getValuesReader("{\"bad\": {\"value\": \"fail\"}, \"good\": {\"value\": \"two\"}}")
268+
.nextMapOrNull(fixture.logger, throwingValueDeserializer)
269+
270+
assertEquals(mapOf("good" to "two"), actual)
271+
}
272+
273+
@Test(timeout = 1000L)
274+
fun `nextMapOfListOrNull skips a failing value`() {
275+
val actual =
276+
getValuesReader("{\"bad\": {\"value\": \"fail\"}}")
277+
.nextMapOfListOrNull(fixture.logger, throwingValueDeserializer)
278+
279+
assertEquals(emptyMap(), actual)
280+
}
281+
282+
@Test(timeout = 1000L)
283+
fun `nextMapOfListOrNull keeps values before a failing value`() {
284+
val actual =
285+
getValuesReader("{\"good\": [{\"value\": \"one\"}], \"bad\": {\"value\": \"fail\"}}")
286+
.nextMapOfListOrNull(fixture.logger, throwingValueDeserializer)
287+
288+
assertEquals(mapOf("good" to listOf("one")), actual)
289+
}
290+
291+
@Test(timeout = 1000L)
292+
fun `nextMapOfListOrNull keeps values after a failing value`() {
293+
val actual =
294+
getValuesReader("{\"bad\": {\"value\": \"fail\"}, \"good\": [{\"value\": \"two\"}]}")
295+
.nextMapOfListOrNull(fixture.logger, throwingValueDeserializer)
296+
297+
assertEquals(mapOf("good" to listOf("two")), actual)
298+
}
299+
201300
// nextDateOrNull
202301

203302
@Test

0 commit comments

Comments
 (0)