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

jqueuniet commented

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

Owner

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

Author

Owner

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

jqueuniet added this to the CLI robustness & polish milestone

2026-07-23 20:52:10 +02:00

jqueuniet added the

area/cli

type/bug

labels

2026-07-23 20:52:43 +02:00

jqueuniet added this to the dm2-tools project

2026-07-23 21:43:33 +02:00

jqueuniet referenced this issue from a commit

2026-09-04 14:01:28 +02:00

fix(cli): surface typed parse errors for corrupt savegames (#206)

jqueuniet referenced this issue

2026-09-04 14:01:43 +02:00

fix(cli): surface typed parse errors for corrupt savegames (#206) #317

jqueuniet commented

2026-09-04 15:57:35 +02:00

Author

Owner

Fixed by #317 (merged).

detect_save is now Result<Option<_>> like its try_parse_graphics / try_parse_ftl / try_open_resource_fork siblings: the looks_like_* header fingerprint stays the family gate, and once it matches, SaveFile::parse's error is propagated instead of being .ok()-ed away. Both the LE (dos-retail / dos-beta-v09) and BE (mac-retail) arms are covered, and all five call sites (info, verify, dump, raw-extract, save via load_save) surface it while keeping their generic message for the genuine no-match case.

The reproduction from the reopening comment now reports the precise cause:

$ head -c 5000 fixtures/dos-en/saves/sksave0.dat > trunc5k.dat
$ dm2 info trunc5k.dat
Error: trunc5k.dat
Caused by:
    matched the dos-retail savegame fingerprint but failed to parse: sksave truncated:
    bitstream_offset 43485 >= file_len 5000 (file too small for computed raw-section layout) at 0xa9dd

Fixed by #317 (merged). `detect_save` is now `Result<Option<_>>` like its `try_parse_graphics` / `try_parse_ftl` / `try_open_resource_fork` siblings: the `looks_like_*` header fingerprint stays the family gate, and once it matches, `SaveFile::parse`'s error is propagated instead of being `.ok()`-ed away. Both the LE (dos-retail / dos-beta-v09) and BE (mac-retail) arms are covered, and all five call sites (`info`, `verify`, `dump`, `raw-extract`, `save` via `load_save`) surface it while keeping their generic message for the genuine no-match case. The reproduction from the reopening comment now reports the precise cause: $ head -c 5000 fixtures/dos-en/saves/sksave0.dat > trunc5k.dat $ dm2 info trunc5k.dat Error: trunc5k.dat Caused by: matched the dos-retail savegame fingerprint but failed to parse: sksave truncated: bitstream_offset 43485 >= file_len 5000 (file too small for computed raw-section layout) at 0xa9dd

jqueuniet closed this issue

2026-09-04 15:57:38 +02:00

jqueuniet referenced this issue from a commit

2026-09-04 18:32:06 +02:00

docs(changelog): backfill the Unreleased section

jqueuniet referenced this issue

2026-09-04 18:32:14 +02:00

docs(changelog): backfill the Unreleased section #318

Rows
Columns

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

Problem

Fix direction