(javax-money) Fail cleanly on non-Object input instead of throwing NullPointerException - #91
Open
pjfanning wants to merge 2 commits into
Open
(javax-money) Fail cleanly on non-Object input instead of throwing NullPointerException#91pjfanning wants to merge 2 commits into
NullPointerException#91pjfanning wants to merge 2 commits into
Conversation
`MonetaryAmountDeserializer.deserialize()` assumes it is positioned on a
START_OBJECT and loops with:
while (parser.nextToken() != JsonToken.END_OBJECT) {
final String field = parser.currentName();
...
if (field.equals(names.getAmount())) {
For any non-Object input `currentName()` returns `null`, so a bare
`NullPointerException: Cannot invoke "String.equals(Object)" because "field"
is null` escapes to the caller rather than a `JacksonException`. Inputs `12`,
`"abc"` and `[1,2]` all reproduce it, and `moneta`'s `FastMoney`/`Money`/
`RoundedMoney` deserializers share this class.
Verify the token up front (as joda-money's `MoneyDeserializer` already does),
and only loop while positioned on a PROPERTY_NAME so malformed content cannot
fall into the same trap. Also drop a duplicated import pair.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Problem
MonetaryAmountDeserializer.deserialize()assumes it is positioned on aSTART_OBJECT:For any non-Object input,
currentName()returnsnulland the very next line dereferences it. A bare NPE escapes to the caller instead of aJacksonException:MonetaryAmount12NullPointerException: Cannot invoke "String.equals(Object)" because "field" is null"abc"[1,2]moneta'sFastMoney/Money/RoundedMoneydeserializers share this class, so they are affected too.Fix
Check the token up front — the same thing joda-money's
MoneyDeserializeralready does — and only loop while actually positioned on aPROPERTY_NAME, so malformed content cannot fall into the same trap:Also removes a duplicated
import javax.money.CurrencyUnit; import javax.money.MonetaryAmount;pair at the top of the file.Tests
New
FailOnNonObjectTest(7 cases): number/string/array/empty-array input, a bad element inside aList<MonetaryAmount>, plus checks that valid Objects and explicitnullstill deserialize.javax-money142 tests andmoneta137 tests all green.