Skip to content

Negative BigInteger values not encoded/decoded correctly #431

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

Closed
bgruber opened this issue Dec 26, 2023 · 4 comments
Closed

Negative BigInteger values not encoded/decoded correctly #431

bgruber opened this issue Dec 26, 2023 · 4 comments
Labels
2.20 cbor pr-needed Feature request for which PR likely needed (no active development but idea is workable)
Milestone

Comments

@bgruber
Copy link

bgruber commented Dec 26, 2023

Hi!
I was doing some work with the CBOR part of the library, and I found that it didn't decode or encode negative BigIntegers as I understand the spec. For example:

var negOne = BigInteger.ONE.negate();
var cborMapper = new ObjectMapper(new CBORFactory());
var bytes = cborMapper.writeValueAsBytes(negOne);
System.out.println(org.apache.commons.codec.binary.Hex.encode(bytes));

This prints out c34101, but per this CBOR playground, that's CBOR for a bigint of -2, not -1.

Similarly on the read side:

    var cborMapper = new ObjectMapper(new CBORFactory());
    var bytes =
        new byte[] {
          (byte) 0xC3,
          (byte) 0x50,
          (byte) 0xFF,
          (byte) 0xFF,
          (byte) 0xFF,
          (byte) 0xFF,
          (byte) 0xFF,
          (byte) 0xFF,
          (byte) 0xFF,
          (byte) 0xFF,
          (byte) 0xFF,
          (byte) 0xFF,
          (byte) 0xFF,
          (byte) 0xFF,
          (byte) 0xFF,
          (byte) 0xFF,
          (byte) 0xFF,
          (byte) 0xFF
        };
    var read = cborMapper.readValue(bytes, BigInteger.class);
    System.out.println(read);

Prints 1, instead of -340282366920938463463374607431768211456 (-2^128). CBOR playground

I think the issue is that the library is constructing the BigIntegers from the byte string, and then negating it, instead of taking the complement of the byte string (or "-1 - n" as stated by the RFC) and constructing the BigInteger from that.

Thanks for all your work on jackson!

@cowtowncoder
Copy link
Member

That sounds plausible to me. Thank you for reporting this @bgruber.

But aside from confirming the case and proposing a fix, the tricky part is compatibility.
It is unfortunate that generation produces wrong value (essentially corrupt encoding); if this was only for decoding there'd be bit less problem.
I suspect it will be necessary to add CBORGenerator.Feature and CBORParser.Features to control handling, and possibly even default these to existing incorrect behavior, given that format backend is used in production and so just changes can be pretty nasty.

@cowtowncoder cowtowncoder added the pr-needed Feature request for which PR likely needed (no active development but idea is workable) label Jun 7, 2024
@cowtowncoder
Copy link
Member

I think I could use help here if anyone has time: I can help in configuration part, but would need a simple(st) reproduction for a small negative number (easiest to verify, handling not dependent on magnitude).

@cowtowncoder cowtowncoder added need-test-case To work on issue, a reproduction (ideally unit test) needed 2.18 and removed 2.17 labels Aug 23, 2024
@cowtowncoder
Copy link
Member

@bgruber Is this something you could help with? I'd really like to address this for 2.18.0, but would need help.

@cowtowncoder cowtowncoder added 2.20 and removed need-test-case To work on issue, a reproduction (ideally unit test) needed 2.18 labels Apr 30, 2025
@cowtowncoder cowtowncoder changed the title CBOR: negative BigInteger values not handled correctly Negative BigInteger values not encoded/decoded correctly Apr 30, 2025
@cowtowncoder cowtowncoder added this to the 2.20.0 milestone Apr 30, 2025
@cowtowncoder
Copy link
Member

Fixed via #578 by @iifawzi -- thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.20 cbor pr-needed Feature request for which PR likely needed (no active development but idea is workable)
Projects
None yet
Development

No branches or pull requests

2 participants