Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ number says nothing about any other port. See the Releases section of
- **`pb::Unmarshal` refuses a `pxf.BigFloat` whose `prec` is past the precision-suffix bound** (#108, protowire#499). That is a `prec` whose literal needs more than `max_numeric_literal_digits` digits: above 13603 bits at the default, zeros included. It is checked before the value is built. An 11-byte message at prec 2^31-1 used to decode, and writing it back widened the mantissa to 256 MiB. **This rejects messages that were accepted before.** `UnmarshalOptions::max_numeric_literal_digits` moves the bound. `pb::Marshal` still writes a hand-built value past it, because it cannot refuse yet (#93).
- **A `BigFloat` `prec` varint wider than 32 bits keeps its low 32 bits** (#106), as a protobuf reader does and protowire-go now does (protowire-go#230). It was clamped to 2^32-1, so 2^32 read as 2^32-1 bits where every other reader reads prec 0, a zero. **The same bytes now decode to a different value.**
- **A PXF `BigFloat` literal wider than 2^31-1 bits is refused** (protowire-go#231), with Go's `precision p<N> exceeds the widest mantissa big.Float can hold, 2147483647 bits`. Only a `max_numeric_literal_digits` raised past 646 million admits such a suffix. It used to build the mantissa: 256 MiB at 2^31 bits. A zero is still accepted.
- **`sbe::Codec::New` diagnoses a stale descriptor, in protowire-go's words** (#122). A file or message that carries `(sbe.schema_id)` or `(sbe.template_id)` at the pre-v1.12.0 option numbers (50100, 50200) now gets an error saying it must be recompiled. A stale message used to be skipped silently, as if it were not a template. **Error text changes:** a file with no schema id now says `sbe: file <name> missing (sbe.schema_id) option`, as Go does.
- **`sbe::Codec::Unmarshal` keeps the sign of a narrow signed encoding** (#120). An `int32` or `int64` field with `(sbe.encoding)` of `int8`, `int16` or `int32` decoded a negative value as a large positive one: -1234 in an `int16` came back as 64302. It is now sign-extended, as protowire-go and `sbe::View` read it. **Such values now decode differently.**
- **A PXF Decimal `-0` or `-0.00` keeps its sign** (#110), as protowire-go reads and writes it. It was read with `negative` unset and written back as `0`. **The PB bytes for such a literal change** (`negative` is now set). The spec's write gate found the difference.
- **`pxf::Marshal` writes a Duration past the literal's range as a block** (#116), as protowire-go does (protowire-go#226). A literal holds 64 bits of nanoseconds, about 292 years. A longer Duration was written as `<duration overflow>`, which no reader accepts. It is now `f { seconds = 9500000000 }`, which keeps the exact value, in a field, list, map or `@dataset` cell. Shorter Durations are written as before.
Expand Down
12 changes: 12 additions & 0 deletions include/protowire/sbe/annotations.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,16 @@ std::optional<uint32_t> GetFieldUint32Option(const google::protobuf::FieldDescri
std::optional<std::string> GetFieldStringOption(const google::protobuf::FieldDescriptor* fd,
int field_number);

// Before protowire v1.12.0, (sbe.schema_id) and (sbe.template_id) were 50100
// and 50200. A descriptor compiled then carries them as unknown option fields.
inline constexpr int kRetiredSchemaID = 50100;
inline constexpr int kRetiredTemplateID = 50200;

// StaleFileError and StaleMessageError are protowire-go's diagnosis for a
// descriptor that carries its option at the retired number, or empty when it
// does not. A number the descriptor's pool declares as an extension is a known
// option, not a stale one.
std::string StaleFileError(const google::protobuf::FileDescriptor* fd);
std::string StaleMessageError(const google::protobuf::Descriptor* md);

} // namespace protowire::sbe
40 changes: 40 additions & 0 deletions src/sbe/annotations.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <cstdint>
#include <span>
#include <string>
#include <vector>

#include <google/protobuf/descriptor.h>
Expand Down Expand Up @@ -110,4 +111,43 @@ std::optional<std::string> GetFieldStringOption(const pb::FieldDescriptor* fd, i
return GetStringOption(fd->options(), field_number);
}

namespace {

// CarriesRetired reports whether opts, the options of a descriptor in pool,
// has field number as an unknown field the pool does not declare.
bool CarriesRetired(const google::protobuf::DescriptorPool* pool,
const google::protobuf::Message& opts,
int number) {
// The pool's own options type, which is not the generated one when the file
// was compiled with descriptor.proto in its pool.
const google::protobuf::Descriptor* type = pool->FindMessageTypeByName(opts.GetTypeName());
if (type == nullptr) type = opts.GetDescriptor();
if (pool->FindExtensionByNumber(type, number) != nullptr) return false;
const google::protobuf::UnknownFieldSet& unknown = opts.GetReflection()->GetUnknownFields(opts);
for (int i = 0; i < unknown.field_count(); ++i) {
if (unknown.field(i).number() == number) return true;
}
return false;
}

std::string StaleText(const std::string& what, const char* option, int retired, int registered) {
return "sbe: " + what + " carries (sbe." + option + ") at retired option number " +
std::to_string(retired) + " (registered as " + std::to_string(registered) +
" since protowire v1.12.0): the descriptor predates the registered extension block and "
"must be recompiled against the current sbe/annotations.proto";
}

} // namespace

std::string StaleFileError(const google::protobuf::FileDescriptor* fd) {
if (!CarriesRetired(fd->pool(), fd->options(), kRetiredSchemaID)) return "";
return StaleText("file " + std::string(fd->name()), "schema_id", kRetiredSchemaID, kExtSchemaID);
}

std::string StaleMessageError(const google::protobuf::Descriptor* md) {
if (!CarriesRetired(md->file()->pool(), md->options(), kRetiredTemplateID)) return "";
return StaleText(
"message " + std::string(md->full_name()), "template_id", kRetiredTemplateID, kExtTemplateID);
}

} // namespace protowire::sbe
10 changes: 8 additions & 2 deletions src/sbe/codec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ StatusOr<Codec> Codec::New(std::vector<const pb::FileDescriptor*> files, CodecOp
for (const pb::FileDescriptor* fd : files) {
auto schema = GetFileUint32Option(fd, kExtSchemaID);
if (!schema.has_value()) {
return Status::Error(std::string(fd->name()) + " missing (sbe.schema_id) file option");
if (std::string stale = StaleFileError(fd); !stale.empty()) return Status::Error(stale);
return Status::Error("sbe: file " + std::string(fd->name()) +
" missing (sbe.schema_id) option");
}
auto version = GetFileUint32Option(fd, kExtVersion).value_or(0);
for (int i = 0; i < fd->message_type_count(); ++i) {
Expand All @@ -39,7 +41,11 @@ StatusOr<Codec> Codec::New(std::vector<const pb::FileDescriptor*> files, CodecOp
}

Status Codec::RegisterMessage(const pb::Descriptor* md, uint16_t schema_id, uint16_t version) {
if (GetMessageUint32Option(md, kExtTemplateID).has_value()) {
if (!GetMessageUint32Option(md, kExtTemplateID).has_value()) {
// Only a message with a template_id is registered; one that carries it at
// the retired number is a stale descriptor, not a plain message.
if (std::string stale = StaleMessageError(md); !stale.empty()) return Status::Error(stale);
} else {
auto tmpl = BuildTemplate(md, schema_id, version);
if (!tmpl.ok()) return tmpl.status();
auto owned = std::make_unique<MessageTemplate>(std::move(tmpl).consume());
Expand Down
32 changes: 0 additions & 32 deletions src/sbe/prototoxml.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
#include <vector>

#include <google/protobuf/descriptor.h>
#include <google/protobuf/descriptor.pb.h>
#include <google/protobuf/unknown_field_set.h>

#include "protowire/sbe/annotations.h"
#include "protowire/sbe/xml.h"
Expand All @@ -27,36 +25,6 @@ namespace gpb = google::protobuf;
using xml_names::SnakeToCamel;
using xml_names::StripEnumPrefix;

// (sbe.schema_id)'s number before protowire v1.12.0. Nothing declares it any
// more, so it survives only in an options message's unknown fields.
constexpr int kRetiredSchemaID = 50100;

// StaleFileError is the diagnosis for a file whose (sbe.schema_id) sits at the
// retired number, as protowire-go's staleFileError; empty when it does not.
std::string StaleFileError(const gpb::FileDescriptor* fd) {
// An extension the pool declares at that number is a known option, as Go's
// compiler stores it, even where libprotobuf keeps it as unknown bytes.
// The pool's own FileOptions, which is not the generated one when the file
// was compiled with descriptor.proto in its pool.
const gpb::Descriptor* file_options =
fd->pool()->FindMessageTypeByName("google.protobuf.FileOptions");
if (file_options == nullptr) file_options = gpb::FileOptions::descriptor();
if (fd->pool()->FindExtensionByNumber(file_options, kRetiredSchemaID) != nullptr) return "";
const gpb::Message& opts = fd->options();
const gpb::UnknownFieldSet& unknown = opts.GetReflection()->GetUnknownFields(opts);
for (int i = 0; i < unknown.field_count(); ++i) {
if (unknown.field(i).number() == kRetiredSchemaID) {
return "sbe: file " + std::string(fd->name()) +
" carries (sbe.schema_id) at retired option number " +
std::to_string(kRetiredSchemaID) + " (registered as " + std::to_string(kExtSchemaID) +
" since protowire v1.12.0): the descriptor predates the registered extension block "
"and "
"must be recompiled against the current sbe/annotations.proto";
}
}
return "";
}

// IsList is protoreflect's IsList: repeated and not a map.
bool IsList(const gpb::FieldDescriptor* f) {
return f->is_repeated() && !f->is_map();
Expand Down
89 changes: 89 additions & 0 deletions test/sbe_codec_test.cc
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2026 TrendVidia, LLC.
#include "protowire/sbe.h"
#include "protowire/sbe/annotations.h"

#include <gtest/gtest.h>
#include "fixture_dir.h"
#include "protoc_compat.h"

#include <cstdio>
#include <cstring>
#include <fstream>
#include <memory>
#include <string>
#include <vector>

#include <google/protobuf/compiler/importer.h>
#include <google/protobuf/descriptor.pb.h>
#include <google/protobuf/dynamic_message.h>

namespace {
Expand Down Expand Up @@ -539,3 +542,89 @@ TEST(SbeSigned, NarrowSignedEncodingsKeepTheirSign) {
}

} // namespace

namespace {

// Codec::New diagnoses a descriptor compiled against the pre-v1.12.0
// annotations, in protowire-go's words (#122).
const pb::FileDescriptor* BuildStale(pb::DescriptorPool* pool, bool stale_file, bool stale_msg) {
pb::FileDescriptorProto proto;
proto.set_name("stale.proto");
proto.set_package("stale.v1");
proto.set_syntax("proto3");
pb::DescriptorProto* m = proto.add_message_type();
m->set_name("M");
pb::FieldDescriptorProto* f = m->add_field();
f->set_name("a");
f->set_number(1);
f->set_type(pb::FieldDescriptorProto::TYPE_INT32);
f->set_label(pb::FieldDescriptorProto::LABEL_OPTIONAL);
pb::FileOptions* fo = proto.mutable_options();
if (stale_file) {
fo->GetReflection()->MutableUnknownFields(fo)->AddVarint(50100, 1);
} else {
fo->GetReflection()->MutableUnknownFields(fo)->AddVarint(protowire::sbe::kExtSchemaID, 1);
}
if (stale_msg) {
pb::MessageOptions* mo = m->mutable_options();
mo->GetReflection()->MutableUnknownFields(mo)->AddVarint(50200, 1);
}
return pool->BuildFile(proto);
}

TEST(SbeStale, CodecNewDiagnosesStaleDescriptors) {
{
pb::DescriptorPool pool;
const pb::FileDescriptor* fd = BuildStale(&pool, true, false);
ASSERT_NE(fd, nullptr);
auto codec = protowire::sbe::Codec::New({fd});
ASSERT_FALSE(codec.ok());
EXPECT_EQ(codec.status().message(),
"sbe: file stale.proto carries (sbe.schema_id) at retired option number 50100 "
"(registered as 1319 since protowire v1.12.0): the descriptor predates the "
"registered extension block and must be recompiled against the current "
"sbe/annotations.proto");
}
{
pb::DescriptorPool pool;
const pb::FileDescriptor* fd = BuildStale(&pool, false, true);
ASSERT_NE(fd, nullptr);
auto codec = protowire::sbe::Codec::New({fd});
ASSERT_FALSE(codec.ok());
EXPECT_EQ(codec.status().message(),
"sbe: message stale.v1.M carries (sbe.template_id) at retired option number 50200 "
"(registered as 1321 since protowire v1.12.0): the descriptor predates the "
"registered extension block and must be recompiled against the current "
"sbe/annotations.proto");
}
{ // Neither: a plain message is skipped, as before.
pb::DescriptorPool pool;
const pb::FileDescriptor* fd = BuildStale(&pool, false, false);
ASSERT_NE(fd, nullptr);
EXPECT_TRUE(protowire::sbe::Codec::New({fd}).ok());
}
}

TEST(SbeStale, CodecNewMissingSchemaIDText) {
pb::compiler::DiskSourceTree tree;
tree.MapPath("", protowire_test::FixtureDir());
tree.MapPath("", WKT_PROTO_DIR);
{
std::ofstream(protowire_test::FixtureDir() + "/declared50100.proto")
<< "syntax = \"proto3\";\npackage d.v1;\nimport \"google/protobuf/descriptor.proto\";\n"
"extend google.protobuf.FileOptions { uint32 old_schema_id = 50100; }\n"
"option (old_schema_id) = 1;\nmessage M { int32 a = 1; }\n";
}
CollectErrors errors;
pb::compiler::Importer importer(&tree, &errors);
const pb::FileDescriptor* fd = importer.Import("declared50100.proto");
ASSERT_NE(fd, nullptr) << errors.last_;
// 50100 is declared here, so it is a known option, as Go's compiler stores
// it: the file is missing its schema id, not stale.
auto codec = protowire::sbe::Codec::New({fd});
ASSERT_FALSE(codec.ok());
EXPECT_EQ(codec.status().message(),
"sbe: file declared50100.proto missing (sbe.schema_id) option");
}

} // namespace
Loading