SoundMatt · SoundMatt · Aug 22, 2026 · Aug 22, 2026
@@ -208,6 +208,31 @@ inline bool is_unicast_mac(const MacAddress& mac) noexcept {
     return (mac[0] & 0x01u) == 0u;
 }
 
+// pending_key combines byte_bus_id (an 11-bit wire field — avtp::ByteBusId's
+// own comment, and acf.hpp's detail::kByteBusIdMask) and transaction_num (a
+// full 8-bit field) into one collision-free correlation key for Client's own
+// pending_ member below (RCP_L2_LINUX branch only — Client is a stub on
+// every other platform). Same formula and same fix as rcp/udp.hpp's own
+// top-level pending_key (that file's own comment explains the collision
+// this widening fixes). Placed here, unconditional of RCP_L2_LINUX like
+// is_unicast_mac just above, rather than in the RCP_L2_LINUX-only detail
+// namespace below, so this pure function — and the test exercising it
+// directly — compiles and runs on every platform, not only Linux.
+//
+// Shifting bus_id left by 8 keeps every (byte_bus_id, transaction_num) pair
+// distinct: transaction_num occupies bits 0-7 and byte_bus_id (0-2047, 11
+// bits) occupies bits 8-18 of the returned uint32_t, so the two fields never
+// overlap. A previous version of this function returned uint16_t, computing
+// the identical shift but then truncating the result back down to 16 bits —
+// silently dropping byte_bus_id's top 3 bits, so e.g. byte_bus_id 5 and 261
+// (differing by exactly 256) collided whenever they shared a
+// transaction_num, misdelivering one Client's pending response to another
+// (or hanging it indefinitely) under concurrent requests. cpp-RCP v3.0.0
+// deep audit finding; fixed by widening this key to uint32_t.
+inline uint32_t pending_key(avtp::ByteBusId bus_id, uint8_t transaction_num) noexcept {
+    return (static_cast<uint32_t>(bus_id) << 8) | static_cast<uint32_t>(transaction_num);
+}
+
 // ── Frame ─────────────────────────────────────────────────────────────────────
 // Same AVTPDU shape as rcp/udp.hpp::Frame (one NTSCF/TSCF header wrapping
 // one ACF_ABB/ACF_GBB message) — deliberately duplicated here rather than
