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

StdDeserializer coerces ints to floats even if configured to fail #3503

Closed
Tomasito665 opened this issue Jun 1, 2022 · 3 comments
Closed
Milestone

Comments

@Tomasito665
Copy link
Contributor

Describe the bug
Coercion configuration makes it possible to configure int-to-float coercions to fail. The StdDeserializer class, however, coerces floats to ints regardless of the coercion config. In fact, the _parseFloatPrimitive method makes no distinction between ints and floats.

case JsonTokenId.ID_NUMBER_INT:
case JsonTokenId.ID_NUMBER_FLOAT:
return p.getFloatValue();

Version information
2.13.2

To Reproduce

package my.package;

import static org.junit.jupiter.api.Assertions.assertThrows;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.cfg.CoercionAction;
import com.fasterxml.jackson.databind.cfg.CoercionInputShape;
import com.fasterxml.jackson.databind.exc.MismatchedInputException;
import com.fasterxml.jackson.databind.type.LogicalType;
import org.junit.jupiter.api.Test;

class MyClass {
    float foo;

    void setFoo(float foo) {
        this.foo = foo;
    }
}

public class IntToFloatCoercionTest {
    @Test
    void intToFloatCoercion_shouldFailWhenSetToFail() throws JsonProcessingException {
        var mapper = new ObjectMapper();
        mapper.coercionConfigFor(LogicalType.Float).setCoercion(CoercionInputShape.Integer, CoercionAction.Fail);

        mapper.readValue("{\"foo\": 11}", MyType.class);

        assertThrows(MismatchedInputException.class, () -> mapper.readValue(
                "{\"foo\": 11}",
                MyClass.class
        ));
    }
}

The test fails.

org.opentest4j.AssertionFailedError: Expected com.fasterxml.jackson.databind.exc.MismatchedInputException to be thrown, but nothing was thrown.

Expected behavior
As specified in the unit test, I would expect readValue to throw some type of MismatchedInputException exception.

Additional context

@Tomasito665 Tomasito665 added the to-evaluate Issue that has been received but not yet evaluated label Jun 1, 2022
@cowtowncoder
Copy link
Member

Hmmh. Interesting question -- does "coercion" mean forceful explicit change that is not allowed by default like in Java -- in which case Integral numbers can become Floating-point ones since there is no loss of accuracy (... except for possible overflow?) -- or should it also include this case?

So, the reason this is not checked is as per above: whereas conversion from integer number to floating point is likely dangerous, the reverse typical is not.

But I think it is not unreasonable to expect one could consider this a coercion too, something that can be prevented.

So if anyone has time and interest to work on a PR to add support, I'd be happy to help
(or if I happen to have time & this gets highly voted, eventually I might work on it myself)

@cowtowncoder cowtowncoder added 2.14 and removed to-evaluate Issue that has been received but not yet evaluated labels Jun 1, 2022
Tomasito665 added a commit to Tomasito665/jackson-databind that referenced this issue Jun 3, 2022
@Tomasito665
Copy link
Contributor Author

Friday evening seemed like the perfect moment to work on some open-source and give this a shot. I sent in a PR. Let me know what you think.

@cowtowncoder
Copy link
Member

Thank you! I really appreciate this & hope to look into it soon.
Right now I have quite a few things going on but I try to give priority to helping with PRs.
So hopefully can check it out some time this week.

Tomasito665 added a commit to Tomasito665/jackson-databind that referenced this issue Jun 30, 2022
Tomasito665 added a commit to Tomasito665/jackson-databind that referenced this issue Jun 30, 2022
@cowtowncoder cowtowncoder added this to the 2.14.0 milestone Jul 1, 2022
@cowtowncoder cowtowncoder changed the title StdDeserializer coerces ints to floats even if configured to fail StdDeserializer coerces ints to floats even if configured to fail Jul 1, 2022
cowtowncoder added a commit that referenced this issue Jul 1, 2022
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