diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c6cc7b..9a3072e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ number says nothing about any other port. See the Releases section of - **A decoder rejects `null` on a field with `(pxf.required)` or `@required`** (#131, decided by the owner; trendvidia/protowire#569, protowire-go#268). **This narrows what decodes.** A required field holds a value, so `null` is now rejected as absence is, whatever the field's type and presence. Before, `null` counted as present, so `r = null` on `optional int32 r [(pxf.required) = true]` decoded and left a required field with no value. The error is `null is not allowed on required field "r": a required field holds a value`, in a body, a nested body, a `@dataset` cell and a dotted cell, from `Unmarshal` as well as `UnmarshalFull`. A plain scalar with `(pxf.required)`, which already rejected `null`, now names the field as required instead of suggesting `optional`. A zero value still satisfies the field. A `_null` mask that names an unset required field no longer makes `AppendDatasetRow` write `null` for it: the writer refuses it, as it does without the mask. - **`pxf::Marshal` writes an unset field that has `(pxf.default)` or `(pxf.required)` so that it reads back** (#129, protowire-go#259; draft -01 § Common Schema Layer, "`decode_PXF(encode_PXF(v)) == v`"), as protowire-go does. This changes what Marshal writes for annotated fields, and narrows what encodes. Marshal left out every unset field, so a reader that applies defaults filled it in: `int32 n = 1 [(pxf.default) = "5"]` holding 0 read back as 5, and a required scalar holding 0 made the full decode refuse the document. A proto3 scalar without presence is now written as its zero value, a required list or map as `[]` or `{}`, and a message-typed field with a default, such as a wrapper or a Timestamp, as `null`. An unset `optional` or oneof scalar with a default is still left out and reads back as its default, the owner's choice in protowire-go#267. An unset `optional` field that is `(pxf.required)` is now an error, `pxf: is unset, and the field is (pxf.required)`; with `emit_defaults` it is written as zero, as before. An Any whose body has such a field stays as `type_url` and `value`. Inside a `@dataset` value cell, the same error now names the field by its full name. Unannotated fields are unchanged. +- **`sbe` refuses a value its `(sbe.encoding)` primitive cannot hold, in both directions** (#133, protowire-go#252; draft -01 § SBE Annotations, "Emitters MUST reject values that fall outside the narrowed type's range"). This narrows what encodes, decodes and binds. `Marshal` used to keep the low bits: `int32 v [(sbe.encoding) = "int8"]` set to 300 was written as 44, and read back as 44 with no error. It now refuses the value with `sbe: field m.v1.M.v: value 300 is outside the range of int8, [-128, 127]`, for every integer primitive, and a `double` narrowed to `float` past ±3.4028234663852886e+38 (NaN and the infinities still pass). An enum is laid out as `uint8`, so an enum value past 255, or a negative one, is refused too. `Unmarshal` refuses a wire value the field cannot hold, such as `uint32` 2^31 into an `int32` field, with `sbe: field m.v1.M.v: wire value 2147483648 (uint32) does not fit a int32 field, [-2147483648, 2147483647]`; it used to cut it to the field's low bits. A signed primitive is now sign-extended whatever the field, so `int8` 0xFF in a `uint32` field is -1 and refused, where it used to read as 255. `Codec::New` refuses `(sbe.encoding)` on a `string` or `bytes` field, and a primitive that crosses between integers and floating point (`int32` with `"float"`, `double` with `"int32"`). A `double` field under `"float"`, and a `float` field under `"double"`, used to write and read the wrong bits; they now convert the value. - **`sbe::Codec::New` refuses a schema the SBE header cannot describe, and `Marshal` a group it cannot count** (#132, protowire-go#250). This narrows what binds and what encodes. Every SBE header field is a 16-bit number, and the codec kept the low 16 bits of a larger one. `bytes b = 1 [(sbe.length) = 65537]` bound as a 1-byte field. A block longer than 65535 bytes, in a message, a group entry or a composite, bound with a wrapped length and offsets. A `(sbe.template_id)`, `(sbe.schema_id)` or `(sbe.version)` of 65537 was written as 1. `Codec::New` now refuses each of these with protowire-go's text, naming the field, message or file, such as `sbe: field m.v1.M.b: (sbe.length) = 65537 is past 65535, the largest blockLength SBE can state`; 65535 still binds. `Marshal` refuses a repeated field with more than 65535 entries, with `sbe: field m.v1.M.e has 65536 entries, past 65535, the largest numInGroup SBE can state`: 65536 entries used to be written with a count of 0, and the message read back empty with no error. Every other bind error of a template now has protowire-go's text as well, prefixed with the field, group or composite it is about, such as `sbe: field m.v1.M.s: string field requires (sbe.length) annotation` where it used to be `string/bytes field requires (sbe.length)`. - **`sbe` tells two schemas' template ids apart** (#134, protowire-go#253 and #257; draft -01 § SBE Annotations, "sbe.template_id MUST be unique among messages within a schema", and § Header and Block Length). This narrows what binds and what decodes. `Codec::New` refuses two different messages with one `(sbe.template_id)` in one schema, with `sbe: messages A and B share (sbe.template_id) = 7 in schema 1; it must be unique within a schema`; the second used to replace the first, and `NewView` then read one message's bytes as the other's. A file passed twice still binds, and so do two schemas that each use a template id. `NewView` now finds a template by the header's schema id and template id together, and refuses an unknown pair with `sbe: unknown template ID 9 in schema 2`, even if another schema has that template id; it used to say `unknown template id 9`. `Unmarshal` and `UnmarshalDescriptor` refuse a header whose schema id is not the target message's, with `sbe: schema ID mismatch: got 1, want 2 for template 7`; the version is not checked. A caller that builds headers by hand must now set the schema id. - **`pxf::Marshal` refuses a message nested past `kMaxNestingDepth`** (#107, protowire#502), as protowire-go does (protowire-go#236), with `pxf: nesting depth exceeds MaxNestingDepth=100`. Depth is counted as a decoder counts it: the root is depth 0, and each `{` or `[` is one level, so a list of lists stops at 50. It used to write a document that a reader at the same limit refused. **This narrows what encodes.** `AppendDatasetRow` refuses too. An Any whose body would be too deep stays as `type_url` and `value`, so an Any's contents never cause a refusal; only an Any that itself sits past the limit does. An Any's value is also no longer cut off at libprotobuf's 100-level parse limit when the writer's limit is raised. `pb::Marshal` cannot refuse yet (#93). diff --git a/src/sbe/encoding_range.h b/src/sbe/encoding_range.h new file mode 100644 index 0000000..a2ec963 --- /dev/null +++ b/src/sbe/encoding_range.h @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 TrendVidia, LLC. +// +// The range of an (sbe.encoding) primitive and of the field it narrows, for +// the checks of draft -01 § SBE Annotations: "Emitters MUST reject values +// that fall outside the narrowed type's range". Marshal refuses a value the +// primitive cannot hold, and Unmarshal one the field cannot hold, rather +// than keep its low bits (protowire-go#252). +#pragma once + +#include +#include +#include +#include +#include +#include + +#include + +#include "protowire/sbe/template.h" + +namespace protowire::sbe { + +inline bool IsIntEncoding(std::string_view e) { + return e == kEncInt8 || e == kEncInt16 || e == kEncInt32 || e == kEncInt64 || e == kEncUint8 || + e == kEncUint16 || e == kEncUint32 || e == kEncUint64; +} + +// EncodingRange holds, for an integer primitive, its range and the field's, +// and whether a value can fall outside the other. For a float primitive, the +// checks are a double that float cannot hold. +struct EncodingRange { + int64_t enc_lo = 0; + uint64_t enc_hi = 0; + int64_t field_lo = 0; + uint64_t field_hi = 0; + bool enc_signed = false; + bool write_check = false; + bool read_check = false; +}; + +// FieldRange is the range of values an integer, bool or enum field holds. An +// enum is open in proto3, so any int32. +inline void FieldRange(const google::protobuf::FieldDescriptor* fd, int64_t* lo, uint64_t* hi) { + using FD = google::protobuf::FieldDescriptor; + switch (fd->type()) { + case FD::TYPE_INT32: + case FD::TYPE_SINT32: + case FD::TYPE_SFIXED32: + case FD::TYPE_ENUM: + *lo = std::numeric_limits::min(); + *hi = std::numeric_limits::max(); + return; + case FD::TYPE_INT64: + case FD::TYPE_SINT64: + case FD::TYPE_SFIXED64: + *lo = std::numeric_limits::min(); + *hi = std::numeric_limits::max(); + return; + case FD::TYPE_UINT32: + case FD::TYPE_FIXED32: + *lo = 0; + *hi = std::numeric_limits::max(); + return; + case FD::TYPE_BOOL: + *lo = 0; + *hi = 1; + return; + default: // TYPE_UINT64, TYPE_FIXED64 + *lo = 0; + *hi = std::numeric_limits::max(); + return; + } +} + +inline EncodingRange RangeOf(const FieldTemplate& ft) { + using FD = google::protobuf::FieldDescriptor; + EncodingRange r; + if (ft.encoding == kEncFloat) { + r.write_check = ft.fd->type() == FD::TYPE_DOUBLE; + } else if (ft.encoding == kEncDouble) { + r.read_check = ft.fd->type() == FD::TYPE_FLOAT; + } else if (IsIntEncoding(ft.encoding)) { + r.enc_signed = ft.encoding[0] == 'i'; + const unsigned bits = 8U * ft.size; + if (r.enc_signed) { + r.enc_hi = (uint64_t{1} << (bits - 1)) - 1; + r.enc_lo = -static_cast(r.enc_hi) - 1; + } else { + r.enc_lo = 0; + r.enc_hi = std::numeric_limits::max() >> (64 - bits); + } + FieldRange(ft.fd, &r.field_lo, &r.field_hi); + r.write_check = r.field_lo < r.enc_lo || r.field_hi > r.enc_hi; + // A bool reads any nonzero primitive as true. + r.read_check = + ft.fd->type() != FD::TYPE_BOOL && (r.enc_lo < r.field_lo || r.enc_hi > r.field_hi); + } + return r; +} + +// InRange reports whether the integer v is within [lo, hi]. v holds a +// uint64 when is_unsigned is true, and an int64's bits otherwise. +inline bool InRange(uint64_t v, bool is_unsigned, int64_t lo, uint64_t hi) { + if (!is_unsigned && static_cast(v) < 0) return static_cast(v) >= lo; + return v <= hi; +} + +// IntString formats an integer held as in InRange. +inline std::string IntString(uint64_t v, bool is_unsigned) { + return is_unsigned ? std::to_string(v) : std::to_string(static_cast(v)); +} + +// FloatFits reports whether a double is within float's range. NaN and the +// infinities are, as float has them; a finite value past ±max float is not, +// even one that would round down to it. +inline bool FloatFits(double f) { + constexpr double kMax = std::numeric_limits::max(); + return std::isinf(f) || !(f > kMax || f < -kMax); +} + +// GoG formats f as Go's %g does: the shortest digits that read back as f, +// in %e form when the exponent is below -4 or at least 6. +inline std::string GoG(double f) { + if (std::isnan(f)) return "NaN"; + if (std::isinf(f)) return f > 0 ? "+Inf" : "-Inf"; + char buf[64]; + auto sci = std::to_chars(buf, buf + sizeof(buf), f, std::chars_format::scientific); + std::string s(buf, sci.ptr); + const size_t e = s.find('e'); + const int exp = std::stoi(s.substr(e + 1)); + if (exp < -4 || exp >= 6) return s; + auto fix = std::to_chars(buf, buf + sizeof(buf), f, std::chars_format::fixed); + return std::string(buf, fix.ptr); +} + +} // namespace protowire::sbe diff --git a/src/sbe/marshal.cc b/src/sbe/marshal.cc index 76ea190..9c1e1c0 100644 --- a/src/sbe/marshal.cc +++ b/src/sbe/marshal.cc @@ -5,9 +5,11 @@ #include "protowire/sbe.h" #include +#include #include #include +#include "encoding_range.h" #include "header_bounds.h" namespace protowire::sbe { @@ -27,59 +29,89 @@ void WriteU64(uint8_t* p, uint64_t v) { for (int i = 0; i < 8; ++i) p[i] = static_cast(v >> (i * 8)); } -void EnsureAndWriteScalar(std::vector& buf, - size_t offset, - const FieldTemplate& ft, - uint64_t v_bits) { +// WriteScalar writes a numeric field as its primitive. The value is read by +// the field's kind and, for an integer primitive, written as the low bytes of +// its two's complement once it is known to fit; a value the primitive cannot +// hold is refused rather than cut to its low bits (draft -01 § SBE +// Annotations, protowire-go#252). +Status WriteScalar(std::vector& buf, + size_t offset, + const FieldTemplate& ft, + const pb::Message& msg) { while (buf.size() < offset + ft.size) buf.push_back(0); uint8_t* p = buf.data() + offset; + const pb::Reflection* r = msg.GetReflection(); + const pb::FieldDescriptor* fd = ft.fd; + const EncodingRange rg = RangeOf(ft); + + if (ft.encoding == kEncFloat || ft.encoding == kEncDouble) { + const double f = fd->cpp_type() == pb::FieldDescriptor::CPPTYPE_FLOAT + ? static_cast(r->GetFloat(msg, fd)) + : r->GetDouble(msg, fd); + if (ft.encoding == kEncDouble) { + uint64_t b = 0; + std::memcpy(&b, &f, 8); + WriteU64(p, b); + return Status::OK(); + } + if (rg.write_check && !FloatFits(f)) { + return Status::Error("sbe: field " + std::string(fd->full_name()) + ": value " + GoG(f) + + " is outside the range of float, ±" + + GoG(std::numeric_limits::max())); + } + const auto narrowed = static_cast(f); + uint32_t b = 0; + std::memcpy(&b, &narrowed, 4); + WriteU32(p, b); + return Status::OK(); + } + + uint64_t v = 0; + bool is_unsigned = false; + switch (fd->cpp_type()) { + case pb::FieldDescriptor::CPPTYPE_BOOL: + v = r->GetBool(msg, fd) ? 1 : 0; + is_unsigned = true; + break; + case pb::FieldDescriptor::CPPTYPE_ENUM: + v = static_cast(int64_t{r->GetEnumValue(msg, fd)}); + break; + case pb::FieldDescriptor::CPPTYPE_UINT32: + v = r->GetUInt32(msg, fd); + is_unsigned = true; + break; + case pb::FieldDescriptor::CPPTYPE_UINT64: + v = r->GetUInt64(msg, fd); + is_unsigned = true; + break; + case pb::FieldDescriptor::CPPTYPE_INT32: + v = static_cast(int64_t{r->GetInt32(msg, fd)}); + break; + default: // CPPTYPE_INT64 + v = static_cast(r->GetInt64(msg, fd)); + break; + } + if (rg.write_check && !InRange(v, is_unsigned, rg.enc_lo, rg.enc_hi)) { + return Status::Error("sbe: field " + std::string(fd->full_name()) + ": value " + + IntString(v, is_unsigned) + " is outside the range of " + ft.encoding + + ", [" + std::to_string(rg.enc_lo) + ", " + std::to_string(rg.enc_hi) + + "]"); + } switch (ft.size) { case 1: - p[0] = static_cast(v_bits); + p[0] = static_cast(v); break; case 2: - WriteU16(p, static_cast(v_bits)); + WriteU16(p, static_cast(v)); break; case 4: - WriteU32(p, static_cast(v_bits)); - break; - case 8: - WriteU64(p, v_bits); + WriteU32(p, static_cast(v)); break; - } -} - -uint64_t EncodeScalar(const pb::Message& msg, const FieldTemplate& ft) { - const pb::Reflection* r = msg.GetReflection(); - const pb::FieldDescriptor* fd = ft.fd; - switch (fd->cpp_type()) { - case pb::FieldDescriptor::CPPTYPE_BOOL: - return r->GetBool(msg, fd) ? 1 : 0; - case pb::FieldDescriptor::CPPTYPE_INT32: - return static_cast(r->GetInt32(msg, fd)); - case pb::FieldDescriptor::CPPTYPE_INT64: - return static_cast(r->GetInt64(msg, fd)); - case pb::FieldDescriptor::CPPTYPE_UINT32: - return r->GetUInt32(msg, fd); - case pb::FieldDescriptor::CPPTYPE_UINT64: - return r->GetUInt64(msg, fd); - case pb::FieldDescriptor::CPPTYPE_FLOAT: { - float f = r->GetFloat(msg, fd); - uint32_t b; - std::memcpy(&b, &f, 4); - return b; - } - case pb::FieldDescriptor::CPPTYPE_DOUBLE: { - double d = r->GetDouble(msg, fd); - uint64_t b; - std::memcpy(&b, &d, 8); - return b; - } - case pb::FieldDescriptor::CPPTYPE_ENUM: - return static_cast(r->GetEnumValue(msg, fd)); default: - return 0; + WriteU64(p, v); + break; } + return Status::OK(); } // CutLength is how many bytes of s fit in a field of size bytes. A string is @@ -109,34 +141,35 @@ void WriteFixedString(std::vector& buf, for (size_t i = n; i < ft.size; ++i) buf[offset + i] = 0; } -void WriteFields(std::vector& buf, - size_t base, - const std::vector& fields, - const pb::Message& msg); +Status WriteFields(std::vector& buf, + size_t base, + const std::vector& fields, + const pb::Message& msg); -void WriteCompositeField(std::vector& buf, - size_t base, - const FieldTemplate& ft, - const pb::Message& msg) { +Status WriteCompositeField(std::vector& buf, + size_t base, + const FieldTemplate& ft, + const pb::Message& msg) { const pb::Message& sub = msg.GetReflection()->GetMessage(msg, ft.fd); - WriteFields(buf, base + ft.offset, ft.composite, sub); + return WriteFields(buf, base + ft.offset, ft.composite, sub); } -void WriteFields(std::vector& buf, - size_t base, - const std::vector& fields, - const pb::Message& msg) { +Status WriteFields(std::vector& buf, + size_t base, + const std::vector& fields, + const pb::Message& msg) { for (const FieldTemplate& ft : fields) { if (!ft.composite.empty()) { - WriteCompositeField(buf, base, ft, msg); + if (Status st = WriteCompositeField(buf, base, ft, msg); !st.ok()) return st; continue; } if (ft.encoding == kEncChar) { WriteFixedString(buf, base + ft.offset, ft, msg); continue; } - EnsureAndWriteScalar(buf, base + ft.offset, ft, EncodeScalar(msg, ft)); + if (Status st = WriteScalar(buf, base + ft.offset, ft, msg); !st.ok()) return st; } + return Status::OK(); } } // namespace @@ -153,7 +186,7 @@ StatusOr> Codec::Marshal(const pb::Message& msg) const { WriteU16(buf.data() + 2, tmpl->template_id); WriteU16(buf.data() + 4, tmpl->schema_id); WriteU16(buf.data() + 6, tmpl->version); - WriteFields(buf, 8, tmpl->fields, msg); + if (Status st = WriteFields(buf, 8, tmpl->fields, msg); !st.ok()) return st; // Repeating groups. for (const GroupTemplate& gt : tmpl->groups) { @@ -174,7 +207,7 @@ StatusOr> Codec::Marshal(const pb::Message& msg) const { size_t entry_off = buf.size(); buf.resize(entry_off + gt.block_length, 0); const pb::Message& entry = r->GetRepeatedMessage(msg, gt.fd, i); - WriteFields(buf, entry_off, gt.fields, entry); + if (Status st = WriteFields(buf, entry_off, gt.fields, entry); !st.ok()) return st; } } return buf; diff --git a/src/sbe/template.cc b/src/sbe/template.cc index cd7f5e7..3cdca04 100644 --- a/src/sbe/template.cc +++ b/src/sbe/template.cc @@ -7,6 +7,7 @@ #include #include +#include "encoding_range.h" #include "header_bounds.h" #include "protowire/detail/go_quote.h" #include "protowire/sbe/annotations.h" @@ -56,6 +57,29 @@ StatusOr EndOfField(int offset, uint16_t size) { StatusOr> FieldEncodingSize(const pb::FieldDescriptor* fd) { if (auto enc = GetFieldStringOption(fd, kExtEncoding); enc.has_value()) { const std::string& e = *enc; + // The draft defines (sbe.encoding) as narrowing a numeric type, and says + // nothing of crossing between integers and floating point + // (protowire#549), so neither binds. + const std::string kind(fd->type_name()); + switch (fd->cpp_type()) { + case pb::FieldDescriptor::CPPTYPE_STRING: + return Status::Error("(sbe.encoding) = " + detail::GoQuote(e) + " on a " + kind + + " field; it narrows a numeric field, and a " + kind + + " field takes (sbe.length)"); + case pb::FieldDescriptor::CPPTYPE_FLOAT: + case pb::FieldDescriptor::CPPTYPE_DOUBLE: + if (IsIntEncoding(e)) { + return Status::Error("(sbe.encoding) = " + detail::GoQuote(e) + + " is an integer primitive and the field is " + kind); + } + break; + default: + if (e == kEncFloat || e == kEncDouble) { + return Status::Error("(sbe.encoding) = " + detail::GoQuote(e) + + " is a floating-point primitive and the field is " + kind); + } + break; + } if (e == kEncInt8 || e == kEncUint8) return std::make_pair(e, uint16_t{1}); if (e == kEncInt16 || e == kEncUint16) return std::make_pair(e, uint16_t{2}); if (e == kEncInt32 || e == kEncUint32 || e == kEncFloat) return std::make_pair(e, uint16_t{4}); diff --git a/src/sbe/unmarshal.cc b/src/sbe/unmarshal.cc index 2f7be84..6067a61 100644 --- a/src/sbe/unmarshal.cc +++ b/src/sbe/unmarshal.cc @@ -12,6 +12,7 @@ #include +#include "encoding_range.h" #include "protowire/detail/utf8.h" namespace protowire::sbe { @@ -72,18 +73,56 @@ Status DecodeScalarField(const uint8_t* p, const FieldTemplate& ft, pb::Message* return Status::OK(); } uint64_t bits = LoadScalar(p, ft.size); - // A signed encoding is sign-extended into a signed field, as protowire-go - // reads it and as View does: an int16 -1234 in an int32 field stays -1234. const auto cpp = fd->cpp_type(); - if (cpp == pb::FieldDescriptor::CPPTYPE_INT32 || cpp == pb::FieldDescriptor::CPPTYPE_INT64) { - if (ft.encoding == kEncInt8) { - bits = static_cast(int64_t{static_cast(bits)}); - } else if (ft.encoding == kEncInt16) { - bits = static_cast(int64_t{static_cast(bits)}); - } else if (ft.encoding == kEncInt32) { - bits = static_cast(int64_t{static_cast(bits)}); + const EncodingRange rg = RangeOf(ft); + + if (ft.encoding == kEncFloat || ft.encoding == kEncDouble) { + double f = 0; + if (ft.encoding == kEncFloat) { + const auto b = static_cast(bits); + float narrow = 0; + std::memcpy(&narrow, &b, 4); + f = narrow; + } else { + std::memcpy(&f, &bits, 8); + if (rg.read_check && !FloatFits(f)) { + return Status::Error("sbe: field " + std::string(fd->full_name()) + ": wire value " + + GoG(f) + " does not fit a float field, ±" + + GoG(std::numeric_limits::max())); + } + } + if (cpp == pb::FieldDescriptor::CPPTYPE_FLOAT) { + r->SetFloat(msg, fd, static_cast(f)); + } else { + r->SetDouble(msg, fd, f); + } + return Status::OK(); + } + + // An integer primitive, sign- or zero-extended per the primitive (draft -01 + // § SBE Annotations), then stored by the field's kind. A value the field + // cannot hold is refused rather than cut to its low bits (protowire-go#252). + if (rg.enc_signed) { + switch (ft.size) { + case 1: + bits = static_cast(int64_t{static_cast(bits)}); + break; + case 2: + bits = static_cast(int64_t{static_cast(bits)}); + break; + case 4: + bits = static_cast(int64_t{static_cast(bits)}); + break; + default: + break; } } + if (rg.read_check && !InRange(bits, !rg.enc_signed, rg.field_lo, rg.field_hi)) { + return Status::Error("sbe: field " + std::string(fd->full_name()) + ": wire value " + + IntString(bits, !rg.enc_signed) + " (" + ft.encoding + + ") does not fit a " + std::string(fd->type_name()) + " field, [" + + std::to_string(rg.field_lo) + ", " + std::to_string(rg.field_hi) + "]"); + } switch (cpp) { case pb::FieldDescriptor::CPPTYPE_BOOL: r->SetBool(msg, fd, bits != 0); @@ -100,21 +139,8 @@ Status DecodeScalarField(const uint8_t* p, const FieldTemplate& ft, pb::Message* case pb::FieldDescriptor::CPPTYPE_UINT64: r->SetUInt64(msg, fd, bits); break; - case pb::FieldDescriptor::CPPTYPE_FLOAT: { - uint32_t b = static_cast(bits); - float f; - std::memcpy(&f, &b, 4); - r->SetFloat(msg, fd, f); - break; - } - case pb::FieldDescriptor::CPPTYPE_DOUBLE: { - double d; - std::memcpy(&d, &bits, 8); - r->SetDouble(msg, fd, d); - break; - } case pb::FieldDescriptor::CPPTYPE_ENUM: - r->SetEnumValue(msg, fd, static_cast(bits)); + r->SetEnumValue(msg, fd, static_cast(bits)); break; default: break; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 7ea43f1..b1a50c3 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -64,7 +64,7 @@ endif() if(TARGET protowire_sbe) set(_sbe_test_srcs "") foreach(f sbe_codec_test sbe_convert_test sbe_navigation_test sbe_stream_test - sbe_template_id_test sbe_header_bounds_test) + sbe_template_id_test sbe_header_bounds_test sbe_encoding_range_test) if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${f}.cc) list(APPEND _sbe_test_srcs ${f}.cc) endif() diff --git a/test/sbe_encoding_range_test.cc b/test/sbe_encoding_range_test.cc new file mode 100644 index 0000000..337d9c9 --- /dev/null +++ b/test/sbe_encoding_range_test.cc @@ -0,0 +1,544 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 TrendVidia, LLC. +// +// An (sbe.encoding) primitive narrows a numeric field, and a value outside +// either range is refused rather than cut to its low bits (draft -01 § SBE +// Annotations, "Emitters MUST reject values that fall outside the narrowed +// type's range"). Ported from protowire-go's +// encoding/sbe/encoding_range_test.go (protowire-go#252). + +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "protowire/sbe.h" +#include "sbe_files.h" + +namespace { + +namespace pb = google::protobuf; +using protowire::sbe::Codec; +using protowire_test::SbeFiles; + +constexpr int64_t kMaxI32 = std::numeric_limits::max(); +constexpr int64_t kMinI32 = std::numeric_limits::min(); +constexpr int64_t kMaxI64 = std::numeric_limits::max(); +constexpr uint64_t kMaxU32 = std::numeric_limits::max(); +constexpr double kMaxF32 = std::numeric_limits::max(); + +bool RangeFile(SbeFiles* files, const std::string& body) { + return files->Compile( + {{"bounds.proto", + "syntax = \"proto3\";\npackage bounds.v1;\nimport \"sbe/annotations.proto\";\noption " + "(sbe.schema_id) = 1;\noption (sbe.version) = 0;\n" + + body + "\n"}}); +} + +// RangeCodec binds one message M whose only field v is declared by decl. +struct RangeCodec { + SbeFiles files; + std::unique_ptr codec; + const pb::Descriptor* md = nullptr; + const pb::FieldDescriptor* f = nullptr; + + bool Bind(const std::string& decl, std::string* err) { + if (!RangeFile( + &files, + "enum K { K0 = 0; } message M { option (sbe.template_id) = 1; " + decl + "; }")) { + *err = files.error(); + return false; + } + auto c = Codec::New(files.files()); + if (!c.ok()) { + *err = c.status().ToString(); + return false; + } + codec = std::make_unique(std::move(c).consume()); + md = files.Message("bounds.v1.M"); + f = md->FindFieldByName("v"); + return true; + } +}; + +// An integer value of the kind a field holds, set and read through +// reflection, and printed as Go's %v prints it. +struct IntVal { + enum Kind { kI32, kI64, kU32, kU64, kEnum } kind; + int64_t i = 0; + uint64_t u = 0; + + void Set(pb::Message* m, const pb::FieldDescriptor* fd) const { + const pb::Reflection* r = m->GetReflection(); + switch (kind) { + case kI32: + r->SetInt32(m, fd, static_cast(i)); + break; + case kI64: + r->SetInt64(m, fd, i); + break; + case kU32: + r->SetUInt32(m, fd, static_cast(u)); + break; + case kU64: + r->SetUInt64(m, fd, u); + break; + case kEnum: + r->SetEnumValue(m, fd, static_cast(i)); + break; + } + } + std::string Get(const pb::Message& m, const pb::FieldDescriptor* fd) const { + const pb::Reflection* r = m.GetReflection(); + switch (kind) { + case kI32: + return std::to_string(r->GetInt32(m, fd)); + case kI64: + return std::to_string(r->GetInt64(m, fd)); + case kU32: + return std::to_string(r->GetUInt32(m, fd)); + case kU64: + return std::to_string(r->GetUInt64(m, fd)); + case kEnum: + return std::to_string(r->GetEnumValue(m, fd)); + } + return ""; + } + std::string Str() const { + return kind == kU32 || kind == kU64 ? std::to_string(u) : std::to_string(i); + } +}; + +IntVal I32(int64_t v) { + return {IntVal::kI32, v, 0}; +} +IntVal I64(int64_t v) { + return {IntVal::kI64, v, 0}; +} +IntVal U32(uint64_t v) { + return {IntVal::kU32, 0, v}; +} +IntVal U64(uint64_t v) { + return {IntVal::kU64, 0, v}; +} +IntVal Enum(int64_t v) { + return {IntVal::kEnum, v, 0}; +} + +// TestMarshalEncodingRange: before the fix Marshal kept the low bits, so int8 +// 300 was written as 44 and read back as 44 with no error. Every refused +// value is paired with the value at the bound, which must round-trip exactly. +TEST(SbeEncodingRange, Marshal) { + struct Row { + std::string decl; + std::vector ok; + std::vector refuse; + std::string want; // the refusal's range, as the error states it + }; + const std::vector rows = { + {R"(int32 v = 1 [(sbe.encoding) = "int8"])", + {I32(127), I32(-128)}, + {I32(128), I32(-129), I32(300)}, + "int8, [-128, 127]"}, + {R"(int32 v = 1 [(sbe.encoding) = "int16"])", + {I32(32767), I32(-32768)}, + {I32(32768), I32(-32769)}, + "int16, [-32768, 32767]"}, + {R"(int64 v = 1 [(sbe.encoding) = "int32"])", + {I64(kMaxI32), I64(kMinI32)}, + {I64(kMaxI32 + 1), I64(kMinI32 - 1)}, + "int32, [-2147483648, 2147483647]"}, + {R"(uint64 v = 1 [(sbe.encoding) = "int64"])", + {U64(kMaxI64)}, + {U64(uint64_t{kMaxI64} + 1)}, + "int64, [-9223372036854775808, 9223372036854775807]"}, + {R"(uint32 v = 1 [(sbe.encoding) = "uint8"])", {U32(255)}, {U32(256)}, "uint8, [0, 255]"}, + {R"(uint32 v = 1 [(sbe.encoding) = "uint16"])", + {U32(65535)}, + {U32(65536), U32(70000)}, + "uint16, [0, 65535]"}, + {R"(uint64 v = 1 [(sbe.encoding) = "uint32"])", + {U64(kMaxU32)}, + {U64(kMaxU32 + 1)}, + "uint32, [0, 4294967295]"}, + {R"(int64 v = 1 [(sbe.encoding) = "uint64"])", + {I64(0), I64(kMaxI64)}, + {I64(-1)}, + "uint64, [0, 18446744073709551615]"}, + {R"(sint32 v = 1 [(sbe.encoding) = "uint8"])", + {I32(0), I32(255)}, + {I32(-1), I32(256)}, + "uint8, [0, 255]"}, + {R"(uint64 v = 1 [(sbe.encoding) = "int8"])", {U64(127)}, {U64(128)}, "int8, [-128, 127]"}, + // An enum with no annotation is laid out as uint8. + {"K v = 1", {Enum(0), Enum(255)}, {Enum(256), Enum(-1)}, "uint8, [0, 255]"}, + {R"(K v = 1 [(sbe.encoding) = "int16"])", + {Enum(-32768), Enum(32767)}, + {Enum(32768)}, + "int16, [-32768, 32767]"}, + }; + for (const Row& row : rows) { + SCOPED_TRACE(row.decl); + RangeCodec rc; + std::string err; + if (!rc.Bind(row.decl, &err)) { + ADD_FAILURE() << err; + continue; + } + for (const IntVal& v : row.ok) { + auto msg = rc.files.New(rc.md); + v.Set(msg.get(), rc.f); + auto data = rc.codec->Marshal(*msg); + if (!data.ok()) { + ADD_FAILURE() << "value " << v.Str() << ": " << data.status().ToString(); + continue; + } + auto back = rc.codec->UnmarshalDescriptor(*data, rc.md); + if (!back.ok()) { + ADD_FAILURE() << "value " << v.Str() << ": " << back.status().ToString(); + continue; + } + EXPECT_EQ(v.Get(**back, rc.f), v.Str()); + } + for (const IntVal& v : row.refuse) { + auto msg = rc.files.New(rc.md); + v.Set(msg.get(), rc.f); + auto data = rc.codec->Marshal(*msg); + if (data.ok()) { + ADD_FAILURE() << "value " << v.Str() << " was written"; + continue; + } + EXPECT_EQ( + data.status().message(), + "sbe: field bounds.v1.M.v: value " + v.Str() + " is outside the range of " + row.want); + } + } +} + +// TestMarshalFloatRange: a double narrowed to float must be within ±max +// float. The infinities and NaN are float values and pass; a finite double +// past the bound is refused even when it would round down to it. +TEST(SbeEncodingRange, MarshalFloat) { + RangeCodec rc; + std::string err; + ASSERT_TRUE(rc.Bind(R"(double v = 1 [(sbe.encoding) = "float"])", &err)) << err; + const double past = std::nextafter(kMaxF32, std::numeric_limits::infinity()); + const double inf = std::numeric_limits::infinity(); + for (double v : {kMaxF32, -kMaxF32, inf, -inf, std::nan(""), 0.1}) { + SCOPED_TRACE(v); + auto msg = rc.files.New(rc.md); + msg->GetReflection()->SetDouble(msg.get(), rc.f, v); + auto data = rc.codec->Marshal(*msg); + ASSERT_TRUE(data.ok()) << data.status().ToString(); + auto back = rc.codec->UnmarshalDescriptor(*data, rc.md); + ASSERT_TRUE(back.ok()) << back.status().ToString(); + const double got = (*back)->GetReflection()->GetDouble(**back, rc.f); + if (std::isnan(v)) { + EXPECT_TRUE(std::isnan(got)); + } else { + EXPECT_EQ(got, static_cast(static_cast(v))); + } + } + for (double v : {past, -past, 1e39}) { + SCOPED_TRACE(v); + auto msg = rc.files.New(rc.md); + msg->GetReflection()->SetDouble(msg.get(), rc.f, v); + auto data = rc.codec->Marshal(*msg); + ASSERT_FALSE(data.ok()); + EXPECT_NE(data.status().message().find("sbe: field bounds.v1.M.v: value"), std::string::npos); + EXPECT_NE(data.status().message().find("is outside the range of float"), std::string::npos); + } + // The text is Go's %g: 1e39 and ±3.4028234663852886e+38. + auto msg = rc.files.New(rc.md); + msg->GetReflection()->SetDouble(msg.get(), rc.f, 1e39); + EXPECT_EQ(rc.codec->Marshal(*msg).status().message(), + "sbe: field bounds.v1.M.v: value 1e+39 is outside the range of float, " + "±3.4028234663852886e+38"); +} + +// TestMarshalEncodingRangeNested: the refusal holds in a composite and in a +// group entry, and names the field there. +TEST(SbeEncodingRange, MarshalNested) { + SbeFiles files; + ASSERT_TRUE(RangeFile(&files, + R"(message C { int32 c = 1 [(sbe.encoding) = "int8"]; } +message E { uint32 e = 1 [(sbe.encoding) = "uint8"]; } +message M { option (sbe.template_id) = 1; C comp = 1; repeated E grp = 2; })")) + << files.error(); + auto codec = Codec::New(files.files()); + ASSERT_TRUE(codec.ok()) << codec.status().ToString(); + const pb::Descriptor* md = files.Message("bounds.v1.M"); + + const auto build = [&](int32_t c, uint32_t e) { + auto msg = files.New(md); + const pb::Reflection* r = msg->GetReflection(); + pb::Message* comp = r->MutableMessage(msg.get(), md->FindFieldByName("comp")); + comp->GetReflection()->SetInt32(comp, comp->GetDescriptor()->FindFieldByName("c"), c); + pb::Message* el = r->AddMessage(msg.get(), md->FindFieldByName("grp")); + el->GetReflection()->SetUInt32(el, el->GetDescriptor()->FindFieldByName("e"), e); + return msg; + }; + EXPECT_TRUE(codec->Marshal(*build(127, 255)).ok()); + EXPECT_EQ(codec->Marshal(*build(128, 255)).status().message(), + "sbe: field bounds.v1.C.c: value 128 is outside the range of int8, [-128, 127]"); + EXPECT_EQ(codec->Marshal(*build(127, 256)).status().message(), + "sbe: field bounds.v1.E.e: value 256 is outside the range of uint8, [0, 255]"); +} + +uint64_t DoubleBits(double d) { + uint64_t b = 0; + std::memcpy(&b, &d, 8); + return b; +} + +// TestUnmarshalEncodingRange: a primitive whose values the field cannot all +// hold is refused on read when the value does not fit, rather than cut to +// the field's low bits. Each refusal is paired with the wire value at the +// bound. The bytes are what another emitter could write; this codec's +// Marshal never does. +TEST(SbeEncodingRange, Unmarshal) { + struct Row { + std::string decl; + std::vector ok; // primitive bits, little-endian in the block + std::vector refuse; + }; + const auto neg = [](int64_t v) { return static_cast(v); }; + const std::vector rows = { + {R"(int32 v = 1 [(sbe.encoding) = "int64"])", + {kMaxI32, neg(kMinI32)}, + {kMaxI32 + 1, neg(kMinI32 - 1)}}, + {R"(int32 v = 1 [(sbe.encoding) = "uint32"])", {kMaxI32}, {kMaxI32 + 1, kMaxU32}}, + {R"(uint32 v = 1 [(sbe.encoding) = "int8"])", {0x7f, 0}, {0xff, 0x80}}, + {R"(uint32 v = 1 [(sbe.encoding) = "uint64"])", {kMaxU32}, {kMaxU32 + 1}}, + {R"(uint64 v = 1 [(sbe.encoding) = "int64"])", {kMaxI64}, {neg(-1)}}, + {R"(int64 v = 1 [(sbe.encoding) = "uint64"])", {kMaxI64}, {uint64_t{kMaxI64} + 1}}, + {R"(K v = 1 [(sbe.encoding) = "uint32"])", {kMaxI32}, {kMaxI32 + 1}}, + {R"(float v = 1 [(sbe.encoding) = "double"])", + {DoubleBits(kMaxF32), DoubleBits(-std::numeric_limits::infinity())}, + {DoubleBits(1e39), DoubleBits(-1e39)}}, + }; + for (const Row& row : rows) { + SCOPED_TRACE(row.decl); + RangeCodec rc; + std::string err; + if (!rc.Bind(row.decl, &err)) { + ADD_FAILURE() << err; + continue; + } + auto empty = rc.files.New(rc.md); + auto data = rc.codec->Marshal(*empty); + if (!data.ok()) { + ADD_FAILURE() << data.status().ToString(); + continue; + } + const size_t size = data->size() - 8; + const auto put = [&](uint64_t bits) { + std::vector b = *data; + for (size_t i = 0; i < size; ++i) b[8 + i] = static_cast(bits >> (8 * i)); + return b; + }; + for (uint64_t bits : row.ok) { + SCOPED_TRACE(bits); + auto back = rc.codec->UnmarshalDescriptor(put(bits), rc.md); + if (!back.ok()) { + ADD_FAILURE() << back.status().ToString(); + continue; + } + // What came back is what Marshal writes again. + auto again = rc.codec->Marshal(**back); + if (!again.ok()) { + ADD_FAILURE() << again.status().ToString(); + continue; + } + EXPECT_EQ(*again, put(bits)); + } + for (uint64_t bits : row.refuse) { + SCOPED_TRACE(bits); + auto back = rc.codec->UnmarshalDescriptor(put(bits), rc.md); + if (back.ok()) { + ADD_FAILURE() << "decoded"; + continue; + } + EXPECT_NE(back.status().message().find("sbe: field bounds.v1.M.v: wire value"), + std::string::npos) + << back.status().message(); + EXPECT_NE(back.status().message().find("does not fit a"), std::string::npos) + << back.status().message(); + } + } +} + +// The full text of each refusal on read, as protowire-go writes it. +TEST(SbeEncodingRange, UnmarshalText) { + struct Row { + std::string decl; + uint64_t bits; + std::string want; + }; + const std::vector rows = { + {R"(uint32 v = 1 [(sbe.encoding) = "int8"])", + 0xff, + "sbe: field bounds.v1.M.v: wire value -1 (int8) does not fit a uint32 field, [0, " + "4294967295]"}, + {R"(int32 v = 1 [(sbe.encoding) = "uint32"])", + kMaxU32, + "sbe: field bounds.v1.M.v: wire value 4294967295 (uint32) does not fit a int32 field, " + "[-2147483648, 2147483647]"}, + {R"(K v = 1 [(sbe.encoding) = "uint32"])", + kMaxI32 + 1, + "sbe: field bounds.v1.M.v: wire value 2147483648 (uint32) does not fit a enum field, " + "[-2147483648, 2147483647]"}, + {R"(float v = 1 [(sbe.encoding) = "double"])", + DoubleBits(-1e39), + "sbe: field bounds.v1.M.v: wire value -1e+39 does not fit a float field, " + "±3.4028234663852886e+38"}, + }; + for (const Row& row : rows) { + SCOPED_TRACE(row.decl); + RangeCodec rc; + std::string err; + if (!rc.Bind(row.decl, &err)) { + ADD_FAILURE() << err; + continue; + } + auto data = rc.codec->Marshal(*rc.files.New(rc.md)); + if (!data.ok()) { + ADD_FAILURE() << data.status().ToString(); + continue; + } + for (size_t i = 0; i + 8 < data->size(); ++i) { + (*data)[8 + i] = static_cast(row.bits >> (8 * i)); + } + EXPECT_EQ(rc.codec->UnmarshalDescriptor(*data, rc.md).status().message(), row.want); + } +} + +// TestEncodingKindBind: (sbe.encoding) narrows a numeric field. A string or +// bytes field, and a crossing between integers and floating point, are +// refused when the codec is built. +TEST(SbeEncodingRange, KindBind) { + const std::vector> cases = { + {R"(int32 v = 1 [(sbe.encoding) = "float"])", + R"(field bounds.v1.M.v: (sbe.encoding) = "float" is a floating-point primitive and the field is int32)"}, + {R"(K v = 1 [(sbe.encoding) = "double"])", + R"(field bounds.v1.M.v: (sbe.encoding) = "double" is a floating-point primitive and the field is enum)"}, + {R"(bool v = 1 [(sbe.encoding) = "float"])", + R"(field bounds.v1.M.v: (sbe.encoding) = "float" is a floating-point primitive and the field is bool)"}, + {R"(double v = 1 [(sbe.encoding) = "int32"])", + R"(field bounds.v1.M.v: (sbe.encoding) = "int32" is an integer primitive and the field is double)"}, + {R"(float v = 1 [(sbe.encoding) = "uint8"])", + R"(field bounds.v1.M.v: (sbe.encoding) = "uint8" is an integer primitive and the field is float)"}, + {R"(string v = 1 [(sbe.length) = 4, (sbe.encoding) = "uint8"])", + R"(field bounds.v1.M.v: (sbe.encoding) = "uint8" on a string field)"}, + {R"(bytes v = 1 [(sbe.length) = 4, (sbe.encoding) = "int32"])", + R"(field bounds.v1.M.v: (sbe.encoding) = "int32" on a bytes field)"}, + {R"(double v = 1 [(sbe.encoding) = "char"])", + R"(field bounds.v1.M.v: unknown encoding "char")"}, + }; + for (const auto& [decl, want] : cases) { + SCOPED_TRACE(decl); + SbeFiles files; + if (!RangeFile( + &files, + "enum K { K0 = 0; } message M { option (sbe.template_id) = 1; " + decl + "; }")) { + ADD_FAILURE() << files.error(); + continue; + } + auto codec = Codec::New(files.files()); + if (codec.ok()) { + ADD_FAILURE() << "bound"; + continue; + } + EXPECT_NE(codec.status().message().find(want), std::string::npos) << codec.status().message(); + } +} + +// TestEncodingMatrix pins every pairing of an integer, bool, enum or +// floating field with every (sbe.encoding): it is refused when the codec is +// built, or the value 1 round-trips. +TEST(SbeEncodingRange, Matrix) { + const std::vector kinds = { + "int32", "int64", "uint32", "uint64", "sint32", "fixed64", "float", "double", "bool", "K"}; + const std::vector encs = { + "int8", "int16", "int32", "int64", "uint8", "uint16", "uint32", "uint64", "float", "double"}; + int bound = 0; + int refused = 0; + for (const std::string& k : kinds) { + for (const std::string& e : encs) { + SCOPED_TRACE(k + "/" + e); + SbeFiles files; + if (!RangeFile(&files, + "enum K { K0 = 0; K1 = 1; } message M { option (sbe.template_id) = " + "1; " + + k + " v = 1 [(sbe.encoding) = \"" + e + "\"]; }")) { + ADD_FAILURE() << files.error(); + continue; + } + auto codec = Codec::New(files.files()); + const bool float_kind = k == "float" || k == "double"; + const bool float_enc = e == "float" || e == "double"; + if (float_kind != float_enc) { + EXPECT_FALSE(codec.ok()); + ++refused; + continue; + } + if (!codec.ok()) { + ADD_FAILURE() << codec.status().ToString(); + continue; + } + ++bound; + const pb::Descriptor* md = files.Message("bounds.v1.M"); + const pb::FieldDescriptor* f = md->FindFieldByName("v"); + auto msg = files.New(md); + const pb::Reflection* r = msg->GetReflection(); + switch (f->cpp_type()) { + case pb::FieldDescriptor::CPPTYPE_INT32: + r->SetInt32(msg.get(), f, 1); + break; + case pb::FieldDescriptor::CPPTYPE_INT64: + r->SetInt64(msg.get(), f, 1); + break; + case pb::FieldDescriptor::CPPTYPE_UINT32: + r->SetUInt32(msg.get(), f, 1); + break; + case pb::FieldDescriptor::CPPTYPE_UINT64: + r->SetUInt64(msg.get(), f, 1); + break; + case pb::FieldDescriptor::CPPTYPE_FLOAT: + r->SetFloat(msg.get(), f, 1); + break; + case pb::FieldDescriptor::CPPTYPE_DOUBLE: + r->SetDouble(msg.get(), f, 1); + break; + case pb::FieldDescriptor::CPPTYPE_BOOL: + r->SetBool(msg.get(), f, true); + break; + default: + r->SetEnumValue(msg.get(), f, 1); + break; + } + auto data = codec->Marshal(*msg); + if (!data.ok()) { + ADD_FAILURE() << data.status().ToString(); + continue; + } + auto back = codec->UnmarshalDescriptor(*data, md); + if (!back.ok()) { + ADD_FAILURE() << back.status().ToString(); + continue; + } + // What came back serializes as what was set, so the value survived the + // round trip whatever its kind. + EXPECT_EQ(msg->SerializeAsString(), (*back)->SerializeAsString()); + } + } + EXPECT_EQ(bound, 68); + EXPECT_EQ(refused, 32); +} + +} // namespace