fix(transport): widen pending_key() to preserve byte_bus_id's full 11 bits

[SoundMatt]

Bug

rcp::udp::Client::pending_key() and rcp::l2::Client::pending_key() computed their response-correlation map key by shifting byte_bus_id left 8 bits and OR-ing in transaction_num, then truncating the result to uint16_t:

static uint16_t pending_key(avtp::ByteBusId bus_id, uint8_t transaction_num) noexcept {
    return static_cast<uint16_t>((static_cast<uint16_t>(bus_id) << 8) | transaction_num);
}

byte_bus_id is an 11-bit wire field (0–2047; avtp::ByteBusId's own comment, acf.hpp's detail::kByteBusIdMask), not 8 bits, so the truncation silently dropped its top 3 bits: pending_key(5, 7) and pending_key(261, 7) both evaluated to 0x0507, even though 5 and 261 are two different, wire-legal byte_bus_id values.

Under concurrent requests sharing a Client instance (multiple threads calling Client::request() against different byte_bus_id endpoints — exactly what REQ-UDP-011's own text describes: "two outstanding requests with distinct (byte_bus_id, transaction_num) pairs each receive their own response"), two colliding keys land in the same pending_ map slot. The second request's insertion silently overwrites the first's promise pointer, so:

• the first request's response gets delivered to the second request's promise (misdelivery — the second caller receives the wrong request's data), and
• the first request's own promise, no longer reachable from pending_, never gets set — that Client::request() call hangs until ctx's deadline (or forever with no deadline).

HIGH severity, closes a finding from the cpp-RCP v3.0.0 deep audit (batch 1, transport pending_key collision).

Fix

Widened pending_key()'s return type to uint32_t, keeping the same left-shift-by-8 but without truncating the result back down — bus_id (0–2047, 11 bits) now occupies bits 8–18 of the key and transaction_num (0–255, 8 bits) occupies bits 0–7, so no two distinct (byte_bus_id, transaction_num) pairs can ever alias to the same key. Client::pending_'s map key type is updated to uint32_t to match, in both udp.hpp and l2.hpp, keeping the two files' fix identical.

Also moved pending_key out of each Client class (where it was a private static method) into a top-level pure function in each file — rcp/udp.hpp's own encode_annexj_datagram/decode_annexj_datagram and rcp/l2.hpp's own is_unicast_mac already establish this file's convention for pure, socket-free helpers placed unconditional of any platform guard, so the function itself (and a test exercising it directly) compiles and runs on every platform. This also makes the fix independently unit-testable without a live socket, which rcp/l2.hpp's Client needs (AF_PACKET/CAP_NET_RAW, Linux-only) but a plain function does not.

No other code in either file was touched.

Tests

• tests/test_udp.cpp:
  • A pure pending_key() unit test proving pending_key(5, 7) != pending_key(261, 7) and sweeping every byte_bus_id 256 apart across the full 11-bit range crossed with several transaction_num values for collisions (tagged REQ-UDP-011, the existing requirement whose own text this bug violated).
  • A new two-thread, real-loopback-socket regression test ("Client::request does not misdeliver or hang for two genuinely concurrent requests...") that drives genuine overlap between a byte_bus_id=5 request (server-side delayed 150ms) and a byte_bus_id=261 request sharing transaction_num 7, and asserts each gets its own correct response.
• tests/test_l2.cpp:
  • The same pure pending_key() unit test (no REQ tag — no existing cpp-RCP L2 requirement currently covers Client-level response correlation by key the way REQ-UDP-011 does for udp.hpp; a new REQ-L2-011 entry in .fusa-reqs.json may be warranted but was deliberately not added here, out of scope for this fix — flagging for follow-up).
  • A live-socket equivalent of the threaded udp.hpp test was not added to tests/l2_veth_roundtrip.cpp, since that harness needs CAP_NET_RAW/root and only runs in the dedicated l2-veth CI job, not the privilege-free ctest suite this fix was verified against.

Verification

1. Full clean rebuild: rm -rf build && cmake -S . -B build -DRCP_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Debug && cmake --build build -j — 0 errors, 0 warnings.

2. Full test suite: ctest --output-on-failure — 100% pass, 58/58 test binaries, including both new pending_key regression tests.

3. Mutation-testing sanity check: reintroduced the original uint16_t truncation arithmetic (keeping the new call sites/map type so it still compiled), rebuilt, and confirmed the new tests fail against it:

   • pending_key(5,7) == pending_key(261,7) (both 0x0507, REQUIRE fails as expected)
   • the concurrent-request regression test's byte_bus_id=5 leg times out (ErrTimeout), exactly matching the "indefinite hang" failure mode this finding describes.

   Reverted the mutation and reran the full suite to confirm 100% pass again.

🤖 Generated with Claude Code