Dont clone full frame for error

[bit-aloo]

closes: #2315
closes: https://github.com/project-loupe/audit-stratum/issues/108
closes: https://github.com/project-loupe/audit-stratum/issues/181
closes: https://github.com/project-loupe/audit-stratum/issues/177
closes: https://github.com/project-loupe/audit-stratum/issues/167

[GitGab19]

Clanker review:

---

Review summary

Overall, the implementation looks correct and directly addresses the reported allocation-amplification issue. The decoder now retains only the 1–3-byte length prefix instead of cloning the entire remaining frame.

Non-blocking documentation issue

The new ValueExceedsMaxSize documentation describes its Vec<u8> as a “bounded diagnostic sample.” That is only true for the decoded-length path changed here.

Other paths in validate_payload still use value.to_vec(), potentially retaining the complete caller-provided invalid value. I suggest qualifying the documentation, for example:

The Vec<u8> contains the offending value when supplied directly, or only the length prefix when an overflow is detected from a declared encoded length.

Hardening the other constructors against large invalid inputs could be tracked separately; I would not consider it a blocker for this PR.

---

Can you check if other constructors have a similar issue to the one you're solving on this PR?

[bit-aloo]

Other places where we use it, is for our own internal construction and not by the caller generated payload.

[GitGab19]

Other places where we use it, is for our own internal construction and not by the caller generated payload.

Clanker answer to this:

---

Since it's a small change, let's handle the construction-side paths in this PR too rather than a follow-up: in validate_payload, replace both value.to_vec() calls with a capped sample, e.g. value[..value.len().min(32)].to_vec(). The full length is already reported in the last field of ValueExceedsMaxSize, so no diagnostic info is lost — and it makes the new doc comment ("bounded diagnostic sample") true for every path that builds this error. Please add a small test like your oversized_length_error_retains_only_the_header one covering Inner::new / InnerOwned::new with an oversized value.

[GitGab19]

Requested changes

The new ERROR_SAMPLE_LEN = 32 limits how many bytes of an invalid value are retained inside ValueExceedsMaxSize.

The value 32 is not a protocol limit. It is simply a diagnostic-memory cap: errors retain the first 32 bytes for debugging while the final field of ValueExceedsMaxSize records the complete input length.

Validate before creating an owned copy

TryFrom<&[u8]> for InnerOwned still copies the complete input before validating it:

Self::new(value.to_vec())

A large invalid value therefore causes a large temporary allocation even though the resulting error retains only 32 bytes.

Please validate first and allocate only when the value is valid:

fn try_from(value: &[u8]) -> Result<Self, Self::Error> {
    validate_payload::<ISFIXED, SIZE, HEADERSIZE, MAXSIZE>(value)?;

    Ok(Self {
        data: value.to_vec(),
    })
}

The variable-array conversions should similarly validate before allocating, preferably by delegating to the slice implementation:

fn try_from(value: [u8; N]) -> Result<Self, Self::Error> {
    Self::try_from(&value[..])
}

fn try_from(value: &[u8; N]) -> Result<Self, Self::Error> {
    Self::try_from(&value[..])
}

fn try_from(value: &mut [u8; N]) -> Result<Self, Self::Error> {
    Self::try_from(&value[..])
}

Please also exercise the owned-from-slice path in the regression test:

match B032Owned::try_from(oversized.as_slice()).unwrap_err() {
    Error::ValueExceedsMaxSize(false, 1, 1, 32, retained, 1_048_576) => {
        assert_eq!(retained.len(), ERROR_SAMPLE_LEN);
    }
    other => panic!("unexpected error: {other:?}"),
}

Clarify the documentation

“Never a full copy” is not strictly correct because invalid values of 32 bytes or fewer are copied completely. Suggested wording:

The Vec<u8> is a bounded diagnostic sample containing at most the first 32 bytes of the offending value, or only the length prefix when the overflow is detected from a declared encoded length. The final field reports the complete offending length.

The constant should also explain why 32 was selected:

// Retain one hash-sized prefix for diagnostics while keeping error allocations bounded.
const ERROR_SAMPLE_LEN: usize = 32;