iloc: preserve versioned fields on encode

[kixelated]

Summary

• select iloc version 1 or 2 when construction methods, extent indexes, large item IDs, or large item counts require it
• widen extent offset, length, and reference-index fields to 64 bits when values exceed u32
• replace lossy numeric casts with checked conversions and reject values that cannot be represented
• add round-trip and boundary regression coverage

Root cause

Iloc::encode_body_ext always emitted version 0 with fixed 32-bit extent fields and no extent index field. Re-encoding decoded HEIF/AVIF metadata could therefore change the meaning of item locations or silently truncate values.

Impact

Versioned iloc data now round-trips without losing construction_method or item_reference_index, and values above the 32-bit and 16-bit boundaries are either encoded with the required representation or rejected explicitly.

Validation

• cargo test --all-targets (241 passed)
• cargo clippy --all-targets --all-features -- -D warnings
• cargo fmt --all -- --check
• git diff --check

Closes #191

[bradh]

Minor updates for valid construction method values. Otherwise LGTM.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 9507cc7f-5646-4caf-befe-1176f2b5c046

📥 Commits

Reviewing files that changed from the base of the PR and between 0d7d9fb and aa6e213.

📒 Files selected for processing (1)

• src/meta/iloc.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

Walkthrough

Iloc encoding now selects the required version and field widths from item and extent data. It validates construction methods and field limits, and returns errors when values cannot fit the selected fields. Tests cover version 1 and version 2 round trips, 64-bit extent values, and rejected inputs.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to aa6e2

Iloc encoding now preserves versioned fields and rejects values that cannot be represented, instead of silently truncating them. No merge-blocking risk was identified in the supplied context.

Security Architecture Review

Security architecture risk: 🔵 Low · up to aa6e2

The change improves metadata integrity by preserving versioned fields and avoiding silent numeric truncation. No concrete security bypass was identified. Remaining uncertainty concerns downstream reader compatibility, output limits, and handling of buffers after encoding errors.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The evidenced exposure is metadata re-encoding into a caller-owned output buffer. Larger collections and wider fields can produce larger output; application-level quotas, deployment exposure, and independently attackable scope are not established.

Trust Boundaries and Controls

• observed — The inspected flow remains decoded or constructed metadata passing through the existing atom serialization interface. Head adds representation checks rather than a new parser, identity transition, or privileged operation.

Resilience and Maintainability Implications

• observed — Iloc validation precedes body writes, but generic wrappers have already appended header placeholders. Errors return without rollback, and buffer reuse can retain those bytes. New rejection cases exercise this generic mechanism; no transactional promise or publication of failed output was established.

Hardening Proposals

• proposed — Make failed-buffer ownership explicit: require callers to discard failed output, or offer scratch-buffer encoding that appends only on success. This would strengthen failure containment without treating the existing nontransactional interface as a verified vulnerability.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the primary change: preserving versioned iloc fields during encoding.
Description check	✅ Passed	The description directly explains the iloc encoding changes, root cause, impact, validation, and linked issue.
Linked Issues check	✅ Passed	The changes satisfy the coding requirements in [#191]. Iloc::encode_body_ext selects version 1 for nonzero construction_method or item_reference_index, and selects version 2 for large item IDs o…
Out of Scope Changes check	✅ Passed	The pull request changes only src/meta/iloc.rs. The implementation and tests directly address [#191] by preserving versioned iloc fields and rejecting unrepresentable values. No unrelated change i…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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.}

[bradh]

@coderabbitai: full review

[coderabbitai]

@bradh, I will review all changes in PR #203 again.

⚠️ Action not completed

Pull request is closed.

---

Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 51 minutes.