Skip to content

Encode uint64 values above INT64_MAX in UBJSON - #753

Merged
danielaparker merged 2 commits into
danielaparker:masterfrom
youdie006:ubjson-uint64-above-int64
Oct 7, 2026
Merged

danielaparker merged 2 commits into
danielaparker:masterfrom
youdie006:ubjson-uint64-above-int64

Conversation

@youdie006

Copy link
Copy Markdown
Contributor

encode_ubjson writes nothing for a uint64_t value above INT64_MAX but still counts it as a container element. [9223372036854775808] becomes 5b235501 (an array that declares one element and contains none), and a top-level value becomes an empty buffer; decode_ubjson and other UBJSON readers reject both.

UBJSON has no unsigned 64-bit type, so this writes such values as high-precision numbers (H), as the encoder already does for bigint strings; the parser reads them back as semantic_tag::bigint. The bytes match py-ubjson's encoding of the same values.

Added a test with the exact bytes and a decode_ubjson round trip, plus a guard that INT64_MAX stays L. The ubjson tests pass under ASan/UBSan.

Written with AI assistance (Claude); I have reviewed the change.

visit_uint64 wrote nothing for values above INT64_MAX but still counted
the element, so the output declared more elements than it held. Write
them as high-precision numbers, as the encoder does for bigint strings.
@danielaparker

Copy link
Copy Markdown
Owner

Thanks for contributing!

I would suggest replacing

for (auto c : s)
{
    sink_.push_back(c);
}

with

sink_.append(s.data(), s.size());

Otherwise it's fine.

@youdie006

Copy link
Copy Markdown
Contributor Author

Thanks, done in d5f5fcf. The bytes sink's append takes const uint8_t*, so it casts the way cbor_encoder does: sink_.append(reinterpret_cast<const uint8_t*>(s.data()), s.size()). The ubjson tests still pass.

@danielaparker
danielaparker merged commit 8f5b66b into danielaparker:master Oct 7, 2026
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

Successfully merging this pull request may close these issues.

2 participants