From 197f44ae783af5dd00269ef8e8613be23d3f90b1 Mon Sep 17 00:00:00 2001 From: Hector Bailey Date: Fri, 11 Sep 2026 20:07:08 +0100 Subject: [PATCH] A zlib chunk with no trailer is read to its declared size The V Maps pack was written by HPIZ Archiver, which leaves the four-byte adler32 trailer off every zlib chunk. The original reads its bytes and never looks for the trailer; RWE's decompressor demanded a clean end of stream and threw "ZLib decompress failed" on every one of the pack's 628 maps, from the lobby as well as the loader. A chunk is now also complete when all of its input has been consumed and exactly the declared number of bytes came out. The chunk's own checksum still covers the compressed data, so a genuinely short or corrupt chunk is still refused, and a test pins both sides. The "Expected property name" crash on [V] A Better Fate from the same pack does not reproduce: with the chunks readable, all 628 OTAs parse. The lobby no longer lets a parse failure escape to the fatal handler either -- a malformed map shows the parser's line and column in a message box and leaves the previous selection standing. Issue: #8 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PJCbwC9MGZnM6erMDKptun --- CMakeLists.txt | 1 + src/rwe/MainMenuScene.cpp | 16 +++++++- src/rwe/io/hpi/hpi_util.cpp | 19 +++++++-- src/rwe/io/hpi/hpi_util.test.cpp | 67 ++++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 src/rwe/io/hpi/hpi_util.test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index fc3ef8cda..673a27169 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -882,6 +882,7 @@ set(TEST_FILES src/rwe/grid/Point.test.cpp src/rwe/io/featuretdf/io.test.cpp src/rwe/io/gui/gui.test.cpp + src/rwe/io/hpi/hpi_util.test.cpp src/rwe/io/ota/ota.test.cpp src/rwe/io/pcx/pcx.test.cpp src/rwe/scene/Screenshot.test.cpp diff --git a/src/rwe/MainMenuScene.cpp b/src/rwe/MainMenuScene.cpp index 283d0566f..d00076380 100644 --- a/src/rwe/MainMenuScene.cpp +++ b/src/rwe/MainMenuScene.cpp @@ -1305,7 +1305,21 @@ namespace rwe std::string otaStr(otaRaw->begin(), otaRaw->end()); - auto ota = parseOta(parseTdfFromString(otaStr)); + // A community map with a malformed OTA used to take the game down + // from this line with a Critical Error box (upstream #38, "Expected + // property name"). The parser says where it gave up; show that and + // leave the previous selection standing. + OtaRecord ota; + try + { + ota = parseOta(parseTdfFromString(otaStr)); + } + catch (const std::exception& e) + { + LOG_ERROR << "Could not read map " << mapName << ": " << e.what(); + openMessageBox("Could not read " + mapName + ": " + e.what()); + return; + } auto minimap = sceneContext.textureService->getMinimap(mapName); diff --git a/src/rwe/io/hpi/hpi_util.cpp b/src/rwe/io/hpi/hpi_util.cpp index ac5b58c05..32af1f631 100644 --- a/src/rwe/io/hpi/hpi_util.cpp +++ b/src/rwe/io/hpi/hpi_util.cpp @@ -157,13 +157,24 @@ namespace rwe stream.avail_out = static_cast(maxBytes); stream.next_out = reinterpret_cast(out); - if (inflate(&stream, Z_NO_FLUSH) != Z_STREAM_END) + auto result = inflate(&stream, Z_NO_FLUSH); + + // A chunk is complete when the stream ends, and also when every + // input byte has been consumed and exactly the declared number of + // output bytes came out. The second case is a stream with no + // adler32 trailer: HPIZ Archiver wrote thousands of those into the + // V Maps pack, and the original engine, which only ever asks for + // its bytes, reads them without complaint. The chunk's own checksum + // still covers the compressed data, so nothing is taken on trust. + auto complete = result == Z_STREAM_END + || ((result == Z_OK || result == Z_BUF_ERROR) && stream.avail_in == 0 && stream.total_out == maxBytes); + + inflateEnd(&stream); + + if (!complete) { - inflateEnd(&stream); throw HpiException("ZLib decompress failed"); } - - inflateEnd(&stream); } std::optional stringSize(const char* begin, const char* end) diff --git a/src/rwe/io/hpi/hpi_util.test.cpp b/src/rwe/io/hpi/hpi_util.test.cpp new file mode 100644 index 000000000..993a8a2dc --- /dev/null +++ b/src/rwe/io/hpi/hpi_util.test.cpp @@ -0,0 +1,67 @@ +#include +#include +#include +#include +#include + +namespace rwe +{ + namespace + { + std::vector deflated(const std::string& text) + { + std::vector out(compressBound(static_cast(text.size()))); + uLongf outLen = static_cast(out.size()); + REQUIRE(compress2(reinterpret_cast(out.data()), &outLen, reinterpret_cast(text.data()), static_cast(text.size()), 9) == Z_OK); + out.resize(outLen); + return out; + } + + std::string inflated(const std::vector& in, std::size_t declaredSize) + { + std::string out(declaredSize, '\0'); + decompressZLib(in.data(), in.size(), out.data(), declaredSize); + return out; + } + } + + TEST_CASE("decompressZLib", "[hpi]") + { + const std::string text = "[GlobalHeader]\n{\nmissionname=A Better Fate;\n}\n"; + auto stream = deflated(text); + + SECTION("a whole stream comes back as written") + { + REQUIRE(inflated(stream, text.size()) == text); + } + + SECTION("a stream with no adler32 trailer still yields its declared bytes") + { + // What HPIZ Archiver writes: the deflate blocks are all there and + // the last one is final, but the four trailer bytes are not. + // The original reads these, and the V Maps pack is full of them. + std::vector untrailed(stream.begin(), stream.end() - 4); + REQUIRE(inflated(untrailed, text.size()) == text); + } + + SECTION("a stream cut short of its declared bytes is still refused") + { + std::vector cut(stream.begin(), stream.end() - 12); + std::string out(text.size(), '\0'); + REQUIRE_THROWS_AS(decompressZLib(cut.data(), cut.size(), out.data(), text.size()), HpiException); + } + + SECTION("a stream that would overrun its declared size is refused") + { + std::string out(text.size() - 5, '\0'); + REQUIRE_THROWS_AS(decompressZLib(stream.data(), stream.size(), out.data(), out.size()), HpiException); + } + + SECTION("garbage is refused") + { + std::vector junk(40, 'x'); + std::string out(text.size(), '\0'); + REQUIRE_THROWS_AS(decompressZLib(junk.data(), junk.size(), out.data(), text.size()), HpiException); + } + } +}