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

BigDecimalAsStringSerializer in NumberSerializer throws IllegalStateException in 2.10 #2513

Closed
johanhaleby opened this issue Oct 19, 2019 · 6 comments
Milestone

Comments

@johanhaleby
Copy link

Hi,

I just updated from Jackson (databind) 2.9.7 to 2.10.0 as a part of updating from Spring Boot 2.1.x to 2.2. In my project I depend on jackson-dataformat-hal to generate responses to web requests. After the update to Jackson 2.10 I get the following exception when trying to serialize a field of type BigDecimal:

Caused by: java.lang.IllegalStateException: null
	at com.fasterxml.jackson.databind.ser.std.NumberSerializer$BigDecimalAsStringSerializer.valueToString(NumberSerializer.java:161)
	at com.fasterxml.jackson.databind.ser.std.NumberSerializer$BigDecimalAsStringSerializer.isEmpty(NumberSerializer.java:132)
	at com.fasterxml.jackson.databind.ser.BeanPropertyWriter.serializeAsField(BeanPropertyWriter.java:711)
	at io.openapitools.jackson.dataformat.hal.ser.HALBeanSerializer$FilteredProperties.serialize(HALBeanSerializer.java:191)
	... 120 common frames omitted

If I have a look at com.fasterxml.jackson.databind.ser.std.NumberSerializer$BigDecimalAsStringSerializer.valueToStringit looks like this:

@Override
public String valueToString(Object value) {
    // should never be called
    throw new IllegalStateException();
}

but this method seems to be called from com.fasterxml.jackson.databind.ser.std.NumberSerializer$BigDecimalAsStringSerializer.isEmpty(NumberSerializer.java:132) just as the exception implies:

@Override
public boolean isEmpty(SerializerProvider prov, Object value) {
    return valueToString(value).isEmpty();
}

The call to BeanPropertyWriter.serializeAsField(..) in jackson-dataformat-hal looks like this:

for (BeanPropertyWriter prop : state) {
    try {
        prop.serializeAsField(bean, jgen, provider);
    } catch (Exception e) {
        wrapAndThrow(provider, e, bean, prop.getName());
    }
}

where jgen is a JsonGenerator and provider is a SerializerProvider from Jackson 2.10.

Could this be a bug in Jackson Databind or is there something else going on?

@cowtowncoder
Copy link
Member

It could be a bug certainly. It would be great to have a reproduction through a real use case (that is, ideally just serializing a bean with BigDecimal field, but with settings that trigger the problem). From isEmpty() call I am guessing this could be related to filtering (via @JsonIncluder or global settings), so it may be easy to reproduce, but obviously existing test suite does not trigger this behavior or problem would have been caught.

@cowtowncoder
Copy link
Member

cowtowncoder commented Oct 20, 2019

And yes, that call looks... very odd. I do not know how this sequence of calls came to be.
But change was for #2230.

On test case: looks like this would only be triggered if BigDecimal is configured to be written as JSON String?

@cowtowncoder cowtowncoder added the need-test-case To work on issue, a reproduction (ideally unit test) needed label Oct 20, 2019
@cowtowncoder
Copy link
Member

I think test could be added to com.fasterxml.jackson.databind.ser.jdk.NumberSerTest, as alternatively configured case of "testNumbersAsString()".

@cowtowncoder cowtowncoder removed the need-test-case To work on issue, a reproduction (ideally unit test) needed label Oct 20, 2019
@cowtowncoder
Copy link
Member

Was able to reproduce, think I can fix it quite easily.

@johanhaleby
Copy link
Author

johanhaleby commented Oct 20, 2019 via email

@cowtowncoder cowtowncoder added this to the 2.10.1 milestone Oct 20, 2019
@cowtowncoder
Copy link
Member

@johanhaleby I think the only thing would be if you can easily either use 2.10.1-SNAPSHOT or build locally from 2.10 branch, to verify that latest jackson-databind works. I think fix should work as it is quite straight-forward but it is always best if submitter can verify with original setup.

@cowtowncoder cowtowncoder removed the 2.10 label Apr 12, 2020
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

No branches or pull requests

2 participants