From 158ebed5c845f888ce0523c79590b0768c8a81ad Mon Sep 17 00:00:00 2001 From: Raphael Maenle Date: Tue, 9 Jun 2026 10:54:45 +0200 Subject: [PATCH] tests: identifier-width wildcard matrix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SEMI E5 allows identifier fields (DATAID, RPTID, VID, CEID, ALID, EXID, OBJID, …) to be encoded as U1, U2, U4, or U8. Our parsers route through any_unsigned_first in messages_helpers.hpp. The existing per-message round-trip tests prove the U4 path; this commit adds the cross-width matrix that the interop incident with secsgem-py demanded: - as_u4_scalar accepts U1/U2/U4/U8 inputs for the same value - as_u8_scalar accepts every narrower width - as_u1_scalar accepts wider widths when the value fits - as_u1_scalar / as_u2_scalar REJECT out-of-range values rather than silently truncating - codec round-trip preserves the format byte AND the value - signed counterparts (as_i4_scalar) follow the same rule for I1/I2 If a future code-gen change hard-codes a single width on any identifier field, the rejection case here breaks loudly. Closes #12 in the test-gap backlog (renumbered: this is gap entry "identifier wildcard matrix"). Co-Authored-By: Claude Opus 4.7 --- CMakeLists.txt | 1 + tests/test_identifier_wildcards.cpp | 122 ++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 tests/test_identifier_wildcards.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 63aa8e0..b55d01c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -130,6 +130,7 @@ add_executable(secsgem_tests tests/test_wire_ceid_emission.cpp tests/test_live_gem300.cpp tests/test_e87_wire_scenarios.cpp + tests/test_identifier_wildcards.cpp ) target_link_libraries(secsgem_tests PRIVATE secsgem doctest::doctest) target_compile_definitions(secsgem_tests PRIVATE diff --git a/tests/test_identifier_wildcards.cpp b/tests/test_identifier_wildcards.cpp new file mode 100644 index 0000000..a5a2822 --- /dev/null +++ b/tests/test_identifier_wildcards.cpp @@ -0,0 +1,122 @@ +// Identifier-width wildcard matrix. +// +// SEMI E5 documents identifier fields (DATAID, RPTID, VID, CEID, ALID, +// EXID, OBJID, …) as "U1 | U2 | U4 | U8" — peers may pick the smallest +// width that fits the value. Our parsers route through +// `any_unsigned_first` (messages_helpers.hpp) which accepts any +// unsigned format and range-checks the cast. +// +// This test enumerates every (target_type, source_width, value) combo +// and asserts the cross-width parsing is symmetric. Without this, a +// future code-gen change that hard-codes a specific width on a given +// field would silently regress wire compatibility (this regression is +// the original bug behind the U1-encoded ALID interop incident with +// secsgem-py 0.3.0, captured in [[secsgem_interop_findings]]). + +#include + +#include +#include + +#include "secsgem/gem/messages_helpers.hpp" +#include "secsgem/secs2/codec.hpp" +#include "secsgem/secs2/item.hpp" + +using namespace secsgem::gem; +namespace s2 = secsgem::secs2; + +// Build an Item at every unsigned width that can hold `value`. +static std::vector all_widths(uint64_t value) { + std::vector out; + if (value <= 0xFF) + out.push_back(s2::Item::u1(static_cast(value))); + if (value <= 0xFFFF) + out.push_back(s2::Item::u2(static_cast(value))); + if (value <= 0xFFFFFFFFu) + out.push_back(s2::Item::u4(static_cast(value))); + out.push_back(s2::Item::u8(value)); + return out; +} + +TEST_CASE("identifier wildcard: as_u4_scalar accepts U1/U2/U4 encodings of small values") { + // The U1 case is the historic interop bug. + for (uint64_t value : {uint64_t{0}, uint64_t{1}, uint64_t{42}, + uint64_t{255}}) { + for (const auto& item : all_widths(value)) { + auto got = as_u4_scalar(item); + REQUIRE(got.has_value()); + CHECK(*got == value); + } + } +} + +TEST_CASE("identifier wildcard: as_u8_scalar accepts every width") { + for (uint64_t value : {uint64_t{0}, uint64_t{1}, uint64_t{255}, + uint64_t{65535}, uint64_t{4294967295u}, + uint64_t{1ULL << 40}}) { + for (const auto& item : all_widths(value)) { + auto got = as_u8_scalar(item); + REQUIRE(got.has_value()); + CHECK(*got == value); + } + } +} + +TEST_CASE("identifier wildcard: as_u1_scalar accepts wider widths if value fits") { + // U2/U4/U8 encodings of small values should still parse as U1 via + // range-checked downcast. This is the symmetric case to the U4 bug. + for (uint64_t value : {uint64_t{0}, uint64_t{1}, uint64_t{200}, + uint64_t{255}}) { + for (const auto& item : all_widths(value)) { + auto got = as_u1_scalar(item); + REQUIRE(got.has_value()); + CHECK(*got == static_cast(value)); + } + } +} + +TEST_CASE("identifier wildcard: as_u1_scalar rejects values that exceed its range") { + // 256 fits U2 but not U1 — the downcast must signal failure rather + // than truncate. + auto u2_val = s2::Item::u2(uint16_t{256}); + CHECK_FALSE(as_u1_scalar(u2_val).has_value()); + + auto u4_val = s2::Item::u4(uint32_t{0x1FFFFu}); + CHECK_FALSE(as_u1_scalar(u4_val).has_value()); + CHECK_FALSE(as_u2_scalar(u4_val).has_value()); +} + +TEST_CASE("identifier wildcard: round-trip through codec preserves value across widths") { + // The byte-level concern: an encoded U1 must decode back to a U1 + // Item; the scalar accessor must still extract the value. This + // proves the bug isn't reintroduced when frames travel through the + // full codec rather than via in-memory Items. + for (uint64_t value : {uint64_t{0}, uint64_t{1}, uint64_t{42}, + uint64_t{12345}}) { + for (const auto& item : all_widths(value)) { + auto bytes = s2::encode(item); + auto decoded = s2::decode(bytes); + CHECK(decoded.format() == item.format()); + auto got = as_u8_scalar(decoded); + REQUIRE(got.has_value()); + CHECK(*got == value); + } + } +} + +TEST_CASE("identifier wildcard: I1/I2/I4/I8 same lenient pattern") { + // The signed variants follow the same wildcard rule per E5. + for (int64_t value : {int64_t{0}, int64_t{1}, int64_t{-1}, int64_t{42}, + int64_t{-128}, int64_t{127}}) { + s2::Item item = s2::Item::i1(static_cast(value)); + auto got = as_i4_scalar(item); + REQUIRE(got.has_value()); + CHECK(*got == value); + } + for (int64_t value : {int64_t{-32768}, int64_t{0}, int64_t{32767}}) { + s2::Item item = s2::Item::i2(static_cast(value)); + auto got = as_i4_scalar(item); + REQUIRE(got.has_value()); + CHECK(*got == value); + } +}