@@ -929,7 +954,7 @@ class Client {
         out.message_timestamp = message_timestamp;
         out.payload            = req_payload;
 
-        const uint16_t key = pending_key(req.byte_bus_id, req.transaction_num);
+        const uint32_t key = pending_key(req.byte_bus_id, req.transaction_num);
         auto result = std::make_shared<std::promise<Frame>>();
         auto future = result->get_future();
         {
@@ -984,10 +1009,6 @@ class Client {
     bool ok() const noexcept { return fd_ >= 0; }
 
 private:
-    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);
-    }
-
     avtp::StreamId stream_id_;
     MacAddress dest_mac_{};
     MacAddress local_mac_{};
@@ -996,7 +1017,7 @@ class Client {
     std::atomic<bool>     closed_{false};
     std::atomic<uint16_t> seq_{0};
     std::mutex mu_;
-    std::map<uint16_t, std::shared_ptr<std::promise<Frame>>> pending_;
+    std::map<uint32_t, std::shared_ptr<std::promise<Frame>>> pending_;
     std::thread read_thread_;
 
     void read_loop() {
@@ -1021,7 +1042,7 @@ class Client {
             Frame     resp;
             if (decode_l2_frame(buf.data(), static_cast<size_t>(n), hdr, resp)) continue;
 
-            const uint16_t key = pending_key(resp.info.byte_bus_id, resp.info.transaction_num);
+            const uint32_t key = pending_key(resp.info.byte_bus_id, resp.info.transaction_num);
             std::lock_guard<std::mutex> lk(mu_);
             auto it = pending_.find(key);
             if (it != pending_.end()) {

@@ -185,6 +185,27 @@ struct FrameResponse {
     std::vector<uint8_t>  payload;
 };
 
+// pending_key combines byte_bus_id (an 11-bit wire field — avtp::ByteBusId's
+// own comment, and acf.hpp's detail::kByteBusIdMask) and transaction_num (a
+// full 8-bit field) into one collision-free correlation key for
+// Client::pending_ below. A pure function, independent of RCP_UDP_POSIX like
+// encode_annexj_datagram/decode_annexj_datagram above, so it — and the test
+// exercising it directly — compiles and runs on every platform.
+//
+// Shifting bus_id left by 8 keeps every (byte_bus_id, transaction_num) pair
+// distinct: transaction_num occupies bits 0-7 and byte_bus_id (0-2047, 11
+// bits) occupies bits 8-18 of the returned uint32_t, so the two fields never
+// overlap. A previous version of this function returned uint16_t, computing
+// the identical shift but then truncating the result back down to 16 bits —
+// silently dropping byte_bus_id's top 3 bits, so e.g. byte_bus_id 5 and 261
+// (differing by exactly 256) collided whenever they shared a
+// transaction_num, misdelivering one Client's pending response to another
+// (or hanging it indefinitely) under concurrent requests. cpp-RCP v3.0.0
+// deep audit finding; fixed by widening this key to uint32_t.
+inline uint32_t pending_key(avtp::ByteBusId bus_id, uint8_t transaction_num) noexcept {
+    return (static_cast<uint32_t>(bus_id) << 8) | static_cast<uint32_t>(transaction_num);
+}
+
 #if defined(RCP_UDP_POSIX)
 
 // ── Frame ─────────────────────────────────────────────────────────────────────
@@ -688,7 +709,7 @@ class Client {
         out.message_timestamp = message_timestamp;
         out.payload           = req_payload;
 
-        const uint16_t key = pending_key(req.byte_bus_id, req.transaction_num);
+        const uint32_t key = pending_key(req.byte_bus_id, req.transaction_num);
         auto result = std::make_shared<std::promise<Frame>>();
         auto future = result->get_future();
         {
@@ -746,18 +767,14 @@ class Client {
     uint32_t last_recv_encap_seq() const noexcept { return last_recv_encap_seq_.load(); }
 
 private:
-    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);
-    }
-
     avtp::StreamId stream_id_;
     int  fd_;
     std::atomic<bool>     closed_{false};
     std::atomic<uint16_t> seq_{0};
     std::atomic<uint32_t> encap_seq_{0};          // Annex J encapsulation sequence number, outgoing
     std::atomic<uint32_t> last_recv_encap_seq_{0}; // most recent one seen on an inbound datagram
     std::mutex mu_;
-    std::map<uint16_t, std::shared_ptr<std::promise<Frame>>> pending_;
+    std::map<uint32_t, std::shared_ptr<std::promise<Frame>>> pending_;
     std::thread read_thread_;
 
     void read_loop() {
@@ -779,7 +796,7 @@ class Client {
             Frame resp;
             if (decode_frame(avtpdu, avtpdu_len, resp)) continue;
 
-            const uint16_t key = pending_key(resp.info.byte_bus_id, resp.info.transaction_num);
+            const uint32_t key = pending_key(resp.info.byte_bus_id, resp.info.transaction_num);
             std::lock_guard<std::mutex> lk(mu_);
             auto it = pending_.find(key);
             if (it != pending_.end()) {

@@ -37,6 +37,8 @@
 #include <rcp/l2.hpp>
 #include <rcp/mock.hpp>
 
+#include <set>
+
 using namespace rcp;
 using namespace rcp::l2;
 
@@ -255,6 +257,45 @@ TEST_CASE("is_unicast_mac reports false for the all-ones broadcast address", "[l
     REQUIRE_FALSE(is_unicast_mac(mac));
 }
 
+// ── pending_key — Client response-correlation key (pure, no socket) ─────────
+//
+// rcp::l2::Client::request()/read_loop() correlate an inbound response to
+// its outstanding request via pending_key(byte_bus_id, transaction_num),
+// the same formula rcp/udp.hpp's own top-level pending_key uses (that
+// file's own comment documents the collision this widening fixes) — no
+// real AF_PACKET socket needed to exercise the pure key-computation logic
+// itself (it is unconditional of RCP_L2_LINUX, like is_unicast_mac just
+// above), so this stays in this privilege-free file rather than
+// tests/l2_veth_roundtrip.cpp (this file's own header comment).
+TEST_CASE("pending_key does not collide for byte_bus_id values that differ by a "
+          "multiple of 256 and share a transaction_num",
+          "[l2]") {
+    // byte_bus_id is an 11-bit wire field (0-2047; avtp::ByteBusId's own
+    // comment, acf.hpp's detail::kByteBusIdMask) -- 5 and 261 are both
+    // wire-legal and differ by exactly 256. A previous version of
+    // pending_key returned uint16_t, computing this same left-shift-by-8
+    // but then truncating the result back down to 16 bits, so these two
+    // collided into the identical map key (0x0507) whenever they shared a
+    // transaction_num (cpp-RCP v3.0.0 deep audit finding) — silently
+    // misdelivering one Client::request() caller's response to another's
+    // promise, or hanging one of them indefinitely.
+    REQUIRE(pending_key(5, 7) != pending_key(261, 7));
+    REQUIRE(pending_key(5, 7) == pending_key(5, 7));
+
+    // Sweep every byte_bus_id that used to alias to the same 16-bit key
+    // under the old truncation (i.e. every value 256 apart, across the
+    // whole 11-bit range) crossed with a few transaction_num values, and
+    // confirm every (byte_bus_id, transaction_num) pair now maps to a
+    // distinct key.
+    std::set<uint32_t> seen;
+    for (uint32_t bus = 0; bus <= 2047; bus += 256) {
+        for (uint32_t txn = 0; txn <= 255; txn += 85) {
+            auto key = pending_key(static_cast<avtp::ByteBusId>(bus), static_cast<uint8_t>(txn));
+            REQUIRE(seen.insert(key).second); // must be a fresh key, never seen before
+        }
+    }
+}
+
 // ── AVTP envelope-only decode (decode_avtp_frame_header/decode_l2_frame_header) ──
 
 TEST_CASE("decode_avtp_frame_header decodes an NTSCF envelope and reports the raw ACF offset",

@@ -37,6 +37,8 @@
 
 #include <algorithm>
 #include <chrono>
+#include <set>
+#include <thread>
 
 using namespace rcp;
 using namespace rcp::udp;
@@ -310,6 +312,33 @@ TEST_CASE("encode_annexj_datagram produces a monotonically increasing wire prefi
     }
 }
 
+TEST_CASE("pending_key does not collide for byte_bus_id values that differ by a multiple of "
+          "256 and share a transaction_num",
+          "[udp][REQ-UDP-011]") {
+    // byte_bus_id is an 11-bit wire field (0-2047; avtp::ByteBusId's own
+    // comment, acf.hpp's detail::kByteBusIdMask) -- 5 and 261 are both
+    // wire-legal and differ by exactly 256. A previous version of
+    // pending_key returned uint16_t, computing this same left-shift-by-8
+    // but then truncating the result back down to 16 bits, so these two
+    // collided into the identical map key (0x0507) whenever they shared a
+    // transaction_num (cpp-RCP v3.0.0 deep audit finding).
+    REQUIRE(pending_key(5, 7) != pending_key(261, 7));
+    REQUIRE(pending_key(5, 7) == pending_key(5, 7));
+
+    // Sweep every byte_bus_id that used to alias to the same 16-bit key
+    // under the old truncation (i.e. every value 256 apart, across the
+    // whole 11-bit range) crossed with a few transaction_num values, and
+    // confirm every (byte_bus_id, transaction_num) pair now maps to a
+    // distinct key.
+    std::set<uint32_t> seen;
+    for (uint32_t bus = 0; bus <= 2047; bus += 256) {
+        for (uint32_t txn = 0; txn <= 255; txn += 85) {
+            auto key = pending_key(static_cast<avtp::ByteBusId>(bus), static_cast<uint8_t>(txn));
+            REQUIRE(seen.insert(key).second); // must be a fresh key, never seen before
+        }
+    }
+}
+
 // ── MultiFrame — multiple ACF requests in one AVTPDU (cpp-RCP-04-fresh) ──────
 
 TEST_CASE("MultiFrame round-trips two ACF_ABB messages packed into one AVTPDU",
@@ -563,6 +592,68 @@ TEST_CASE("Client::request correlates concurrent requests by byte_bus_id/transac
     client.close();
 }
 
+TEST_CASE("Client::request does not misdeliver or hang for two genuinely concurrent requests "
+          "whose byte_bus_id values differ by a multiple of 256 and share a transaction_num "
+          "(pending_key collision regression)",
+          "[udp][REQ-UDP-011]") {
+    Server server(make_stream_id(0x02, 6), "127.0.0.1", 0);
+    REQUIRE(server.ok());
+
+    // byte_bus_id 5's response is deliberately delayed so its request is
+    // still genuinely pending (inserted into Client::pending_, not yet
+    // erased) when byte_bus_id 261's request -- sharing the same
+    // transaction_num, and differing from 5 by exactly 256 -- gets inserted
+    // into that very same map from a second thread. Under the old uint16_t
+    // pending_key both requests computed the identical map key, so the
+    // second insertion silently clobbered the first's map entry; whichever
+    // response arrived first then got delivered to the WRONG promise (or,
+    // depending on arrival order, byte_bus_id 5's request never got a
+    // response delivered to it at all and hung until ctx's timeout).
+    server.set_handler(make_echo_handler([](const acf::AcfMessageInfo& req, const std::vector<uint8_t>&) {
+        if (req.byte_bus_id == 5) std::this_thread::sleep_for(std::chrono::milliseconds(150));
+        FrameResponse r;
+        r.info = acf::make_response(req, acf::ResponseKind::ReadResponse);
+        // Echo byte_bus_id back (big-endian u16) so this test can prove
+        // each request's OWN response actually came back, not merely that
+        // *a* response arrived — a misdelivered response under the bug
+        // still completes the waiting future, just with the wrong
+        // request's data.
+        r.payload = {static_cast<uint8_t>(req.byte_bus_id >> 8),
+                     static_cast<uint8_t>(req.byte_bus_id)};
+        return r;
+    }));
+
+    Client client(make_stream_id(0x03, 6), "127.0.0.1", server.port());
+    REQUIRE(client.ok());
+
+    acf::AcfMessageInfo   resp5, resp261;
+    std::vector<uint8_t>  payload5, payload261;
+    std::error_code       ec5, ec261;
+
+    std::thread t5([&]{
+        auto ctx = Context::with_timeout(std::chrono::seconds(2));
+        ec5 = client.request(ctx, standard_request(5, 7), {}, resp5, payload5);
+    });
+    std::this_thread::sleep_for(std::chrono::milliseconds(30)); // let t5's insert land first
+    std::thread t261([&]{
+        auto ctx = Context::with_timeout(std::chrono::seconds(2));
+        ec261 = client.request(ctx, standard_request(261, 7), {}, resp261, payload261);
+    });
+
+    t5.join();
+    t261.join();
+
+    REQUIRE_FALSE(ec5);
+    REQUIRE_FALSE(ec261);
+    REQUIRE(resp5.byte_bus_id   == 5);
+    REQUIRE(resp261.byte_bus_id == 261);
+    REQUIRE(payload5   == std::vector<uint8_t>{0, 5});
+    REQUIRE(payload261 == std::vector<uint8_t>{1, 5});
+
+    server.close();
+    client.close();
+}
+
 // ── Annex J encapsulation sequence number (real sockets) ────────────────────
 
 TEST_CASE("Client's Annex J encapsulation sequence number increments monotonically "