Jackson can't handle underscores in numbers

[smedelyan]

Not sure I've chosen the right repository to report the issue, but there's a similar one: #65

I can't see a "dataformats-text" explicit dependency so here's how I use it:

compile 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.9.9'
compile 'com.fasterxml.jackson.core:jackson-databind:2.9.9.3'

Below are tests proving it fails to parse numbers with underscores.

    private static class LongHolder {
        private Long v;

        public Long getV() {
            return v;
        }

        public void setV(Long v) {
            this.v = v;
        }
    }

    private static class IntHolder {
        private Integer v;

        public Integer getV() {
            return v;
        }

        public void setV(Integer v) {
            this.v = v;
        }
    }

    @Test
    public void testYamlUnderscores() throws IOException {
        ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
        mapper.readValue("v: 1000000", IntHolder.class); // ok
        mapper.readValue("v: 1_000_000", IntHolder.class); // ok
        mapper.readValue("v: 1000000", LongHolder.class); // ok

        // throws: com.fasterxml.jackson.databind.JsonMappingException: For input string: "1_000_000"
        mapper.readValue("v: 1_000_000", LongHolder.class);
    }

    @Test
    public void testJsonUnderscores() throws IOException {
        ObjectMapper mapper = new ObjectMapper();
        mapper.readValue("{\"v\": \"1000000\"}", IntHolder.class); // ok

        // throws:
        // com.fasterxml.jackson.databind.exc.InvalidFormatException: Cannot deserialize value of type `java
        // .lang.Integer` from String "1_000_000": not a valid Integer value
        // at [Source: (String)"{"v": "1_000_000"}"; line: 1, column: 7]
        mapper.readValue("{\"v\": \"1_000_000\"}", IntHolder.class);

        mapper.readValue("{\"v\": \"1000000\"}", LongHolder.class); // ok

        // throws:
        // com.fasterxml.jackson.databind.exc.InvalidFormatException: Cannot deserialize value of type `java
        // .lang.Long` from String "1_000_000": not a valid Long value
        // at [Source: (String)"{"v": "1_000_000"}"; line: 1, column: 7]
        mapper.readValue("{\"v\": \"1_000_000\"}", LongHolder.class);
    }

[cowtowncoder]

Yes, this is correct place: thank you reporting the issue!

[cowtowncoder]

Adding label easy in hopes of getting new contributors to help.

[chennakesava]

@cowtowncoder can you assign this to me?

[cowtowncoder]

@chennakesava Done!

A good first step might be to do PR to add a failing test under

yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/failing/

(failing is used to filter out these tests so they don't block successful build, but can be manually run)

After that, YAMLParser is the class that handles parsing: it uses SnakeYAML for decoding, and then inspects Events to determine JsonToken to expose. I don't remember details but I think there's a regexp to check for valid numbers, so it might be as simple as changing that.

[cowtowncoder]

One more note: if possible, it'd be great to make the change against 2.10 branch: but I can cherry-pick from master too (backport into 2.10). The reason 2.10 is important is that this allows releasing fix quicker (as next patch will be 2.10.1, which will be out sooner than 2.11.0 minor release).

[conor-ward]

Hi,
Just wondering if this issue will be fixed in the next release of jackson-dataformats-text. Currently have the same issue and wondering if it will be fixed.

Thanks

[cowtowncoder]

@conor-ward I don't know if @chennakesava is working on this, but as you can see there is no work linking to this issue yet so nothing committed so far.

[conor-ward]

Thanks @cowtowncoder. I believe I have a fix for the issue in YamlParser on the 2.10 branch which passes the testYamlUnderscores test above. Do I need to be assigned to an issue in order to be able to add code to the repo?

[cowtowncoder]

@conor-ward No, doing a Pull Request is the usual way, but I will assign it to you just as marker for others.

[cowtowncoder]

Fixed for 2.10.5, but looks like snakeyaml-engine (to be used for 3.0) does NOT recognize underscores on numbers, for some reason.

@asomov I wonder if you know whether YAML 1.2 handling makes this:

v: 1_000_000

be tagged as String value; org.snakeyaml tags it as int.

[cowtowncoder]

Looks like this might be change between YAML 1.1 and 1.2:

symfony/symfony#27120

if so, could remove some code from 3.0 as well.
Will need to document so users will not be super-surprised.

[conor-ward]

Fixed for 2.10.5, but looks like snakeyaml-engine (to be used for 3.0) does NOT recognize underscores on numbers, for some reason.

@asomov I wonder if you know whether YAML 1.2 handling makes this:

v: 1_000_000

be tagged as String value; org.snakeyaml tags it as int.

Thanks for the help @cowtowncoder, just wondering when 2.10.5 is likely to be released?

[asomov]

YAML 1.2 is closer to JSON and _ is not a number. But there is a usecase where underscore can be accepted inside integers.
Why would we need it ? Would it lead to confusion ? (JSON does not have it)

[cowtowncoder]

@asomov I am not arguing for keeping it (although I can see both some pros and some cons), but mostly wondering about compatibility part, documentation. While Jackson could, if need be, add optional extra coercions, I think I would prefer sticking to default specification defined behavior.

[asomov]

YAML 1.2 has added confusion. They introduced schemas. The JSON schema does not accept underscore, and the other schema does. SnakeYAML Engine now only supports JSON schema

[cowtowncoder]

Ah. Interesting, did not know this was the main reason.

[cowtowncoder]

After implementing #133, underscores are actually handled as expected with upcoming Jackson 2.12.
Handling for Jackson 3.x is bit of an open question, but for 2.x issue is resolved (as "old" snakeyaml does tag such integers as expected and now Jackson decodes them ok too).

NOTE: as per earlier comments, handling for standard decimal numbers has been working since 2.10.5.
In 2.12 other types support this as well.