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

Cannot deserialize value of type java.math.BigDecimal from String "3." (not a valid representation) #4577

Closed
1 task done
dmelisso opened this issue Jun 12, 2024 · 11 comments
Labels
2.17 Issues planned at earliest for 2.17
Milestone

Comments

@dmelisso
Copy link

Search before asking

  • I searched in the issues and found nothing similar.

Describe the bug

This bug probably heavily relates to #4435 which was previously fixed using FasterXML/jackson-core#1241

Since jackson 2.17 while deserializing BigDecimals (or integers) from JsonInput with a string value of the format ".f" e.g. {"value":"3."} we get an exception that looks like the following:

com.fasterxml.jackson.databind.exc.InvalidFormatException: Cannot deserialize value of type `java.math.BigDecimal` from String "5.": not a valid representation
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 10] (through reference chain: TestDTO["value"])

	at com.fasterxml.jackson.databind.exc.InvalidFormatException.from(InvalidFormatException.java:67)
	at com.fasterxml.jackson.databind.DeserializationContext.weirdStringException(DeserializationContext.java:1958)
	at com.fasterxml.jackson.databind.DeserializationContext.handleWeirdStringValue(DeserializationContext.java:1245)
	at com.fasterxml.jackson.databind.deser.std.NumberDeserializers$BigDecimalDeserializer.deserialize(NumberDeserializers.java:1058)
	at com.fasterxml.jackson.databind.deser.std.NumberDeserializers$BigDecimalDeserializer.deserialize(NumberDeserializers.java:990)
	at com.fasterxml.jackson.databind.deser.impl.MethodProperty.deserializeAndSet(MethodProperty.java:129)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.vanillaDeserialize(BeanDeserializer.java:310)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:177)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:342)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4905)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3848)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3816)
	at BigDecimalTest.shouldDeserializeBigDecimalFromJacksonDataBind(BigDecimalTest.java:26)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

I am not sure if this deserialization behavior change is intended or not. However, both Java and javascript appear to understand "5." as a number.

Version Information

2.17 and 2.17.1

Reproduction

The reproduction steps are equivalent to the steps described in #4435

i.e., in https://github.com/EAlf91/jackson-issue simply switch the test case from ".05" to "5."

Expected behavior

If this change in behavior is not intended, I would expect jackson to parse "3." as a decimal, (i.e., 3.0), or as an integer (i.e., 3)

Additional context

See the related issue #4435 that describes a similar bug that was previously fixed

Without knowing the jackson codebase, I would expect that the allowed pattern for https://github.com/pjfanning/jackson-core/blob/1c60872851893a8994bfa7131edcb13e2e67e4fe/src/main/java/com/fasterxml/jackson/core/io/NumberInput.java#L41 should be modified to also allow float number definitions which do not have numerals after the period.

@dmelisso dmelisso added the to-evaluate Issue that has been received but not yet evaluated label Jun 12, 2024
@JooHyukKim
Copy link
Member

Makes sense, the fix would most likely be in Jackson-core module tho.

@pjfanning
Copy link
Member

I think we should consider removing looksLikeValidNumber. The check is going to get so complicated or so lax that it just acts as performance drain with little benefit.

@cowtowncoder
Copy link
Member

No, not a bug unless "3." is a valid JSON number -- which I don't think it is. Java or Javascript rules are not the driver, JSON specification. This is different from ".05" which is valid JSON number representation

In some cases from-String conversions may allow some invalid numbers: this is unfortunate, but solution is not to allow more invalid numbers.

As to looksLikeValidNumber(), if I remember its existence has to do with maximum number length constraint; idea being that it catches most invalid number cases before attempt to parse. It is only used for "from-String" cases: native numbers are decoded by JsonParser itself.

@cowtowncoder
Copy link
Member

Checked https://www.json.org/json-en.html: number ending with . is not a valid JSON Number. Closing.

@pjfanning
Copy link
Member

@cowtowncoder Is it possible that this worked in older versions of Jackson - eg v2.16?
If it used to work, maybe we should add a Feature that allows users to choose to accept '3.' as valid again.

@cowtowncoder
Copy link
Member

It is possible, but I don't think there has been intention to support non-JSON numbers.
It is regrettable if such numbers were not failing in the past but I don't want to go on making point changes since there is no good definition of what would be acceptable.

For this particular case I would recommend implementing custom BigDecimal deserializer.

@dmelisso
Copy link
Author

Thank you all of the amazingly quick response. I agree it's not part of JSON standard and it complexifies implementation that it would not make sense to add this feature.

