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 @@ -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 field in a oneof, in a message or a composite** (#141), as protowire-go does, with `sbe: oneof field <M>.<f> not supported`, or `composite <C> contains oneof field <f>` under the caller's prefix. **This narrows what binds:** such a message used to bind with a block slot for every member, and `Marshal` wrote the unset ones as zero. A proto3 `optional` field still binds, and so does a oneof in a group entry, which Go does not check either.
- **`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.
Expand Down
7 changes: 7 additions & 0 deletions src/sbe/template.cc
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@ StatusOr<std::vector<FieldTemplate>> BuildCompositeFields(const pb::Descriptor*
return Status::Error("composite " + std::string(md->full_name()) +
" contains list/map field " + std::string(fd->name()));
}
if (fd->real_containing_oneof() != nullptr) {
return Status::Error("composite " + std::string(md->full_name()) + " contains oneof 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);
Expand Down Expand Up @@ -237,6 +241,9 @@ StatusOr<MessageTemplate> BuildTemplate(const pb::Descriptor* md,
if (fd->is_map()) {
return Status::Error("sbe: map field " + FieldPath(md, fd) + " not supported");
}
if (fd->real_containing_oneof() != nullptr) {
return Status::Error("sbe: oneof field " + FieldPath(md, fd) + " not supported");
}
if (fd->is_repeated() && fd->cpp_type() == pb::FieldDescriptor::CPPTYPE_MESSAGE) {
auto gt = BuildGroupTemplate(fd);
if (!gt.ok()) return gt.status();
Expand Down
3 changes: 2 additions & 1 deletion test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ 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_encoding_range_test)
sbe_template_id_test sbe_header_bounds_test sbe_encoding_range_test
sbe_oneof_test)
if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${f}.cc)
list(APPEND _sbe_test_srcs ${f}.cc)
endif()
Expand Down
79 changes: 79 additions & 0 deletions test/sbe_oneof_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2026 TrendVidia, LLC.
//
// A oneof has no place in an SBE block, so Codec::New refuses a field in a
// real oneof in a message or a composite, as protowire-go's template.go
// does. A proto3 optional field sits in a synthetic oneof and binds. A group
// entry has no such check in Go, so a oneof there binds too (#141).

#include <gtest/gtest.h>

#include <string>
#include <vector>

#include "protowire/sbe.h"
#include "sbe_files.h"

namespace {

using protowire::sbe::Codec;
using protowire_test::SbeFiles;

TEST(SbeOneof, NewCodec) {
struct Case {
const char* name;
std::string body;
std::string reject; // the whole error; empty means it must bind
};
const std::vector<Case> cases = {
{"oneof in message",
"message M { option (sbe.template_id) = 1; uint32 t = 1; oneof o { uint32 a = 2; uint64 "
"b = 3; } }",
"sbe: oneof field oneof.v1.M.a not supported"},
{"oneof in composite",
"message C { oneof o { uint32 a = 1; uint64 b = 2; } } message M { option "
"(sbe.template_id) = 1; C c = 1; }",
"sbe: composite oneof.v1.M.c: composite oneof.v1.C contains oneof field a"},
{"oneof in nested composite",
"message D { oneof o { uint32 a = 1; } } message C { D d = 1; } message M { option "
"(sbe.template_id) = 1; C c = 1; }",
"sbe: composite oneof.v1.M.c: composite oneof.v1.D contains oneof field a"},
{"oneof in composite in group",
"message C { oneof o { uint32 a = 1; } } message E { C c = 1; } message M { option "
"(sbe.template_id) = 1; repeated E e = 1; }",
"sbe: composite in group oneof.v1.E.c: composite oneof.v1.C contains oneof field a"},
{"oneof in group entry binds",
"message E { oneof o { uint32 a = 1; uint64 b = 2; } } message M { option "
"(sbe.template_id) = 1; repeated E e = 1; }",
""},
{"proto3 optional in message binds",
"message M { option (sbe.template_id) = 1; optional uint32 a = 1; }",
""},
{"proto3 optional in composite binds",
"message C { optional uint32 a = 1; } message M { option (sbe.template_id) = 1; C c = 1; }",
""},
};
for (const Case& tc : cases) {
SCOPED_TRACE(tc.name);
SbeFiles files;
if (!files.Compile({{"oneof.proto",
"syntax = \"proto3\";\npackage oneof.v1;\nimport "
"\"sbe/annotations.proto\";\noption (sbe.schema_id) = 1;\n" +
tc.body + "\n"}})) {
ADD_FAILURE() << files.error();
continue;
}
auto codec = Codec::New(files.files());
if (tc.reject.empty()) {
EXPECT_TRUE(codec.ok()) << codec.status().ToString();
continue;
}
if (codec.ok()) {
ADD_FAILURE() << "bound; want " << tc.reject;
continue;
}
EXPECT_EQ(codec.status().message(), tc.reject);
}
}

} // namespace
Loading