Fix NPE in JsonCodec.deserializeShape that suppresses deserialization errors#1203
Merged
adwsingh merged 1 commit intoMay 22, 2026
Merged
Conversation
ed9e262 to
09850be
Compare
Contributor
Author
|
Whoops - forgot spotless. Updated. |
… errors (smithy-lang#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.
09850be to
9d0ddb1
Compare
Contributor
|
Thanks for the fix @etspaceman . Did you get a chance to take the new SmithyJson serde for a spin? Its opt in right now and we are looking for some real world feedback. Its much faster than Jackson see #1148 You can opt in by setting the system property |
adwsingh
approved these changes
May 22, 2026
Contributor
Author
|
No I haven't! Id love to give it a try. Will report back when I do. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
Fixes #1202
Description of changes:
When
deserializeShape()encounters invalid input (e.g., a nested struct in a list with missing required fields), the builder throws aSerializationExceptionmid-parse. Thefinallyblock inJsonCodec.deserializeShape()then callsclose()on the deserializer, which triggers an NPE that suppresses the original helpful exception.The NPE occurs in
JacksonJsonDeserializer.close()because:parser.nextToken()returns non-null (unconsumed tokens remain after the aborted parse)parser.close()is called, thenparser = nulldescribeToken()is called, which dereferences the now-nullparser→ NPEFix:
JacksonJsonDeserializer.close(), capture the token description before nulling the parser reference.JsonCodec.deserializeShape(), usecloseQuietly()on the error path so exceptions fromclose()don't suppress the original deserialization/validation exception.Test: Added
deserializeShapeWithNestedMissingRequiredFieldReportsFieldNamewhich fails without this fix (NPE suppresses the "Missing required members" exception) and passes with it.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.