I have one last question though about #4577 and https://www.json.org/json-en.html

If I read the diagram for "number" correctly in https://www.json.org/json-en.html then numbers like ".5" should also not be permitted. However, FasterXML/jackson-core#1241 seems to have re-added the feature.

I am not sure if you truly intended to allow ".5" back into jackson, if you want to stay strict with the JSON specification standard.

@cowtowncoder
Copy link
Member

cowtowncoder commented Jun 13, 2024

Thank you for checking @dmelisso . I could have sworn that .5 was valid but if not, yes, that seems like something that ideally would not also not be supported. But then again blocking that such values in 2.x probably will affect someone (break someone's handling), similar to change discussed here (unintentional).

Given this information, I think what would make sense is doing 2 things:

  1. For 2.(17?), allow 3. again (change validation)
  2. For 3.0, dis-allow both .5 and .3.

I will re-open this issue, file new one for (2)

@cowtowncoder cowtowncoder reopened this Jun 13, 2024
@cowtowncoder cowtowncoder added 2.17 Issues planned at earliest for 2.17 and removed to-evaluate Issue that has been received but not yet evaluated labels Jun 13, 2024
@cowtowncoder
Copy link
Member

Hmmmh. Ok, re-thinking this one again: while for JSON processing this might make sense, I realize that other Jackson backends -- like XML and YAML -- have different number representations.
And while this is fine for "native" numbers (values that are numbers within data formats) -- because there is specific YAMLParser or FromXmlParser to consider format-specific rules, same is NOT true for "stringified" numbers, because parser cannot do validation (needed coercion/conversion happens at high level). And instead, format-agnostic handling (with JsonSerializer implemnetation) is applied.

What this means is that for Stringified numbers we have no way to adapt to format-specific rules: and I think that -- just as an example -- both .3 and 3. are actually valid YAML numbers; and may well be (but I'm less sure about this) be legal XML (schema) numbers as well.
If so, using stricter, JSON-based representation rules makes no sense with those backends.

So: while there is no quick solution here (other than adding support for such coercion via JsonParsre, something we could consider), it seems sensible to actually allow more lenient handling for Stringified numbers when decoded by databind, from native String value.
Sort of superset of allowed representations.

I realize that this won't make it easier for users/developers to deduce rules, but I think it probably the most pragmatic thing to do.
And at least for now apply this to 2.x as well as 3.0, not changing logic until we have something better to offer.

@cowtowncoder
Copy link
Member

To be fixed via FasterXML/jackson-core#1308

@cowtowncoder cowtowncoder added this to the 2.172. milestone Jun 13, 2024
@cowtowncoder cowtowncoder changed the title Cannot deserialize value of type java.math.BigDecimal from String "3." from String "3.": not a valid representation Cannot deserialize value of type java.math.BigDecimal from String "3." (not a valid representation) Jun 13, 2024
@cowtowncoder
Copy link
Member

This is now fixed for Jackson 2.17; to be released as 2.17.2 (and 2.18.0).

Actually fix is in jackson-core; databind has tests to verify functioning.

LuciferYang pushed a commit to apache/spark that referenced this issue Jul 9, 2024
### What changes were proposed in this pull request?

This PR amis to upgrade `fasterxml.jackson` from 2.17.1 to 2.17.2.

### Why are the changes needed?

There are some bug fixes about [Databind](https://github.com/FasterXML/jackson-databind):
[#4561](FasterXML/jackson-databind#4561): Issues using jackson-databind 2.17.1 with Reactor (wrt DeserializerCache and ReentrantLock)
[#4575](FasterXML/jackson-databind#4575): StdDelegatingSerializer does not consider a Converter that may return null for a non-null input
[#4577](FasterXML/jackson-databind#4577): Cannot deserialize value of type java.math.BigDecimal from String "3." (not a valid representation)
[#4595](FasterXML/jackson-databind#4595): No way to explicitly disable wrapping in custom annotation processor
[#4607](FasterXML/jackson-databind#4607): MismatchedInput: No Object Id found for an instance of X to assign to property 'id'
[#4610](FasterXML/jackson-databind#4610): DeserializationFeature.FAIL_ON_UNRESOLVED_OBJECT_IDS does not work when used with Polymorphic type handling

The full release note of 2.17.2:
https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.17.2

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #47241 from wayneguow/upgrade_jackson.

Authored-by: Wei Guo <guow93@gmail.com>
Signed-off-by: yangjie01 <yangjie01@baidu.com>
ericm-db pushed a commit to ericm-db/spark that referenced this issue Jul 10, 2024
### What changes were proposed in this pull request?

This PR amis to upgrade `fasterxml.jackson` from 2.17.1 to 2.17.2.

### Why are the changes needed?

There are some bug fixes about [Databind](https://github.com/FasterXML/jackson-databind):
[apache#4561](FasterXML/jackson-databind#4561): Issues using jackson-databind 2.17.1 with Reactor (wrt DeserializerCache and ReentrantLock)
[apache#4575](FasterXML/jackson-databind#4575): StdDelegatingSerializer does not consider a Converter that may return null for a non-null input
[apache#4577](FasterXML/jackson-databind#4577): Cannot deserialize value of type java.math.BigDecimal from String "3." (not a valid representation)
[apache#4595](FasterXML/jackson-databind#4595): No way to explicitly disable wrapping in custom annotation processor
[apache#4607](FasterXML/jackson-databind#4607): MismatchedInput: No Object Id found for an instance of X to assign to property 'id'
[apache#4610](FasterXML/jackson-databind#4610): DeserializationFeature.FAIL_ON_UNRESOLVED_OBJECT_IDS does not work when used with Polymorphic type handling

The full release note of 2.17.2:
https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.17.2

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#47241 from wayneguow/upgrade_jackson.

Authored-by: Wei Guo <guow93@gmail.com>
Signed-off-by: yangjie01 <yangjie01@baidu.com>
jingz-db pushed a commit to jingz-db/spark that referenced this issue Jul 22, 2024
### What changes were proposed in this pull request?

This PR amis to upgrade `fasterxml.jackson` from 2.17.1 to 2.17.2.

### Why are the changes needed?

There are some bug fixes about [Databind](https://github.com/FasterXML/jackson-databind):
[apache#4561](FasterXML/jackson-databind#4561): Issues using jackson-databind 2.17.1 with Reactor (wrt DeserializerCache and ReentrantLock)
[apache#4575](FasterXML/jackson-databind#4575): StdDelegatingSerializer does not consider a Converter that may return null for a non-null input
[apache#4577](FasterXML/jackson-databind#4577): Cannot deserialize value of type java.math.BigDecimal from String "3." (not a valid representation)
[apache#4595](FasterXML/jackson-databind#4595): No way to explicitly disable wrapping in custom annotation processor
[apache#4607](FasterXML/jackson-databind#4607): MismatchedInput: No Object Id found for an instance of X to assign to property 'id'
[apache#4610](FasterXML/jackson-databind#4610): DeserializationFeature.FAIL_ON_UNRESOLVED_OBJECT_IDS does not work when used with Polymorphic type handling

The full release note of 2.17.2:
https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.17.2

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#47241 from wayneguow/upgrade_jackson.

Authored-by: Wei Guo <guow93@gmail.com>
Signed-off-by: yangjie01 <yangjie01@baidu.com>
attilapiros pushed a commit to attilapiros/spark that referenced this issue Oct 4, 2024
### What changes were proposed in this pull request?

This PR amis to upgrade `fasterxml.jackson` from 2.17.1 to 2.17.2.

### Why are the changes needed?

There are some bug fixes about [Databind](https://github.com/FasterXML/jackson-databind):
[apache#4561](FasterXML/jackson-databind#4561): Issues using jackson-databind 2.17.1 with Reactor (wrt DeserializerCache and ReentrantLock)
[apache#4575](FasterXML/jackson-databind#4575): StdDelegatingSerializer does not consider a Converter that may return null for a non-null input
[apache#4577](FasterXML/jackson-databind#4577): Cannot deserialize value of type java.math.BigDecimal from String "3." (not a valid representation)
[apache#4595](FasterXML/jackson-databind#4595): No way to explicitly disable wrapping in custom annotation processor
[apache#4607](FasterXML/jackson-databind#4607): MismatchedInput: No Object Id found for an instance of X to assign to property 'id'
[apache#4610](FasterXML/jackson-databind#4610): DeserializationFeature.FAIL_ON_UNRESOLVED_OBJECT_IDS does not work when used with Polymorphic type handling

The full release note of 2.17.2:
https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.17.2

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#47241 from wayneguow/upgrade_jackson.

Authored-by: Wei Guo <guow93@gmail.com>
Signed-off-by: yangjie01 <yangjie01@baidu.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.17 Issues planned at earliest for 2.17
Projects
None yet
Development

No branches or pull requests

4 participants