Skip to content

sbe: refuse a value its (sbe.encoding) primitive cannot hold (#133) - #143

Merged
trendvidia merged 1 commit into
mainfrom
sbe-encoding-range-133
Nov 5, 2026
Merged

trendvidia merged 1 commit into
mainfrom
sbe-encoding-range-133

Conversation

@bfrancojr

Copy link
Copy Markdown
Collaborator

Fixes #133.

A value that doesn't fit its (sbe.encoding) primitive is now refused instead of losing its high bits. Before, int32 v [(sbe.encoding) = "int8"] set to 300 was written as 44 and read back as 44. On read, uint32 2^31 in an int32 field wrapped to a negative number. The draft says "Emitters MUST reject values that fall outside the narrowed type's range". Ported from protowire-go #255 (issue #252).

  • Marshal reads the value by the field's kind and checks it against the primitive's range: sbe: field M.v: value 300 is outside the range of int8, [-128, 127]. A double under "float" is checked against ±3.4028234663852886e+38 (NaN and ±Inf pass). An enum, laid out as uint8 by default, refuses a value past 255 or below 0.
  • Unmarshal sign-extends per the primitive, then checks the result against the field's range: sbe: field M.v: wire value 2147483648 (uint32) does not fit a int32 field, [-2147483648, 2147483647]. A bool still reads any nonzero value as true.
  • Codec::New refuses (sbe.encoding) on a string or bytes field, and an integer/float crossing, with Go's three texts.
  • View is unchanged, as in Go's commit.
  • The ranges are computed per field from the descriptor at encode and decode time. Go precomputes them at bind time. Doing it per call avoids adding fields to the public FieldTemplate struct. Go's %g in the float messages is reproduced by a small GoG helper in the new internal src/sbe/encoding_range.h.

Also fixed (same code path): a double field under "float" used to write the low 4 bytes of the double's bits, and a float field under "double" the float's bits zero-extended. Both read back as garbage. They now convert the value, as Go does. Go's 100-pairing matrix test pins it.

Stack and merge order: #123 → #124 → #137 (#134) → #140 (#132) → this PR. Base: sbe-header-bounds-132. This PR builds on #132's rewrite of FieldEncodingSize and its Go-style error wrapping.

Evidence, at 0529a25, on macOS:

@bfrancojr

Copy link
Copy Markdown
Collaborator Author

Rebased onto main e1c1b72 (#136 merged); head is now 448b8af. The PR's own diff is unchanged, checked by comparing its added and removed lines before and after the rebase. Rebased as part of the SBE stack; evidence is on #123.

@bfrancojr

Copy link
Copy Markdown
Collaborator Author

Rebased with the stack onto main df6b102; head is now 456dd6e. Evidence is on #124.

@trendvidia
trendvidia force-pushed the sbe-encoding-range-133 branch from 456dd6e to b945fdf Compare November 5, 2026 00:49
@trendvidia
trendvidia force-pushed the sbe-header-bounds-132 branch from 43eabbe to 3486d29 Compare November 5, 2026 00:49
@bfrancojr

Copy link
Copy Markdown
Collaborator Author

Rebased onto main 197e05b, after #124 and #137 merged as squashes: 456dd6e → b945fdf. The old #124 and #137 commits were dropped (git rebase --onto origin/main bcedc19), so this PR's own commit is the only one replayed. Its diff is unchanged, apart from where its CHANGELOG line lands. At the stack top (39e1261): build OK, ctest 100% tests passed out of 705. The remote head was read back with git ls-remote: b945fdf.

Base automatically changed from sbe-header-bounds-132 to main November 5, 2026 02:07
Marshal narrowed a value to its SBE primitive by keeping the low bits,
so an int32 300 under "int8" was written as 44 and read back as 44. The
draft says "Emitters MUST reject values that fall outside the narrowed
type's range". Unmarshal likewise cut a wire value to the field's low
bits.

As protowire-go does (#252): the value is read by the field's kind and
checked against the primitive's range before it is written; Unmarshal
sign-extends per the primitive and checks the result against the
field's range before storing it. A double narrowed to float is checked
against ±max float. Codec::New refuses (sbe.encoding) on a string or
bytes field and a crossing between integers and floating point.

A double field under "float", and a float field under "double", used to
write and read raw bits of the wrong width; they now convert the value.

Tests ported from Go's encoding/sbe/encoding_range_test.go, including
its 100-pairing matrix (68 bind and round-trip, 32 are refused).
@trendvidia
trendvidia force-pushed the sbe-encoding-range-133 branch from b945fdf to 72c54f1 Compare November 5, 2026 02:08
@bfrancojr

Copy link
Copy Markdown
Collaborator Author

Rebased onto main 9525e9b, after #140 merged as a squash: b945fdf → 72c54f1. I dropped #140's old commit with git rebase --onto origin/main 3486d29. The diff is unchanged apart from where the CHANGELOG line lands. At the stack top (2ee007c): build OK, ctest 100% tests passed out of 705. The remote head was read back with git ls-remote: 72c54f1.

@trendvidia
trendvidia merged commit 1706247 into main Nov 5, 2026
11 checks passed
@trendvidia
trendvidia deleted the sbe-encoding-range-133 branch November 5, 2026 02:45
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.

sbe: a value its (sbe.encoding) primitive cannot hold keeps its low bits instead of being refused (protowire-go#255)

2 participants