fix: drop unreachable sub-conditions from the cbor_decode corrupt-meta guard

[thedavidmeister]

Closes #190.

The issue asked for a decision: keep the two unreachable sub-conditions as defensive redundancy, or simplify so every guard is exercisable. This takes the second option — a guard that reads as if it enforces a check it cannot enforce is the liability, and a comment saying so would be a second one.

error.offset() == len is not a check

Re-derived from the serde_cbor 0.11.2 source rather than only empirically. Over a slice there are exactly two ways an eof error is built:

• SliceRead::end (src/read.rs) raises EofWhileParsingValue with offset self.slice.len();
• Deserializer::error (src/de.rs) stamps self.read.offset(), and the eof codes only reach it from a next()/peek() that returned None, which for SliceRead means index == slice.len().

Deserializer::from_slice uses SliceRead, and len in cbor_decode is exactly that slice's length (data.len(), minus the 8 prefix bytes when the slice is data[8..]). So is_eof() implies offset() == len, and the else arm mapping "eof at the wrong offset" to SerdeCborError could not run. The eof arm now just ends the loop; trailing truncation is caught where it always actually was, by len != consumed.

track collapses to one offset

track.push and metas.push are in the same iteration with an early return between them, so track.len() != metas.len() cannot hold at the guard and track.is_empty() cannot differ from metas.is_empty(). Removing only the length cross-check would have left track.is_empty() as a fresh equivalent mutant, so the whole Vec goes: it only ever had its last element read, and it is now a single consumed: usize. That also drops the track[track.len() - 1] index whose bounds safety rested on the redundant track.is_empty().

Both surviving sub-conditions are exercisable, and the mutation evidence below shows each one killed.

Behaviour

None intended. No input changes result or error variant; the diff removes conditions no input can make evaluate differently.

Overlap with in-flight work

Textual conflicts in cbor_decode are likely with #186, #188, #191 and #203, which change what the decode loop does with items (drop unknown magics, drop items missing keys 0/1, reject duplicate map keys) and with the document prefix. This change is orthogonal in intent — it removes dead sub-conditions and changes no accept/reject decision — so whichever lands second should keep the other's semantics and rebase the guard onto them. #187 (adding a KnownMagic variant) does not touch this function.

QA

• Discriminating tests: test_cbor_decode_truncated_item_is_corrupt — does NOT fail on base, and cannot: the diff removes sub-conditions no input can make observable, so it is behaviour-preserving by construction, which is the issue's own finding. Verified by reconstructing the pre-fix cbor_decode byte-for-byte in the tree with the new test present and running cargo test -p rain-metadata --lib meta::tests::test_cbor_decode — 7 passed, 0 failed. It discriminates against mutants of the surviving guard instead (next line). What it adds over the existing tests is the EofWhileParsingMap path ([0xa2, 0x00, 0x41, 0x01], a map header promising an entry the input does not carry), which no existing test reaches; the other two cases pin eof via SliceRead::end for a truncated sole item and for a truncated item after a complete one.
• Mutations applied: each mutation applied to the post-fix cbor_decode, suite run, mutation reverted.
  • if metas.is_empty() || len != consumed → if len != consumed → killed by test_cbor_decode_empty_is_corrupt (1 failed).
  • if metas.is_empty() || len != consumed → if metas.is_empty() → killed by test_cbor_decode_trailing_truncated_is_corrupt and test_cbor_decode_truncated_item_is_corrupt (2 failed).
  • eof arm false → Err(Error::SerdeCborError(error))? (loop no longer ends on eof) → killed by test_cbor_decode_empty_is_corrupt, test_cbor_decode_trailing_truncated_is_corrupt, test_cbor_decode_truncated_item_is_corrupt (3 failed).
  • consumed = deserializer.byte_offset() → consumed = len → killed by test_cbor_decode_trailing_truncated_is_corrupt and test_cbor_decode_truncated_item_is_corrupt (2 failed).
• Oracle: the serde_cbor 0.11.2 source for which offset each eof error carries (SliceRead::end and Deserializer::error, cited above) — not the behaviour of cbor_decode; and RFC 8949 head bytes for the test inputs, which are handwritten byte by byte (handwritten_map() plus 0xa2 0x00 …) rather than produced by cbor_encode.
• Category check: the issue asks for a decision on two sub-conditions — (1) the eof offset() == len equality, (2) track.len() != metas.len() — and offers keep-with-comment or simplify. Both covered by simplifying; the third sub-condition track.is_empty(), which (2)'s removal would have turned into a new equivalent mutant, is covered too.

Also run locally: cargo test -p rain-metadata --lib meta::tests:: 51 passed, cargo clippy -p rain-metadata --lib --all-targets clean, cargo fmt --check clean. The full suite was not run locally by request (many sibling agents on this machine); CI runs it.

[coderabbitai]

Warning

Review limit reached

Next included review available in 27 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 942591ba-b0ce-453a-ac9c-9ad2135925ab

📥 Commits

Reviewing files that changed from the base of the PR and between 45ca96c and 63d496a.

📒 Files selected for processing (1)

• crates/cli/src/meta/mod.rs

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}