build_crc32_coverage_buffer_for_fragment_train never recomputes acf_msg_length from the combined payload (distinct from #169's header-model question)

[SoundMatt]

Summary

Investigating issue #169 (REQ-CRC-015/016 vs the RC4/RC5 TC18.txt revision) surfaced a distinct, concrete implementation bug in build_crc32_coverage_buffer_for_fragment_train (src/e2e.rs) that exists independent of #169's own open question (which fragment's header TC18 wants for CRC coverage). No matter how #169 resolves, this function's acf_msg_length field is wrong today.

The bug

pub fn build_crc32_coverage_buffer_for_fragment_train(
    header: &HeaderVariant,
    final_fragment: &AcfCoverageMessage,
    segments: &[&[u8]],
) -> Result<Vec<u8>, RcpError> {
    let combined = CombinedFragmentPayload::assemble(segments);
    match final_fragment {
        AcfCoverageMessage::Abb(msg) => {
            let combined_msg = AcfAbbMessage {
                info: msg.info,           // <-- acf_msg_length taken verbatim from ONE fragment
                payload: combined.0,       // <-- payload is ALL fragments concatenated
            };
            build_crc32_coverage_buffer(header, &AcfCoverageMessage::Abb(&combined_msg))
        }
        // ... Gbb arm identical
    }
}

combined.0 is the full reassembled payload — every segment concatenated (CombinedFragmentPayload::assemble). But msg.info (which carries acf_msg_length) is reused unmodified from final_fragment's own header — i.e. whatever acf_msg_length that single fragment carried on the wire, describing only that fragment's own (much smaller) size.

build_crc32_coverage_buffer then does nothing to reconcile this — it takes adjusted_info.acf_msg_length (still the single-fragment value) and only adds a constant +1 quadlet (CRC32_COVERAGE_LENGTH_PREADJUST_QUADLETS) for the CRC trailer itself. It never derives a length from payload.len().

The existing unit test documents this as intended, not as a bug, which is why it's shipped unnoticed:

let final_info = acf::ByteMessageInfo { acf_msg_length: 0x20, ms: false, ..Default::default() };
// ...
// "The final fragment's own payload field is irrelevant to the train
// buffer — only its header fields (info) are consulted; the payload
// region comes from `segments` instead."

The test never questions whether acf_msg_length: 0x20 (32 quadlets — presumably describing the final fragment's own on-wire size) should instead reflect the combined payload's real length. It just asserts the (inconsistent) current behavior against itself.

Why this is a real bug regardless of #169's outcome

Issue #169 asks: should the CRC coverage header come from the first fragment, the final fragment, or a synthesized whole-message header (a third model current TC18.txt arguably implies)? That question is genuinely open.

This bug is orthogonal to that: under any of those three answers, a coverage buffer computed over the full reassembled payload needs a length field that actually describes that payload's real size — not an arbitrary individual fragment's own smaller count. A conformant peer reconstructing the same CRC would derive acf_msg_length from the real total message size; this crate's current code never does that for any header-model choice.

Scope / exposure

• e2e/fragment are pub mod, and build_crc32_coverage_buffer_for_fragment_train/crc32_tc18_for_fragment_train/fragment::verify_reassembled_train_crc are all pub fn — reachable by an external integrator using this crate directly.
• Confirmed via full-repo grep: none of these three functions has a call site anywhere outside their own #[cfg(test)] modules. No transport module, dispatch path, or mock.rs-equivalent composition calls into this fragment-train CRC chain today. This is not currently exercised by any live wire-facing behavior in this crate.
• So: a real defect-in-waiting in public API surface, not (yet) a shipping conformance failure.

Recommendation

This needs a real fix, not a citation change, and depends on #169's own header-model resolution (which header's other fields — stream_id, avtp_timestamp, acf_msg_type — to use is still an open question). But independent of that: build_crc32_coverage_buffer_for_fragment_train should derive acf_msg_length from combined.0.len() (converted to quadlets, consistent with build_crc32_coverage_buffer's own existing +1-quadlet CRC-trailer adjustment), not copy it from final_fragment's own header. The existing test (coverage_buffer_for_fragment_train_matches_manual_concatenation) will need its own fixture updated once this is fixed, since it currently asserts the buggy behavior.

Given this touches a pub fn's contract and interacts with #169's still-open header-model question, this is left for human/engineering judgment rather than an autonomous fix — same disposition this session has used elsewhere for safety-relevant contract changes (e.g. c-RCP issue #600).

References

• Issue Investigate REQ-CRC-015/REQ-CRC-016 against RC4/RC5 TC18.txt: old supporting spec text appears removed, not just relocated #169 (open) — the header-model ambiguity this bug is distinct from but related to
• src/e2e.rs — build_crc32_coverage_buffer_for_fragment_train (the buggy function), coverage_buffer_for_fragment_train_matches_manual_concatenation (the test documenting current, inconsistent behavior)
• src/fragment.rs — verify_reassembled_train_crc (the only real caller, itself test-only today)