dm2-cli: auto-detect chains swallow typed parse errors — corrupt files of recognised families report generic 'could not identify' #206

New issue

Open

opened 2026-07-05 11:58:45 +02:00 by jqueuniet · 1 comment

jqueuniet commented

2026-07-05 11:58:45 +02:00

(Migrated from codeberg.org)

Problem

The CLI's format auto-detection swallows the useful diagnostics. dm2 info, dm2 extract, and (partially) dm2 verify probe formats with if let Ok(…) chains (crates/dm2-cli/src/commands/{info.rs,extract.rs}), so a corrupt file of a recognised family falls through every probe and dies with a generic message:

A graphics.dat with the 0x8000 signature but a truncated payload region produces the precise typed error PayloadCoverageMismatch { computed, file } inside Archive::parse — which the chain discards, reporting only "could not identify " / "not a recognized archive format".
Same for an FTL file with the 0x6160 magic but an out-of-bounds hunk (TruncatedHunk { .. } discarded), or a MacBinary-wrapped fork with a bad AppleDouble version.

For a tool whose users routinely feed it half-understood dumps, "the file matched graphics.dat's signature but failed to parse: " is the difference between a bug report and a shrug.

Fix direction

Two-stage dispatch: a cheap magic pre-check per family (graphics.dat sig & 0x8000, FTL 0x6160, HMP HMIMIDIP, MacBinary/AppleDouble sniff), and when the magic matches but the full parse fails, surface that family's parse error instead of continuing the chain. Families without a magic (dungeon.dat, saves) keep the current plausibility-probe behaviour. verify already reports per-family results and needs only the graphics/FTL arms adjusted; info and extract gain the most.

## Problem The CLI's format auto-detection swallows the useful diagnostics. `dm2 info`, `dm2 extract`, and (partially) `dm2 verify` probe formats with `if let Ok(…)` chains (`crates/dm2-cli/src/commands/{info.rs,extract.rs}`), so a **corrupt file of a recognised family** falls through every probe and dies with a generic message: - A graphics.dat with the `0x8000` signature but a truncated payload region produces the precise typed error `PayloadCoverageMismatch { computed, file }` inside `Archive::parse` — which the chain discards, reporting only "could not identify <path>" / "not a recognized archive format". - Same for an FTL file with the `0x6160` magic but an out-of-bounds hunk (`TruncatedHunk { .. }` discarded), or a MacBinary-wrapped fork with a bad AppleDouble version. For a tool whose users routinely feed it half-understood dumps, "the file matched graphics.dat's signature but failed to parse: <typed error>" is the difference between a bug report and a shrug. ## Fix direction Two-stage dispatch: a cheap magic pre-check per family (graphics.dat `sig & 0x8000`, FTL `0x6160`, HMP `HMIMIDIP`, MacBinary/AppleDouble sniff), and when the magic matches but the full parse fails, surface that family's parse error instead of continuing the chain. Families without a magic (dungeon.dat, saves) keep the current plausibility-probe behaviour. `verify` already reports per-family results and needs only the graphics/FTL arms adjusted; `info` and `extract` gain the most.

jqueuniet commented

2026-07-16 08:27:28 +02:00

(Migrated from codeberg.org)

Reopening: the fix (51170f7a) closed this for the magic-bearing families (graphics.dat, FTL, resource forks) but the savegame family was left with the original swallow-the-error behaviour, so the exact symptom this issue describes is still reproducible for saves.

The fix commit message states "Magic-less families (dungeon.dat, saves) already propagated post-probe errors and are unchanged." That is not true for saves. detect_save (crates/dm2-cli/src/commands/mod.rs:59-79) runs the full parse and discards the typed error with .ok():

if let Some(v) = variant {
    return dm2_saves::SaveFile::parse(bytes, Little)
        .ok()                       // <- collapses a recognised-but-corrupt save into None
        .map(|s| (v, Little, s));
}

Unlike the try_parse_graphics / try_parse_ftl / try_open_resource_fork siblings (all Result<Option<_>>, so a fingerprint match plus a parse failure surfaces the typed error), detect_save folds "recognised family, corrupt" into the same None as "not a save at all."

Reproduction (the header, which carries the w_timer_flag fingerprint, stays intact):

$ head -c 5000 fixtures/dos-en/saves/sksave0.dat > truncated.dat   # full file is 51553 B
$ dm2 info truncated.dat
Error: could not identify truncated.dat
$ dm2 verify truncated.dat
Error: no known format matched for truncated.dat

The library already computes the precise cause inside SaveFile::parse (e.g. "sksave truncated in map_table: need N bytes, got M", crates/dm2-saves/src/sksave.rs:373-382); the CLI throws it away. The dm2 save show/champion/global/party/inventory subcommands route through load_save (commands/save/mod.rs:138-149) and report the same generic "{file} is not a recognised DM2 savegame" for the same input.

Fix direction (unchanged from the original): make detect_save return Result<Option<(...)>> like its try_parse_* siblings — keep the cheap header / looks_like_* probe as the family gate, and once a variant fingerprint matches, propagate SaveFile::parse's Err instead of .ok()-ing it; update the four call sites to ? the outer Result.

Reopening: the fix (51170f7a) closed this for the magic-bearing families (graphics.dat, FTL, resource forks) but the savegame family was left with the original swallow-the-error behaviour, so the exact symptom this issue describes is still reproducible for saves. The fix commit message states "Magic-less families (dungeon.dat, saves) already propagated post-probe errors and are unchanged." That is not true for saves. `detect_save` (crates/dm2-cli/src/commands/mod.rs:59-79) runs the *full* parse and discards the typed error with `.ok()`: ```rust if let Some(v) = variant { return dm2_saves::SaveFile::parse(bytes, Little) .ok() // <- collapses a recognised-but-corrupt save into None .map(|s| (v, Little, s)); } ``` Unlike the `try_parse_graphics` / `try_parse_ftl` / `try_open_resource_fork` siblings (all `Result<Option<_>>`, so a fingerprint match plus a parse failure surfaces the typed error), `detect_save` folds "recognised family, corrupt" into the same `None` as "not a save at all." Reproduction (the header, which carries the `w_timer_flag` fingerprint, stays intact): ``` $ head -c 5000 fixtures/dos-en/saves/sksave0.dat > truncated.dat # full file is 51553 B $ dm2 info truncated.dat Error: could not identify truncated.dat $ dm2 verify truncated.dat Error: no known format matched for truncated.dat ``` The library already computes the precise cause inside `SaveFile::parse` (e.g. "sksave truncated in map_table: need N bytes, got M", crates/dm2-saves/src/sksave.rs:373-382); the CLI throws it away. The `dm2 save show/champion/global/party/inventory` subcommands route through `load_save` (commands/save/mod.rs:138-149) and report the same generic "{file} is not a recognised DM2 savegame" for the same input. Fix direction (unchanged from the original): make `detect_save` return `Result<Option<(...)>>` like its `try_parse_*` siblings — keep the cheap header / `looks_like_*` probe as the family gate, and once a variant fingerprint matches, propagate `SaveFile::parse`'s `Err` instead of `.ok()`-ing it; update the four call sites to `?` the outer `Result`.

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

jqueuniet/dm2-tools#206

No description provided.

Rows
Columns