sbe: tell two schemas' template ids apart (#134)

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` 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.
 

include/protowire/sbe.h

@@ -108,7 +108,17 @@ class Codec {
                         size_t pos) const;
 
   std::unordered_map<std::string, std::unique_ptr<MessageTemplate>> by_name_;
-  std::unordered_map<uint16_t, MessageTemplate*> by_id_;
+  // A registered message, found by its schema id and template id together:
+  // a template id is unique only within a schema (draft -01 § SBE
+  // Annotations), so two schemas in one codec may each use one.
+  struct ByID {
+    std::string name;
+    MessageTemplate* tmpl = nullptr;
+  };
+  static uint32_t IDKey(uint16_t schema_id, uint16_t template_id) {
+    return (static_cast<uint32_t>(schema_id) << 16U) | static_cast<uint32_t>(template_id);
+  }
+  std::unordered_map<uint32_t, ByID> by_id_;
   int max_message_size_ = kMaxMessageSize;
   int max_repeated_ = kMaxRepeatedCount;
   check::Validator* validator_ = nullptr;

src/sbe/codec.cc

@@ -49,8 +49,21 @@ Status Codec::RegisterMessage(const pb::Descriptor* md, uint16_t schema_id, uint
     auto tmpl = BuildTemplate(md, schema_id, version);
     if (!tmpl.ok()) return tmpl.status();
     auto owned = std::make_unique<MessageTemplate>(std::move(tmpl).consume());
-    by_id_[owned->template_id] = owned.get();
-    by_name_[std::string(md->full_name())] = std::move(owned);
+    // draft -01 § SBE Annotations: "sbe.template_id MUST be unique among
+    // messages within a schema". A second message with the same id would
+    // replace the first in by_id_, and NewView would then read one as the
+    // other. The same message seen again, as when a file is passed twice,
+    // is not a duplicate.
+    std::string name(md->full_name());
+    const uint32_t key = IDKey(schema_id, owned->template_id);
+    if (auto prev = by_id_.find(key); prev != by_id_.end() && prev->second.name != name) {
+      return Status::Error("sbe: messages " + prev->second.name + " and " + name +
+                           " share (sbe.template_id) = " + std::to_string(owned->template_id) +
+                           " in schema " + std::to_string(schema_id) +
+                           "; it must be unique within a schema");
+    }
+    by_id_[key] = ByID{name, owned.get()};
+    by_name_[name] = std::move(owned);
   }
   for (int i = 0; i < md->nested_type_count(); ++i) {
     Status st = RegisterMessage(md->nested_type(i), schema_id, version);
@@ -113,27 +126,28 @@ StatusOr<View> Codec::NewView(std::span<const uint8_t> data) const {
   }
   uint16_t block_length = LoadU16(data.data());
   uint16_t template_id = LoadU16(data.data() + 2);
-  auto it = by_id_.find(template_id);
-  if (it == by_id_.end()) {
-    return Status::Error("sbe: unknown template id " + std::to_string(template_id));
+  uint16_t schema_id = LoadU16(data.data() + 4);
+  // The header's schema id and template id together name the template: a
+  // template id is unique only within a schema.
+  auto found = by_id_.find(IDKey(schema_id, template_id));
+  if (found == by_id_.end()) {
+    return Status::Error("sbe: unknown template ID " + std::to_string(template_id) + " in schema " +
+                         std::to_string(schema_id));
   }
+  const MessageTemplate* tmpl = found->second.tmpl;
   // Schema evolution may push the wire block past the template's (newer
   // schemas append fields); a smaller one would underrun field reads.
-  if (block_length < it->second->block_length) {
+  if (block_length < tmpl->block_length) {
     return Status::Error("sbe: wire blockLength " + std::to_string(block_length) +
-                         " < schema blockLength " + std::to_string(it->second->block_length) +
+                         " < schema blockLength " + std::to_string(tmpl->block_length) +
                          " for template " + std::to_string(template_id));
   }
   size_t end = kHeaderSize + block_length;
   if (data.size() < end) {
     return Status::Error("sbe: data too short for root block");
   }
-  if (Status st = ValidateGroups(data, *it->second, end); !st.ok()) return st;
-  return View(data,
-              data.subspan(kHeaderSize, block_length),
-              it->second,
-              &it->second->fields,
-              &it->second->groups);
+  if (Status st = ValidateGroups(data, *tmpl, end); !st.ok()) return st;
+  return View(data, data.subspan(kHeaderSize, block_length), tmpl, &tmpl->fields, &tmpl->groups);
 }
 
 }  // namespace protowire::sbe

src/sbe/unmarshal.cc

@@ -192,6 +192,15 @@ Status Codec::UnmarshalInto(std::span<const uint8_t> data,
     return Status::Error("sbe: template ID mismatch: got " + std::to_string(template_id) +
                          ", want " + std::to_string(tmpl->template_id));
   }
+  // A template id is unique only within a schema, so the template id alone
+  // does not say the bytes are this message's: they may be another schema's
+  // message with the same id. The version is not checked.
+  uint16_t schema_id = LoadU16(data.data() + 4);
+  if (schema_id != tmpl->schema_id) {
+    return Status::Error("sbe: schema ID mismatch: got " + std::to_string(schema_id) + ", want " +
+                         std::to_string(tmpl->schema_id) + " for template " +
+                         std::to_string(tmpl->template_id));
+  }
   if (block_length < tmpl->block_length) {
     return Status::Error("sbe: wire blockLength " + std::to_string(block_length) +
                          " < schema blockLength " + std::to_string(tmpl->block_length) +

test/CMakeLists.txt

@@ -63,7 +63,8 @@ endif()
 
 if(TARGET protowire_sbe)
   set(_sbe_test_srcs "")
-  foreach(f sbe_codec_test sbe_convert_test sbe_navigation_test sbe_stream_test)
+  foreach(f sbe_codec_test sbe_convert_test sbe_navigation_test sbe_stream_test
+            sbe_template_id_test)
     if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${f}.cc)
       list(APPEND _sbe_test_srcs ${f}.cc)
     endif()

test/sbe_files.h

@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2026 TrendVidia, LLC.
+//
+// SbeFiles compiles .proto sources written at runtime, each importing
+// sbe/annotations.proto, and keeps the pool and a message factory alive for
+// the test. It mirrors the in-memory compiles of protowire-go's SBE tests.
+
+#pragma once
+
+#include <gtest/gtest.h>
+
+#include <fstream>
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include <google/protobuf/compiler/importer.h>
+#include <google/protobuf/dynamic_message.h>
+
+#include "fixture_dir.h"
+#include "protoc_compat.h"
+
+namespace protowire_test {
+
+class SbeFiles {
+ public:
+  // Compile writes each (name, source) pair into the fixture directory and
+  // imports them in order. It returns false, with the error in error(), if
+  // any fails to compile.
+  bool Compile(const std::vector<std::pair<std::string, std::string>>& srcs) {
+    tree_.MapPath("", PROTO_DIR);
+    tree_.MapPath("", WKT_PROTO_DIR);
+    tree_.MapPath("", FixtureDir());
+    for (const auto& [name, src] : srcs) {
+      std::ofstream(FixtureDir() + "/" + name) << src;
+    }
+    importer_ = std::make_unique<google::protobuf::compiler::Importer>(&tree_, &errors_);
+    for (const auto& [name, src] : srcs) {
+      const google::protobuf::FileDescriptor* fd = importer_->Import(name);
+      if (fd == nullptr) return false;
+      files_.push_back(fd);
+    }
+    factory_ = std::make_unique<google::protobuf::DynamicMessageFactory>(importer_->pool());
+    return true;
+  }
+
+  const std::vector<const google::protobuf::FileDescriptor*>& files() const { return files_; }
+  const std::string& error() const { return errors_.last_; }
+
+  const google::protobuf::Descriptor* Message(const std::string& full_name) const {
+    return importer_->pool()->FindMessageTypeByName(full_name);
+  }
+
+  std::unique_ptr<google::protobuf::Message> New(const google::protobuf::Descriptor* md) {
+    return std::unique_ptr<google::protobuf::Message>(factory_->GetPrototype(md)->New());
+  }
+
+ private:
+  class Errors : public google::protobuf::compiler::MultiFileErrorCollector {
+   public:
+    PROTOWIRE_PROTOC_RECORD_ERROR(filename, line, column, msg) {
+      last_ = std::string(filename) + ":" + std::to_string(line) + ":" + std::to_string(column) +
+              ": " + std::string(msg);
+    }
+    std::string last_;
+  };
+
+  google::protobuf::compiler::DiskSourceTree tree_;
+  Errors errors_;
+  std::unique_ptr<google::protobuf::compiler::Importer> importer_;
+  std::unique_ptr<google::protobuf::DynamicMessageFactory> factory_;
+  std::vector<const google::protobuf::FileDescriptor*> files_;
+};
+
+}  // namespace protowire_test