pxf: refuse (pxf.required) with (pxf.default) on one field at bind time

.github/workflows/ci.yml

@@ -20,7 +20,7 @@ env:
   # PXF v0.72-series feature set (@<name> / @entry / @table directive
   # grammar, schema validator, Result accessors, TableReader streaming)
   # the Python port wraps. Bump in lockstep with cpp release cuts.
-  PROTOWIRE_CPP_REF: v1.0.0
+  PROTOWIRE_CPP_REF: v1.1.0
 
 jobs:
   # ---------------------------------------------------------------------

.github/workflows/codeql.yml

@@ -17,7 +17,7 @@ permissions:
 
 env:
   # See ci.yml for the rationale on this pin.
-  PROTOWIRE_CPP_REF: v1.0.0
+  PROTOWIRE_CPP_REF: v1.1.0
 
 jobs:
   analyze:

.github/workflows/publish.yml

@@ -28,7 +28,7 @@ env:
   # feature set (@<name> / @entry / @table grammar, schema validator,
   # Result accessors, TableReader streaming) that this Python port
   # wraps. Bump in lockstep with cpp release cuts.
-  PROTOWIRE_CPP_REF: v1.0.0
+  PROTOWIRE_CPP_REF: v1.1.0
 
 jobs:
   # ---------------------------------------------------------------------

CHANGELOG.md

@@ -11,6 +11,21 @@ format changes.
 
 ## [Unreleased]
 
