Repository navigation
sbe: refuse sizes and ids past SBE's 16-bit header fields (#132) - #140
Merged
Merged
Conversation
trendvidia
force-pushed
the
sbe-template-lookup-134
branch
from
November 4, 2026 19:17
3e8f6d4 to
c7e097e
Compare
trendvidia
force-pushed
the
sbe-header-bounds-132
branch
from
November 4, 2026 19:17
db81e1c to
95517ac
Compare
trendvidia
force-pushed
the
sbe-template-lookup-134
branch
from
November 4, 2026 22:51
c7e097e to
bcedc19
Compare
trendvidia
force-pushed
the
sbe-header-bounds-132
branch
from
November 4, 2026 22:51
95517ac to
43eabbe
Compare
Collaborator
Author
Every SBE header field is a uint16, and the codec kept the low 16 bits of a larger value: (sbe.length) = 65537 bound as a 1-byte field, a block past 65535 bytes bound with wrapped offsets, a template_id, schema_id or version of 65537 was written as 1, and a group of 65536 entries was written with numInGroup 0. As protowire-go does (#250): offsets are accumulated in int and checked before they become a uint16, Codec::New refuses each schema shape with Go's text, and Marshal refuses a group of more than 65535 entries. The template builders now word every bind error as Go does, prefixed with the field, group or composite it is about. Tests ported from Go's encoding/sbe/header_bounds_test.go, each refusal paired with the same shape at 65535, which binds and round-trips.
trendvidia
force-pushed
the
sbe-header-bounds-132
branch
from
November 5, 2026 00:49
43eabbe to
3486d29
Compare
Collaborator
Author
|
Rebased onto main 197e05b, after #124 and #137 merged as squashes: 43eabbe → 3486d29. The old #124 and #137 commits were dropped ( |
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.
Fixes #132.
A schema or message that SBE's 16-bit header fields can't describe is now refused instead of being written with the wrong numbers. Before,
(sbe.length) = 65537bound as a 1-byte field. A block past 65535 bytes bound with wrapped offsets. A template id, schema id or version of 65537 was written as 1. A group of 65536 entries was written with a count of 0 and read back empty, with no error. Ported from protowire-go #254 (issue #250).BuildTemplate,BuildGroupTemplateandBuildCompositeFieldsadd up offsets inintand check each field's end before narrowing, as Go'sendOfFielddoes. Every offset in a template is then in range.Codec::Newrefuses(sbe.template_id),(sbe.schema_id),(sbe.version)and(sbe.length)past 65535, with Go's text.Marshalrefuses a repeated field with more than 65535 entries:sbe: field M.e has 65536 entries, past 65535, the largest numInGroup SBE can state.Also fixed: every bind error of a template now has Go's text and prefix, such as
sbe: field m.v1.M.s: string field requires (sbe.length) annotation(it wasstring/bytes field requires (sbe.length)). Go's bound errors carry that prefix, so matching them meant matching the rest. No test pinned the old texts.Stack and merge order: #123 → #124 → #137 (#134) → this PR → #133's PR. Base:
sbe-template-lookup-134. The two are independent in code, but both editCodec::Newand the CHANGELOG, so stacking avoids a conflict.Evidence, at db81e1c, on macOS:
ctest: 100% passed, 682 of 682, includingSbeHeaderBounds.NewCodec(Go's 18 cases, each refusal paired with the same shape at 65535, which binds, round-trips, and whose header states the declared values) andSbeHeaderBounds.MarshalGroupCount(65535 entries round-trip; 65536 are refused).EndOfFieldandnumInGroupchecks disabled, both tests fail.scripts/go_differential.shagainst protowire-go main 207fdfc: exit 1, from the same 450 PXF diff lines as at sbe: tell two schemas' template ids apart (#134) #137 (compared withcmp). Every section ran (18 SBE schemas converted alike,362 documents), and no differing line mentions sbe. What remains is pxf: Marshal leaves out an unset (pxf.default) or (pxf.required) field, so it doesn't read back #129, pxf: FormatDocument drops every directive, so a @dataset document formats to nothing #130 and pxf: reject null on a (pxf.required) or @required field #131, each with its own PR.--dry-run --Werror: clean.LoadU16,WriteU16/32/64, the sbe: cut an overlong string on a character boundary #115 cut loop), which pass CI's tidy-18.Not in this PR: Go also refuses a oneof field at bind time (
sbe: oneof field M.f not supported), and this port doesn't. That changes behaviour, so it's filed separately rather than folded in here.