sbe: refuse sizes and ids past SBE's 16-bit header fields (#132)

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: <full name> 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::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).
 - **A schema that redefines a well-known type with the wrong shape is refused at bind time** (draft -01 §wkt-shapes, protowire#487, protowire-go#218). This narrows what binds. A message named, say, `pxf.Secret` or `google.protobuf.Timestamp` without the fields the codec reads now gives a violation of the new kind `ViolationKind::kWellKnownShape`. Copies of the canonical schemas are unaffected, and field numbers are not checked. The check also reaches a schema's own file at a `google/protobuf/` path, which the closure walk used to skip.

src/sbe/codec.cc

@@ -5,6 +5,7 @@
 #include <string>
 #include <utility>
 
+#include "header_bounds.h"
 #include "protowire/sbe/annotations.h"
 
 namespace protowire::sbe {
@@ -31,6 +32,16 @@ StatusOr<Codec> Codec::New(std::vector<const pb::FileDescriptor*> files, CodecOp
                            " missing (sbe.schema_id) option");
     }
     auto version = GetFileUint32Option(fd, kExtVersion).value_or(0);
+    if (*schema > static_cast<uint32_t>(kMaxUint16)) {
+      return Status::Error("sbe: file " + std::string(fd->name()) +
+                           ": (sbe.schema_id) = " + std::to_string(*schema) + " is past " +
+                           std::to_string(kMaxUint16) + ", the largest schemaId SBE can state");
+    }
+    if (version > static_cast<uint32_t>(kMaxUint16)) {
+      return Status::Error("sbe: file " + std::string(fd->name()) +
+                           ": (sbe.version) = " + std::to_string(version) + " is past " +
+                           std::to_string(kMaxUint16) + ", the largest version SBE can state");
+    }
     for (int i = 0; i < fd->message_type_count(); ++i) {
       Status st = c.RegisterMessage(
           fd->message_type(i), static_cast<uint16_t>(*schema), static_cast<uint16_t>(version));

src/sbe/header_bounds.h

@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2026 TrendVidia, LLC.
+#pragma once
+
+namespace protowire::sbe {
+
+// kMaxUint16 is the largest value of every SBE header field: blockLength,
+// templateId, schemaId and version in the message header, blockLength and
+// numInGroup in a group header. A schema or a message needing a larger value
+// cannot be written: narrowing it would write a header that disagrees with
+// the bytes after it (protowire-go#250).
+inline constexpr int kMaxUint16 = 65535;
+
+}  // namespace protowire::sbe

src/sbe/marshal.cc

@@ -5,8 +5,11 @@
 #include "protowire/sbe.h"
 
 #include <cstring>
+#include <string>
 #include <vector>
 
+#include "header_bounds.h"
+
 namespace protowire::sbe {
 
 namespace pb = google::protobuf;
@@ -156,6 +159,13 @@ StatusOr<std::vector<uint8_t>> Codec::Marshal(const pb::Message& msg) const {
   for (const GroupTemplate& gt : tmpl->groups) {
     const pb::Reflection* r = msg.GetReflection();
     int n = r->FieldSize(msg, gt.fd);
+    // numInGroup is a uint16: a larger count would be written as its low 16
+    // bits, and a reader would take the entries past them for trailing bytes.
+    if (n > kMaxUint16) {
+      return Status::Error("sbe: field " + std::string(gt.fd->full_name()) + " has " +
+                           std::to_string(n) + " entries, past " + std::to_string(kMaxUint16) +
+                           ", the largest numInGroup SBE can state");
+    }
     size_t header_off = buf.size();
     buf.resize(header_off + 4, 0);
     WriteU16(buf.data() + header_off + 0, gt.block_length);

src/sbe/template.cc

@@ -7,6 +7,8 @@
 #include <utility>
 #include <vector>
 
+#include "header_bounds.h"
+#include "protowire/detail/go_quote.h"
 #include "protowire/sbe/annotations.h"
 
 namespace protowire::sbe {
@@ -29,14 +31,36 @@ std::vector<const pb::FieldDescriptor*> SortedFields(const pb::Descriptor* md) {
   return out;
 }
 
+// "pkg.Msg.field", as Go's "%s.%s" of a message's full name and a field's
+// name writes it.
+std::string FieldPath(const pb::Descriptor* md, const pb::FieldDescriptor* fd) {
+  return std::string(md->full_name()) + "." + std::string(fd->name());
+}
+
+Status Wrap(const std::string& prefix, const Status& st) {
+  return Status::Error(prefix + ": " + st.message());
+}
+
+// EndOfField returns the offset after a field of size bytes placed at
+// offset, refusing a field that ends past what a uint16 blockLength can
+// state. Every offset in the block is then a uint16 as well.
+StatusOr<int> EndOfField(int offset, uint16_t size) {
+  const int end = offset + size;
+  if (end > kMaxUint16) {
+    return Status::Error("ends at byte " + std::to_string(end) + " of its block, past " +
+                         std::to_string(kMaxUint16) + ", the largest blockLength SBE can state");
+  }
+  return end;
+}
+
 StatusOr<std::pair<std::string, uint16_t>> FieldEncodingSize(const pb::FieldDescriptor* fd) {
   if (auto enc = GetFieldStringOption(fd, kExtEncoding); enc.has_value()) {
     const std::string& e = *enc;
     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});
     if (e == kEncInt64 || e == kEncUint64 || e == kEncDouble) return std::make_pair(e, uint16_t{8});
-    return Status::Error("unknown encoding \"" + e + "\"");
+    return Status::Error("unknown encoding " + detail::GoQuote(e));
   }
   switch (fd->cpp_type()) {
     case pb::FieldDescriptor::CPPTYPE_BOOL:
@@ -58,12 +82,18 @@ StatusOr<std::pair<std::string, uint16_t>> FieldEncodingSize(const pb::FieldDesc
     case pb::FieldDescriptor::CPPTYPE_STRING: {
       auto len = GetFieldUint32Option(fd, kExtLength);
       if (!len.has_value()) {
-        return Status::Error("string/bytes field requires (sbe.length)");
+        return Status::Error(std::string(fd->type_name()) +
+                             " field requires (sbe.length) annotation");
+      }
+      if (*len > static_cast<uint32_t>(kMaxUint16)) {
+        return Status::Error("(sbe.length) = " + std::to_string(*len) + " is past " +
+                             std::to_string(kMaxUint16) +
+                             ", the largest blockLength SBE can state");
       }
       return std::make_pair(std::string(kEncChar), static_cast<uint16_t>(*len));
     }
     default:
-      return Status::Error("unsupported proto type for SBE");
+      return Status::Error("unsupported proto kind " + std::string(fd->type_name()));
   }
 }
 
@@ -73,71 +103,88 @@ StatusOr<std::vector<FieldTemplate>> BuildCompositeFields(const pb::Descriptor*
 StatusOr<std::vector<FieldTemplate>> BuildCompositeFields(const pb::Descriptor* md,
                                                           uint16_t* total_size) {
   std::vector<FieldTemplate> out;
-  uint16_t offset = 0;
+  int offset = 0;
   for (const pb::FieldDescriptor* fd : SortedFields(md)) {
+    const std::string where = "composite field " + FieldPath(md, fd);
     if (fd->is_repeated() || fd->is_map()) {
-      return Status::Error("composite contains list/map field");
+      return Status::Error("composite " + std::string(md->full_name()) +
+                           " contains list/map field " + std::string(fd->name()));
     }
     if (fd->cpp_type() == pb::FieldDescriptor::CPPTYPE_MESSAGE) {
       uint16_t sub_size = 0;
       auto subs = BuildCompositeFields(fd->message_type(), &sub_size);
       if (!subs.ok()) return subs.status();
+      auto end = EndOfField(offset, sub_size);
+      if (!end.ok()) return Wrap(where, end.status());
       FieldTemplate ft;
       ft.fd = fd;
-      ft.offset = offset;
+      ft.offset = static_cast<uint16_t>(offset);
       ft.size = sub_size;
       ft.composite = std::move(subs).consume();
       out.push_back(std::move(ft));
-      offset += sub_size;
+      offset = *end;
       continue;
     }
     auto es = FieldEncodingSize(fd);
-    if (!es.ok()) return es.status();
+    if (!es.ok()) return Wrap(where, es.status());
+    auto end = EndOfField(offset, es->second);
+    if (!end.ok()) return Wrap(where, end.status());
     FieldTemplate ft;
     ft.fd = fd;
-    ft.offset = offset;
+    ft.offset = static_cast<uint16_t>(offset);
     ft.encoding = es->first;
     ft.size = es->second;
     out.push_back(ft);
-    offset += ft.size;
+    offset = *end;
   }
-  *total_size = offset;
+  *total_size = static_cast<uint16_t>(offset);
   return out;
 }
 
 StatusOr<GroupTemplate> BuildGroupTemplate(const pb::FieldDescriptor* fd) {
   const pb::Descriptor* md = fd->message_type();
   GroupTemplate gt;
   gt.fd = fd;
-  uint16_t offset = 0;
+  int offset = 0;
   for (const pb::FieldDescriptor* f : SortedFields(md)) {
-    if (f->is_map() || f->is_repeated()) {
-      return Status::Error("group field cannot be repeated/map");
+    if (f->is_map()) {
+      return Status::Error("sbe: map field in group " + std::string(md->full_name()) +
+                           " not supported");
+    }
+    if (f->is_repeated()) {
+      return Status::Error("sbe: nested repeated field in group " + std::string(md->full_name()) +
+                           " not supported");
     }
     if (f->cpp_type() == pb::FieldDescriptor::CPPTYPE_MESSAGE) {
+      const std::string where = "sbe: composite in group " + FieldPath(md, f);
       uint16_t sub_size = 0;
       auto subs = BuildCompositeFields(f->message_type(), &sub_size);
-      if (!subs.ok()) return subs.status();
+      if (!subs.ok()) return Wrap(where, subs.status());
+      auto end = EndOfField(offset, sub_size);
+      if (!end.ok()) return Wrap(where, end.status());
       FieldTemplate ft;
       ft.fd = f;
-      ft.offset = offset;
+      ft.offset = static_cast<uint16_t>(offset);
       ft.size = sub_size;
       ft.composite = std::move(subs).consume();
       gt.fields.push_back(std::move(ft));
-      offset += sub_size;
+      offset = *end;
       continue;
     }
+    const std::string where = "sbe: group field " + FieldPath(md, f);
     auto es = FieldEncodingSize(f);
-    if (!es.ok()) return es.status();
+    if (!es.ok()) return Wrap(where, es.status());
+    auto end = EndOfField(offset, es->second);
+    if (!end.ok()) return Wrap(where, end.status());
     FieldTemplate ft;
     ft.fd = f;
-    ft.offset = offset;
+    ft.offset = static_cast<uint16_t>(offset);
     ft.encoding = es->first;
     ft.size = es->second;
     gt.fields.push_back(ft);
-    offset += ft.size;
+    offset = *end;
   }
-  gt.block_length = offset;
+  gt.block_length = static_cast<uint16_t>(offset);
   return gt;
 }
 
@@ -146,19 +193,25 @@ StatusOr<GroupTemplate> BuildGroupTemplate(const pb::FieldDescriptor* fd) {
 StatusOr<MessageTemplate> BuildTemplate(const pb::Descriptor* md,
                                         uint16_t schema_id,
                                         uint16_t version) {
+  const std::string name(md->full_name());
   auto tid = GetMessageUint32Option(md, kExtTemplateID);
   if (!tid.has_value()) {
-    return Status::Error(std::string(md->full_name()) + " missing (sbe.template_id)");
+    return Status::Error("sbe: message " + name + " missing (sbe.template_id)");
+  }
+  if (*tid > static_cast<uint32_t>(kMaxUint16)) {
+    return Status::Error("sbe: message " + name + ": (sbe.template_id) = " + std::to_string(*tid) +
+                         " is past " + std::to_string(kMaxUint16) +
+                         ", the largest templateId SBE can state");
   }
   MessageTemplate tmpl;
   tmpl.template_id = static_cast<uint16_t>(*tid);
   tmpl.schema_id = schema_id;
   tmpl.version = version;
 
-  uint16_t offset = 0;
+  int offset = 0;
   for (const pb::FieldDescriptor* fd : SortedFields(md)) {
     if (fd->is_map()) {
-      return Status::Error("map field not supported in SBE");
+      return Status::Error("sbe: map field " + FieldPath(md, fd) + " not supported");
     }
     if (fd->is_repeated() && fd->cpp_type() == pb::FieldDescriptor::CPPTYPE_MESSAGE) {
       auto gt = BuildGroupTemplate(fd);
@@ -167,32 +220,39 @@ StatusOr<MessageTemplate> BuildTemplate(const pb::Descriptor* md,
       continue;
     }
     if (fd->is_repeated()) {
-      return Status::Error("repeated scalar not supported in SBE");
+      return Status::Error("sbe: repeated scalar field " + FieldPath(md, fd) +
+                           " not supported; wrap in a message");
     }
     if (fd->cpp_type() == pb::FieldDescriptor::CPPTYPE_MESSAGE) {
+      const std::string where = "sbe: composite " + FieldPath(md, fd);
       uint16_t sub_size = 0;
       auto subs = BuildCompositeFields(fd->message_type(), &sub_size);
-      if (!subs.ok()) return subs.status();
+      if (!subs.ok()) return Wrap(where, subs.status());
+      auto end = EndOfField(offset, sub_size);
+      if (!end.ok()) return Wrap(where, end.status());
       FieldTemplate ft;
       ft.fd = fd;
-      ft.offset = offset;
+      ft.offset = static_cast<uint16_t>(offset);
       ft.size = sub_size;
       ft.composite = std::move(subs).consume();
       tmpl.fields.push_back(std::move(ft));
-      offset += sub_size;
+      offset = *end;
       continue;
     }
+    const std::string where = "sbe: field " + FieldPath(md, fd);
     auto es = FieldEncodingSize(fd);
-    if (!es.ok()) return es.status();
+    if (!es.ok()) return Wrap(where, es.status());
+    auto end = EndOfField(offset, es->second);
+    if (!end.ok()) return Wrap(where, end.status());
     FieldTemplate ft;
     ft.fd = fd;
-    ft.offset = offset;
+    ft.offset = static_cast<uint16_t>(offset);
     ft.encoding = es->first;
     ft.size = es->second;
     tmpl.fields.push_back(ft);
-    offset += ft.size;
+    offset = *end;
   }
-  tmpl.block_length = offset;
+  tmpl.block_length = static_cast<uint16_t>(offset);
   return tmpl;
 }
 

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_template_id_test sbe_header_bounds_test)
     if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${f}.cc)
       list(APPEND _sbe_test_srcs ${f}.cc)
     endif()