A zlib chunk with no trailer is read to its declared size

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

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);
 

src/rwe/io/hpi/hpi_util.cpp

@@ -157,13 +157,24 @@ namespace rwe
         stream.avail_out = static_cast<uInt>(maxBytes);
         stream.next_out = reinterpret_cast<unsigned char*>(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<std::size_t> stringSize(const char* begin, const char* end)

src/rwe/io/hpi/hpi_util.test.cpp

@@ -0,0 +1,67 @@
+#include <catch2/catch_test_macros.hpp>
+#include <rwe/io/hpi/hpi_util.h>
+#include <string>
+#include <vector>
+#include <zlib.h>
+
+namespace rwe
+{
+    namespace
+    {
+        std::vector<char> deflated(const std::string& text)
+        {
+            std::vector<char> out(compressBound(static_cast<uLong>(text.size())));
+            uLongf outLen = static_cast<uLongf>(out.size());
+            REQUIRE(compress2(reinterpret_cast<Bytef*>(out.data()), &outLen, reinterpret_cast<const Bytef*>(text.data()), static_cast<uLong>(text.size()), 9) == Z_OK);
+            out.resize(outLen);
+            return out;
+        }
+
+        std::string inflated(const std::vector<char>& 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<char> 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<char> 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<char> junk(40, 'x');
+            std::string out(text.size(), '\0');
+            REQUIRE_THROWS_AS(decompressZLib(junk.data(), junk.size(), out.data(), text.size()), HpiException);
+        }
+    }
+}