Skip to content
New issue

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

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

Already on GitHub? # to your account

Incorrect BigDecimal fraction representation (2.9 -> 2.10 change) #139

Closed
wlukowicz opened this issue Jun 15, 2018 · 6 comments
Closed

Incorrect BigDecimal fraction representation (2.9 -> 2.10 change) #139

wlukowicz opened this issue Jun 15, 2018 · 6 comments
Labels
cbor compatibility Change that has compatibility aspects (possible breakage)
Milestone

Comments

@wlukowicz
Copy link

According to the spec, decimal fractions (tag 4) of value m * 10^e should be represented as a tagged array containing the exponent e and the mantissa m. However, Jackson writes BigDecimals as tagged arrays containing their scale and unscaled value. The difference being that the value of BigDecimal is unscaled value * 10^-scale, which means the scale is the opposite of the exponent e.

The spec gives an example how 273.15 should be represented:

C4             -- Tag 4
   82          -- Array of length 2
      21       -- -2
      19 6AB3  -- 27315

Here is the actual representation:

C4             -- Tag 4
   82          -- Array of length 2
      02       -- 2
      19 6AB3  -- 27315
@cowtowncoder
Copy link
Member

Ok. Sounds like there is a problem, and looks like information included should be enough to reproduce the problem as a unit test.
Thank you for reporting this.

@cowtowncoder cowtowncoder changed the title [cbor] Incorrect decimal fraction representation Incorrect decimal fraction representation Nov 28, 2018
@cowtowncoder
Copy link
Member

Hmmh. Ok, yes. Looks like I interpreted exponent the wrong way, but for better or worse it is consistently so, meaning that Jackson can read content it writes.

The challenge here is backwards compatibility. I think that I will fix this for 2.10, instead of 2.9 patch, since this does change working quite drastically.

@cowtowncoder cowtowncoder added this to the 2.10.0 milestone Nov 28, 2018
@wlukowicz
Copy link
Author

Looks good, thanks for fixing.

msqr added a commit to SolarNetwork/jackson-dataformats-binary that referenced this issue Dec 16, 2019
… by CBOR generator < v2.10 with bug FasterXML#139. Assumes all generated BigDecimal values had negative exponents to begin with so not generally applicable to community at large.
msqr added a commit to SolarNetwork/solarnetwork-node that referenced this issue Dec 17, 2019
…FasterXML/jackson-dataformats-binary#139. This way the recipient can see if the message was encoded with >= 2.10 CBOR version, or else < 2.10 with the bug present.
msqr added a commit to SolarNetwork/jackson-dataformats-binary that referenced this issue Dec 17, 2019
…nt negation, so bug FasterXML#139 fix can be toggled on/off to support backwards compatibility.
@cynogit
Copy link

cynogit commented Mar 10, 2021

@cowtowncoder,
recently tried to migrate from 2.9.9. It was a stroke of big luck we found this breaking change you introduced before it reached our production. Did you consider adding a DeserializationOption and a default value safe to the people who already have the older format in their production DBs? Why doesn't Jackson release notes mark this commit as breaking change?

It's a great shame, guys, great shame :-(

@cowtowncoder
Copy link
Member

@cynogit Mark what as breaking change? A fix? What kind of sense would that make?
Or perpetuating incorrect encoding?

It is unfortunate that the bug existed of course. I will add a note on 2.10 release wiki page, but this is the first time I have heard a complaint from users.

Also: if you wanted to actually help instead of bitch and whine about a free product, you are free to help with release candidate process: this is where most compatibility problems are found, by people who are collaborating on improving things. Instead of simply enjoying free labor of others.

@cowtowncoder cowtowncoder added the compatibility Change that has compatibility aspects (possible breakage) label Mar 10, 2021
@cowtowncoder cowtowncoder changed the title Incorrect decimal fraction representation Incorrect BigDecimal fraction representation (2.9 -> 2.10 change) Mar 10, 2021
@cynogit
Copy link

cynogit commented Mar 10, 2021

@cowtowncoder, consider you already have a production DB written with 2.9. Reading it with 2.10 will produce objects with incorrect values. That is why it is a breaking change.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
cbor compatibility Change that has compatibility aspects (possible breakage)
Projects
None yet
Development

No branches or pull requests

3 participants