Repository navigation
Never crash: Refuse an ERF or MEF whose raw strip runs past the file (#2119) - #2155
endthestart wants to merge 5 commits into
Conversation
…rip-length # Conflicts: # docs/whats-new.md
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
ERF and MEF strip length checks crates/raw/src/vendor/erf.rs, crates/raw/src/vendor/mef.rs, docs/whats-new.md |
Both decoders return corruption errors when the first strip is unavailable or its byte length differs from its declared length. Regression tests verify that format probing succeeds while probe_info and decode return corruption errors for oversized declarations. The release notes describe the change. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related issues
- Raw vendor decoders (ERF, MEF, ORF, ARW) size the sensor buffer from declared strip byte counts, not the bytes present #2561 describes related declared-strip-length behavior across several raw formats. This PR addresses ERF and MEF, but does not change the other formats named in the issue.
Possibly related PRs
- storytold/lightcraft#2740: Adds equivalent strip-length validation for ARW and SRW.
- storytold/lightcraft#2780: Adds a MEF raw-strip check for insufficient bytes before unpacking.
Suggested reviewers: dylanmtaylor, fredapps
Merge Risk: 
ab3f6
-
ERF and MEF imports now reject incomplete raw strips. No outstanding issue in the supplied evidence prevents merging after normal checks.
Architecture risk: 
ab3f6
-
The change affects 2 systems.
Changed systems:
crates,docsArchitecture concerns
No architecture-level concerns identified.Review details
-
Systems and components
- observed — crates (service) was modified; 2 changed files map to changed impact.
- observed — docs (service) was modified; 1 changed file maps to changed impact.
Before / after behavior
- observed — Modified behavior in crates/raw/src/vendor/erf.rs: The decoder now requires the first strip to be readable and its actual byte length to equal the declared chunk length; otherwise it returns a corruption error. Previously, any readable first strip was accepted without checking that length.
- observed — Modified behavior in crates/raw/src/vendor/erf.rs: Adds the test helper
declare, which rewrites the raw IFD’s image height, rows per strip, and strip byte count in the TIFF bytes, handling both short and long tag values. - observed — Modified behavior in crates/raw/src/vendor/erf.rs: Adds a regression test with a strip declared as 2,000 rows and 64,000 bytes despite having only the original two-row data. It checks that ERF probing succeeds, while
probe_infoanddecodereturnRawError::Corrupt. - observed — Modified behavior in crates/raw/src/vendor/mef.rs: The decoder now requires the bytes obtained for the first strip to match its declared length exactly; missing or short strip data returns a corruption error before sample-buffer allocation.
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title names the main change: ERF and MEF decoding now refuses a raw strip that extends past the end of the file. The raw summary confirms that both erf.rs and mef.rs reject such strips with a Corr… Linked Issues check Issue #2119is closed and completed, so it supplies historical context only. No active directly linked issue supplies coding requirements for this pull request.Out of Scope Changes check The ERF and MEF strip-bound checks, regression tests, and the related whats-new entry address the reported raw-strip allocation problem. The available summary shows no unrelated change. Full details: Title check
-
Explanation
The title names the main change: ERF and MEF decoding now refuses a raw strip that extends past the end of the file. The raw summary confirms that both erf.rs and mef.rs reject such strips with a Corrupt error, and that the docs and tests cover the same behavior. The title's '
#2119' reference matches the linked issue, but the PR objectives cite#2155in the description, a numbering mismatch that does not affect the title's relevance. The title is 76 characters, slightly above a short single-line ideal, but it is specific and clear.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
…rip-length # Conflicts: # docs/whats-new.md
…st the file Port of storytold#2155: the header probe sized its sample buffer from the declared strip length; a short strip is now a Corrupt error. Tests adapted to the fork's arithmetic lint. Sources: own design (upstream PR). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Upstream storytold#2155: refuse ERF/MEF whose raw strip runs past the file
…p-length # Conflicts: # crates/raw/src/vendor/mef.rs
The ERF and MEF readers picked the raw IFD by checking that StripByteCounts matched width × height, but they never checked that the file actually holds that many bytes. A tiny file declaring a huge frame got through, and the header probe then allocated the full sample buffer to measure white balance, so a few KB could cost gigabytes during import.
Now both decoders refuse a strip that runs past the end of the file before they size anything from it.
I added a test for each format that takes a small valid sample, rewrites its height and strip length to claim a much larger frame, and checks that probe_info and decode return a Corrupt error. Both failed before the change. Let me know if you'd rather fix it another way. Thank you.
Fixes #2119
Summary by CodeRabbit