Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JacksonJsonIO may throw exception when trying to read a number #2054

Closed
Azquelt opened this issue Nov 4, 2024 · 1 comment · Fixed by #2057
Closed

JacksonJsonIO may throw exception when trying to read a number #2054

Azquelt opened this issue Nov 4, 2024 · 1 comment · Fixed by #2057

Comments

@Azquelt
Copy link
Contributor

Azquelt commented Nov 4, 2024

JacksonJsonIO.getJsonBigDecimal does not check that the node contains a number before trying to parse it as a big decimal.

This results in a NumberFormatException rather than a null as the Javadoc would suggest.

Admittedly, the Javadoc was written after the implementation, so arguably it's the Javadoc that's wrong here, but having this method return a null would be consistent with the other methods in this class.

This can result in throwing an exception when parsing an OpenAPI 3.0 schema rather than ignoring the invalid field.

Ideally we should unit test the JsonIO implementations. There may be other similar bugs.

@MikeEdgar
Copy link
Member

+1

Currently it supports both text and numeric JSON values. I wonder if that it correct or whether it should be limited to only numeric values. That can easily be tested with JsonNode#isNumber together with JsonNode#decimalValue, but allowing text would require a bit more involved check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants