Skip to content

Add freeze MPQ unit tests and fix 64-bit memory bugs - #178

Merged
armoha merged 2 commits into
mainfrom
mpq-unit-tests
Sep 15, 2026
Merged

armoha merged 2 commits into
mainfrom
mpq-unit-tests

Conversation

@armoha

@armoha armoha commented Sep 15, 2026

Copy link
Copy Markdown
Owner

Adds a mpq_unit_tests target (doctest) for the freeze MPQ code and fixes the
real bugs the new tests exposed. CI runs it on Windows/macOS/Ubuntu per push and PR.

Tests

  • New mpq/test/test_keycalc_crypt.cpp: T/mix known answers, mix/unmix
    roundtrip, HashString semantics, Encrypt/Decrypt roundtrip,
    GetFileDecryptKey recovery, keycalc determinism + input guards,
    deterministic garbagify overload.
  • Existing compression roundtrip test now actually runs (it was never wired
    into CMake) — and it caught a heap overflow on first run.
  • test_cmpdcmp.cpp: compare via size + memcmp instead of CHECK(str == str);
    doctest has no std::string overload, so the latter instantiates
    operator<<(ostream, string), which fails to link on Xcode Clang.

Fixes (behavior-preserving except the overflow)

  • Heap overflow in PKWARE explode/implode on 64-bit: the work structs
    assume 32-bit long/pointers and outgrew EXP_BUFFER_SIZE/CMP_BUFFER_SIZE.
    Buffers enlarged with compile-time fit asserts (exp.c, imp.c, pklib.h).
  • uLong/LPDWORD width mismatch in Deflate/Inflate (scomp.cpp):
    casting 32-bit LPDWORD to 64-bit unsigned long* corrupts memory on LP64.
    Now bounces through a uLong temp.
  • keycalc: null/tiny-table/zero-sector guards (throw instead of UB),
    explicit wrapping-arithmetic docs + fixed bit-identity notes vs freeze/.
  • GetFileDecryptKey: RAII buffer (exception-safe), null check, memcpy
    instead of unaligned uint32_t* dereference.
  • garbagifyHashTable: deterministic (table, max, gen&) overload for tests;
    production path unchanged (still random_device).
  • Remove unimplemented getFileCount/getBlockEntryCount declarations;
    catch (const std::exception&) instead of slicing catch (runtime_error);
    system("pause") is _WIN32-only.

CI

  • New .github/workflows/test.yml: builds + runs mpq_unit_tests on the
    3-OS matrix using a bare uv venv (no full dep sync needed).
  • FREEZE_BUILD_TESTS option (default ON) in mpqprt/CMakeLists.txt.

Verification

  • mpq_unit_tests: 8/8 cases, 61/61 assertions pass (also clean under ASan).
  • lib/freezeMpq.so rebuilt with Python 3.14t; full ed_profile run passes
    (9.25s, no regression vs 9.23s before).

- Add mpq_unit_tests target (doctest): crypt/keycalc known-answer,
  roundtrip, determinism/guard, garbagify, and compression tests
- Fix heap overflow in PKWARE explode/implode work buffers on 64-bit
  (structs outgrew EXP/CMP_BUFFER_SIZE); add fit asserts
- Fix uLong/LPDWORD width mismatch in Deflate/Inflate (LP64)
- Harden keycalc/GetFileDecryptKey inputs, RAII decrypt buffer,
  deterministic garbagify overload for tests
- CI: run mpq unit tests on Windows/macOS/Ubuntu per push and PR
doctest has no std::string overload, so CHECK(str == str) instantiates
operator<<(ostream, string), which fails to link on some toolchains
(Xcode Clang). Compare via size + memcmp instead.
@armoha
armoha merged commit 075f745 into main Sep 15, 2026
6 checks passed
@armoha
armoha deleted the mpq-unit-tests branch September 15, 2026 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant