Skip to content

Commit 8173842

Browse files
etspacemanadwsingh
authored andcommitted
Fix NPE in JsonCodec.deserializeShape that suppresses deserialization errors (#1202)
When deserializeShape() encounters invalid input (e.g., a nested struct with missing required fields), the builder throws a SerializationException mid-parse. The finally block then calls close(), which: 1. Calls parser.nextToken() (returns non-null due to unconsumed tokens) 2. Calls parser.close() then sets parser = null 3. Calls describeToken() which dereferences the now-null parser -> NPE This NPE suppresses the original helpful exception (e.g., "Missing required members: [field]"). Fix: - In JacksonJsonDeserializer.close(), capture the token description before nulling the parser reference. - In JsonCodec.deserializeShape(), use closeQuietly() on the error path so that exceptions from close() don't suppress the original deserialization/validation exception.
1 parent 7805bd1 commit 8173842

3 files changed

Lines changed: 40 additions & 8 deletions

File tree

codecs/json-codec/src/main/java/software/amazon/smithy/java/json/JsonCodec.java

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,20 +56,36 @@ public ShapeSerializer createSerializer(OutputStream sink) {
5656
@Override
5757
public <T extends SerializableShape> T deserializeShape(byte[] source, ShapeBuilder<T> builder) {
5858
var deserializer = createDeserializer(source);
59+
T result;
5960
try {
60-
return builder.deserialize(deserializer).errorCorrection().build();
61-
} finally {
62-
deserializer.close();
61+
result = builder.deserialize(deserializer).errorCorrection().build();
62+
} catch (Exception e) {
63+
closeQuietly(deserializer);
64+
throw e;
6365
}
66+
deserializer.close();
67+
return result;
6468
}
6569

6670
@Override
6771
public <T extends SerializableShape> T deserializeShape(ByteBuffer source, ShapeBuilder<T> builder) {
6872
var deserializer = createDeserializer(source);
73+
T result;
74+
try {
75+
result = builder.deserialize(deserializer).errorCorrection().build();
76+
} catch (Exception e) {
77+
closeQuietly(deserializer);
78+
throw e;
79+
}
80+
deserializer.close();
81+
return result;
82+
}
83+
84+
private static void closeQuietly(ShapeDeserializer deserializer) {
6985
try {
70-
return builder.deserialize(deserializer).errorCorrection().build();
71-
} finally {
7286
deserializer.close();
87+
} catch (Exception ignored) {
88+
// Don't suppress the original deserialization exception.
7389
}
7490
}
7591

codecs/json-codec/src/main/java/software/amazon/smithy/java/json/jackson/JacksonJsonDeserializer.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,18 @@ final class JacksonJsonDeserializer implements ShapeDeserializer {
5151
public void close() {
5252
if (parser != null && !parser.isClosed()) {
5353
try {
54-
// Close the parser, but also ensure there's no trailing garbage input.
5554
var nextToken = parser.nextToken();
55+
// Capture token description before nulling parser to avoid NPE in describeToken().
56+
var tokenDesc = nextToken != null ? JsonToken.valueDescFor(nextToken) : null;
5657
parser.close();
5758
parser = null;
58-
if (nextToken != null) {
59-
throw new SerializationException("Unexpected JSON content: " + describeToken());
59+
if (tokenDesc != null) {
60+
throw new SerializationException("Unexpected JSON content: " + tokenDesc);
6061
}
6162
} catch (SerializationException e) {
6263
throw e;
6364
} catch (Exception e) {
65+
parser = null;
6466
throw new SerializationException(e);
6567
}
6668
}

codecs/json-codec/src/test/java/software/amazon/smithy/java/json/GeneratedModelSerdeTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,4 +745,18 @@ static Stream<Arguments> invalidInputs() {
745745
}
746746
return arguments.stream();
747747
}
748+
749+
@PerProvider
750+
void deserializeShapeWithNestedMissingRequiredFieldReportsFieldName(JsonSerdeProvider provider) {
751+
var codec = codec(provider);
752+
// NestedStruct requires field1 (String) and field2 (Integer).
753+
// Omitting field2 from the nested struct inside a list should produce
754+
// a SerializationException mentioning the missing field, not an NPE from close().
755+
byte[] json =
756+
"{\"id\":\"x\",\"count\":1,\"enabled\":true,\"ratio\":1.0,\"score\":1.0,\"bigCount\":1,\"structList\":[{\"field1\":\"hello\"}]}"
757+
.getBytes(StandardCharsets.UTF_8);
758+
var e = assertThrows(SerializationException.class,
759+
() -> codec.deserializeShape(json, ComplexStruct.builder()));
760+
assertThat(e.getMessage()).contains("field2");
761+
}
748762
}

0 commit comments

Comments
 (0)