+### Changed
+
+- **Built against protowire-cpp v1.1.0** in CI and the wheels (#23). v1.0.0
+  segfaulted on deeply nested PXF and read the PXF and SBE options at their
+  old numbers. `Violation.kind` gains `"key_option"`, which v1.1.0 reports.
+
+### Fixed
+
+- **`(pxf.required)` with `(pxf.default)` on one field is a bind-time
+  violation** (#22), as draft -01 now requires (protowire#453). Such a schema
+  used to bind. `unmarshal` and `unmarshal_full` raise `ValueError` unless
+  `skip_validate` is set, and `validate_descriptor` reports a
+  `"required_default"` violation, over the file and its imports.
+  `(pxf.required) = false` beside a default stays accepted.
+
 ## [1.0.0] — 2026-05-13
 
 First major-version cut. Implements the three one-time spec changes

src/_protowire/module.cc

@@ -18,6 +18,7 @@
 #include <cstdint>
 #include <memory>
 #include <optional>
+#include <set>
 #include <span>
 #include <sstream>
 #include <string>
@@ -32,6 +33,7 @@
 #include <google/protobuf/message.h>
 
 #include "protowire/pxf.h"
+#include "protowire/pxf/annotations.h"
 #include "protowire/sbe.h"
 
 namespace nb = nanobind;
@@ -129,12 +131,76 @@ nb::object CellToPyTuple(const std::optional<protowire::pxf::ValuePtr>& cell) {
       *cell);
 }
 
+// --- (pxf.required) with (pxf.default) --------------------------------------
+//
+// Draft -01 § Required and Default forbids the pair on one field, and a tool
+// binding a descriptor must refuse it over the import closure. The pinned
+// protowire-cpp does not check this yet (protowire-cpp#50), so the bridge does,
+// using cpp's own option readers: the option numbers stay in one place. Once
+// the pin carries the upstream check, this one goes.
+
+struct RequiredWithDefault {
+  std::string file;
+  std::string element;  // fully-qualified field name
+  std::string name;     // bare field name
+};
+
+void WalkRequiredWithDefault(const pbuf::Descriptor* md,
+                             std::vector<RequiredWithDefault>* out) {
+  for (int i = 0; i < md->field_count(); ++i) {
+    const pbuf::FieldDescriptor* f = md->field(i);
+    if (protowire::pxf::IsRequired(f) && protowire::pxf::GetDefault(f).has_value()) {
+      out->push_back({std::string(md->file()->name()), std::string(f->full_name()),
+                      std::string(f->name())});
+    }
+  }
+  for (int i = 0; i < md->nested_type_count(); ++i) {
+    WalkRequiredWithDefault(md->nested_type(i), out);
+  }
+}
+
+void WalkRequiredWithDefault(const pbuf::FileDescriptor* fd,
+                             std::set<const pbuf::FileDescriptor*>* seen,
+                             std::vector<RequiredWithDefault>* out) {
+  if (!seen->insert(fd).second) return;
+  for (int i = 0; i < fd->message_type_count(); ++i) {
+    WalkRequiredWithDefault(fd->message_type(i), out);
+  }
+  for (int i = 0; i < fd->dependency_count(); ++i) {
+    WalkRequiredWithDefault(fd->dependency(i), seen, out);
+  }
+}
+
+std::vector<RequiredWithDefault> FindRequiredWithDefault(const pbuf::Descriptor* desc) {
+  std::set<const pbuf::FileDescriptor*> seen;
+  std::vector<RequiredWithDefault> out;
+  WalkRequiredWithDefault(desc->file(), &seen, &out);
+  return out;
+}
+
+// Raises when the schema carries the pair; `op` prefixes the error as the
+// decoder's own errors are prefixed. Honors skip_validate through its callers,
+// like cpp's CheckSchema.
+void CheckRequiredWithDefault(const pbuf::Descriptor* desc, const std::string& op) {
+  auto found = FindRequiredWithDefault(desc);
+  if (found.empty()) return;
+  std::string m = op + ": PXF schema violations:";
+  for (const auto& v : found) {
+    m += "\n  " + v.file + ": field \"" + v.element +
+         "\": (pxf.required) and (pxf.default) are exclusive on one field: the first rejects "
+         "a document where the field is absent, and the second treats it as supplied "
+         "(draft -01 §Required and Default)";
+  }
+  throw nb::value_error(m.c_str());
+}
+
 // PXF text -> binary proto bytes.
 nb::bytes PxfUnmarshal(nb::bytes text, nb::bytes fds_bytes,
                        const std::string& full_name, bool discard_unknown,
                        bool skip_validate) {
   auto schema = BuildSchema(std::string_view(fds_bytes.c_str(), fds_bytes.size()));
   const auto* desc = FindDescriptor(schema, full_name);
+  if (!skip_validate) CheckRequiredWithDefault(desc, "pxf.unmarshal");
   std::unique_ptr<pbuf::Message> msg(
       schema.factory->GetPrototype(desc)->New());
 
@@ -174,6 +240,7 @@ PxfUnmarshalFull(nb::bytes text, nb::bytes fds_bytes,
                  bool skip_validate) {
   auto schema = BuildSchema(std::string_view(fds_bytes.c_str(), fds_bytes.size()));
   const auto* desc = FindDescriptor(schema, full_name);
+  if (!skip_validate) CheckRequiredWithDefault(desc, "pxf.unmarshal_full");
   std::unique_ptr<pbuf::Message> msg(
       schema.factory->GetPrototype(desc)->New());
 
@@ -231,7 +298,8 @@ PxfUnmarshalFull(nb::bytes text, nb::bytes fds_bytes,
 
 // PXF schema reserved-name check (draft §3.13). Returns a list of
 // (kind, element, name, file) tuples. Empty list ⇒ conformant schema.
-// kind values: "field" / "oneof" / "enum_value".
+// kind values: "field" / "oneof" / "enum_value" / "key_option" /
+// "required_default".
 std::vector<std::tuple<std::string, std::string, std::string, std::string>>
 PxfValidateDescriptor(nb::bytes fds_bytes, const std::string& full_name) {
   auto schema = BuildSchema(std::string_view(fds_bytes.c_str(), fds_bytes.size()));
@@ -245,9 +313,13 @@ PxfValidateDescriptor(nb::bytes fds_bytes, const std::string& full_name) {
       case protowire::pxf::ViolationKind::kField:     kind = "field";      break;
       case protowire::pxf::ViolationKind::kOneof:     kind = "oneof";      break;
       case protowire::pxf::ViolationKind::kEnumValue: kind = "enum_value"; break;
+      case protowire::pxf::ViolationKind::kKeyOption: kind = "key_option"; break;
     }
     out.emplace_back(std::move(kind), v.element, v.name, v.file);
   }
+  for (const auto& v : FindRequiredWithDefault(desc)) {
+    out.emplace_back("required_default", v.element, v.name, v.file);
+  }
   return out;
 }
 

src/protowire/pxf.py

@@ -102,15 +102,17 @@ class ProtoDirective:
 
 @dataclass(frozen=True)
 class Violation:
-    """A schema reserved-name violation, draft §3.13.
+    """A schema that must not be bound for PXF, draft -01.
 
-    Returned by `validate_descriptor`. `kind` is one of
-    `"field"` / `"oneof"` / `"enum_value"`.
+    Returned by `validate_descriptor`. `kind` is one of `"field"` / `"oneof"`
+    / `"enum_value"` (a reserved name, §3.13), `"key_option"` (a misplaced
+    `(pxf.key)`) or `"required_default"` (`(pxf.required)` and `(pxf.default)`
+    on one field).
     """
 
-    kind: Literal["field", "oneof", "enum_value"]
+    kind: Literal["field", "oneof", "enum_value", "key_option", "required_default"]
     element: str  # fully-qualified protobuf name, e.g. "trades.v1.Side.null"
-    name: str  # the bare reserved identifier ("null" / "true" / "false")
+    name: str  # the reserved identifier, the (pxf.key) value, or the field's name
     file: str  # .proto file path the element is declared in
 
 
@@ -147,9 +149,9 @@ def null_fields(self) -> list[str]:
 
 
 def validate_descriptor(msg: Message) -> list[Violation]:
-    """Return schema reserved-name violations on `msg`'s descriptor.
+    """Return the bind-time violations of `msg`'s descriptor and its imports.
 
-    An empty list means the schema is conformant. The check is
+    An empty list means the schema is conformant. The reserved-name check is
     case-sensitive: `NULL` / `True` lex as identifiers and are accepted.
     """
     fds = fds_for_message(msg)

tests/test_pxf_required_default.py

@@ -0,0 +1,153 @@
+# SPDX-License-Identifier: MIT
+# Copyright (c) 2026 TrendVidia, LLC.
+"""Draft -01 § Required and Default: (pxf.required) and (pxf.default) on one
+field must be refused when the descriptor is bound.
+
+The schemas are built from raw option bytes, at the registered numbers, because
+that is what reaches the decoder: this port has no annotations.proto of its own.
+"""
+
+from __future__ import annotations
+
+import pytest
+from google.protobuf import descriptor_pb2, descriptor_pool, message_factory
+
+from protowire import pxf
+
+_VARINT, _LEN = 0, 2
+_REQUIRED, _DEFAULT, _KEY = 1314, 1315, 1316
+
+
+def _varint(n: int) -> bytes:
+    out = bytearray()
+    while n > 0x7F:
+        out.append(n & 0x7F | 0x80)
+        n >>= 7
+    out.append(n)
+    return bytes(out)
+
+
+def _opt_bool(number: int, value: bool) -> bytes:
+    return _varint(number << 3 | _VARINT) + _varint(int(value))
+
+
+def _opt_str(number: int, value: str) -> bytes:
+    return _varint(number << 3 | _LEN) + _varint(len(value)) + value.encode()
+
+
+REQUIRED = _opt_bool(_REQUIRED, True)
+NOT_REQUIRED = _opt_bool(_REQUIRED, False)
+DEFAULT = _opt_str(_DEFAULT, "x")
+
+STRING = descriptor_pb2.FieldDescriptorProto.TYPE_STRING
+MESSAGE = descriptor_pb2.FieldDescriptorProto.TYPE_MESSAGE
+
+
+def _add_field(msg, name, number, options=b"", type_=STRING, type_name=""):
+    f = msg.field.add(name=name, number=number, type=type_)
+    if type_name:
+        f.type_name = type_name
+    if options:
+        f.options.MergeFromString(options)
+
+
+def _file(name, *deps) -> descriptor_pb2.FileDescriptorProto:
+    fd = descriptor_pb2.FileDescriptorProto(name=name, package="t", syntax="proto3")
+    fd.dependency.extend(deps)
+    return fd
+
+
+def _bind(*files):
+    """Return the FDS bytes and a class for t.M, the first file's message."""
+    fds = descriptor_pb2.FileDescriptorSet()
+    pool = descriptor_pool.DescriptorPool()
+    for fd in reversed(files):
+        fds.file.append(fd)
+        pool.Add(fd)
+    desc = pool.FindMessageTypeByName("t.M")
+    return fds.SerializeToString(), message_factory.GetMessageClass(desc)
+
+
+def _one_field(options: bytes):
+    fd = _file("one.proto")
+    _add_field(fd.message_type.add(name="M"), "f", 1, options)
+    return _bind(fd)
+
+
+def _assert_refused(fds: bytes, cls, element: str, file: str) -> None:
+    # A document supplying the field: the schema is wrong whatever the input.
+    doc = 'f = "y"\n' if cls.DESCRIPTOR.fields_by_name.get("f") else ""
+    for call in (
+        lambda: pxf.unmarshal(doc, cls()),
+        lambda: pxf.unmarshal_full(doc, cls()),
+        lambda: pxf.unmarshal_bytes(doc, fds, "t.M"),
+        lambda: pxf.unmarshal_full_bytes(doc, fds, "t.M"),
+    ):
+        with pytest.raises(ValueError) as e:
+            call()
+        assert "(pxf.required) and (pxf.default) are exclusive on one field" in str(e.value)
+        assert element in str(e.value)
+    assert pxf.validate_descriptor(cls()) == [
+        pxf.Violation(
+            kind="required_default",
+            element=element,
+            name=element.rsplit(".", 1)[1],
+            file=file,
+        )
+    ]
+
+
+def test_required_with_default_is_refused_at_bind():
+    fds, cls = _one_field(REQUIRED + DEFAULT)
+    _assert_refused(fds, cls, "t.M.f", "one.proto")
+
+
+def test_option_order_does_not_matter():
+    fds, cls = _one_field(DEFAULT + REQUIRED)
+    _assert_refused(fds, cls, "t.M.f", "one.proto")
+
+
+def test_required_false_beside_default_stays_accepted():
+    fds, cls = _one_field(NOT_REQUIRED + DEFAULT)
+    assert pxf.validate_descriptor(cls()) == []
+    msg = cls()
+    pxf.unmarshal_full("", msg)
+    assert msg.f == "x"  # Proves cpp read the default from these bytes.
+
+
+def test_required_alone_and_default_alone_are_accepted():
+    _, required = _one_field(REQUIRED)
+    assert pxf.validate_descriptor(required()) == []
+    with pytest.raises(ValueError, match="required"):  # cpp read this one too.
+        pxf.unmarshal_full("", required())
+    _, default = _one_field(DEFAULT)
+    assert pxf.validate_descriptor(default()) == []
+
+
+def test_nested_message_is_covered():
+    fd = _file("nested.proto")
+    _add_field(fd.message_type.add(name="M").nested_type.add(name="In"), "f", 1, REQUIRED + DEFAULT)
+    fds, cls = _bind(fd)
+    _assert_refused(fds, cls, "t.M.In.f", "nested.proto")
+
+
+def test_import_closure_is_covered_even_when_no_field_uses_the_type():
+    dep = _file("dep.proto")
+    _add_field(dep.message_type.add(name="Other"), "f", 1, REQUIRED + DEFAULT)
+    root = _file("root.proto", "dep.proto")
+    root.message_type.add(name="M")
+    fds, cls = _bind(root, dep)
+    _assert_refused(fds, cls, "t.Other.f", "dep.proto")
+
+
+def test_skip_validate_skips_this_check_like_the_others():
+    fds, cls = _one_field(REQUIRED + DEFAULT)
+    pxf.unmarshal('f = "y"\n', cls(), skip_validate=True)
+    pxf.unmarshal_bytes('f = "y"\n', fds, "t.M", skip_validate=True)
+
+
+def test_misplaced_key_option_is_reported_with_its_kind():
+    fd = _file("key.proto")
+    _add_field(fd.message_type.add(name="M"), "f", 1, _opt_str(_KEY, "id"))
+    _, cls = _bind(fd)
+    assert [v.kind for v in pxf.validate_descriptor(cls())] == ["key_option"]