Repository navigation
Fix to_bjdata() emitting the Draft-3-only 'B' marker in default Draft-2 mode - #5479
Merged
Merged
Conversation
nlohmann
force-pushed
the
issue-5404-bjdata-byte-draft-gate
branch
from
September 9, 2026 08:21
50e392a to
daa2e74
Compare
nlohmann
force-pushed
the
issue-5404-bjdata-byte-draft-gate
branch
2 times, most recently
from
September 11, 2026 15:56
eb45908 to
93295e0
Compare
gregmarr
reviewed
Sep 11, 2026
| // readers reject, so such an object falls back to a plain object | ||
| // encoding instead (see the "Binary values" section of the BJData | ||
| // documentation) | ||
| if (dtype == 'B' && bjdata_version != bjdata_version_t::draft3) |
Contributor
There was a problem hiding this comment.
Is there a way to future-proof this to "at least draft3"?
Owner
Author
There was a problem hiding this comment.
Fair point — bjdata_version_t only has two values today, so != and < happened to coincide. Pushed a change to bjdata_version < bjdata_version_t::draft3, which keeps working if a later draft keeps the 'B' marker valid.
Written by Claude Code on behalf of @nlohmann.
…-2 mode _ArrayType_ = "byte" mapped unconditionally to the BJData type marker 'B', regardless of the requested bjdata_version. 'B' is defined only by BJData Draft 3; with the default version (draft2), this produced a stream that is invalid for Draft 2 and, unlike every other _ArrayType_, round-tripped back as a binary value instead of the original annotated object. Only accept "byte" / emit 'B' when bjdata_version selects Draft 3. Under Draft 2, fall back to the same plain-object encoding used elsewhere in this function for other invalid-annotation cases, so the value round-trips correctly. Fixes #5404. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@gregmarr pointed out that dtype == 'B' && bjdata_version != draft3 only future-proofs by accident, since bjdata_version_t currently has exactly two values. Compare with < instead, so a later draft that keeps the 'B' marker valid does not need this gate revisited. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann
force-pushed
the
issue-5404-bjdata-byte-draft-gate
branch
from
September 16, 2026 18:17
9b19e94 to
b205250
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked PR
This PR is stacked on top of #5473 (fix for #5403) and is based on its branch
issue-5403-bjdata-uint-rangerather thandevelop. It should be reviewed and merged after that PR.Summary
In the same BJData ndarray writer touched by #5473,
_ArrayType_ = "byte"mapped unconditionally to the BJData type marker'B', regardless of thebjdata_versionrequested forto_bjdata(). But'B'is defined only by BJData Draft 3; the docs (docs/mkdocs/docs/features/binary_formats/bjdata.md, "Binary values") say the Draft 3 optimized binary array "must be explicitly enabled using theversionparameter ofto_bjdata".With the default
version = draft2, this produced a stream containing a marker that Draft 2 does not define and, unlike every other_ArrayType_, the value round-tripped back throughfrom_bjdataas a binary value instead of the original annotated object — because a'B'-typed optimized array without the ndarray flag is otherwise the library's own encoding for binary data.Fix
write_bjdata_ndarraynow checksbjdata_versionright after resolvingdtypefrom_ArrayType_: ifdtype == 'B'andbjdata_versionis notdraft3, it falls back to the same plain-object encoding already used by this function for other invalid-annotation cases (the same mechanism added/reused in #5473). Draft 3 explicitly selected keeps emitting the compact'B'ndarray encoding exactly as before.Tests
v_Bin the"optimized ndarray (type and vector-size ndarray with JData annotations)"section to passbjdata_version_t::draft3explicitly, sincev_Buses the Draft-3-only'B'marker."byte"out of the generic type loop in"ndarray parsed from text is written as a typed array"into its own check that explicitly selects Draft 3."ndarray with _ArrayType_ "byte" is gated by the BJData draft version", covering:'B'ndarray encoding, with an exact expected byte sequence, and round-trips correctlyRan the full
unit-bjdatasuite offline (compiled againstinclude/with a stubtest_data.hpp): 693941/693942 assertions pass; the one failure and the one skipped test case are pre-existing and unrelated (they require downloaded test data, unavailable in this offline setup) — identical to the baseline ondevelop. Also ranunit-ubjson,unit-binary_formats, andunit-regression2offline with no new failures.Breaking change?
No breaking changes to the public API. This changes only the wire output of
to_bjdata()for annotated objects with_ArrayType_ = "byte"under the default (Draft 2) mode: such objects now serialize as plain objects (matching every other invalid-for-the-current-mode annotation already handled by this function) instead of emitting a Draft-3-only marker that Draft 2 readers do not define. Draft 3 output is unchanged.Fixes #5404.
— opened by Claude Code on behalf of @nlohmann