fix(zip): write an entry's size into its local header

CHANGELOG.md

@@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- A saved document opens in LibreOffice again: every zip entry's size goes into
+  its local header instead of a trailing data descriptor, which LibreOffice
+  rejects on a stored entry — an odf package always stores `mimetype`.
 - `HtmlConfig::min_content_margin` puts a floor under the distance the
   generated content keeps from the view's border, per side. A set side raises
   the inset a view already has, never lowers it; a sheet is never inset. Bound

src/odr/internal/zip/zip_archive.cpp

@@ -9,6 +9,7 @@
 #include <odr/internal/zip/zip_util.hpp>
 
 #include <algorithm>
+#include <ostream>
 #include <string>
 
 #include <miniz/miniz.h>
@@ -72,20 +73,38 @@ std::shared_ptr<abstract::Filesystem> ZipArchive::as_filesystem() const {
   return filesystem;
 }
 
+namespace {
+
+struct WriteSink {
+  std::ostream *out{};
+  std::streamoff base{};
+};
+
+} // namespace
+
 void ZipArchive::save(std::ostream &out) const {
   bool state{};
   const auto time =
       std::chrono::system_clock::to_time_t(std::chrono::system_clock::now());
 
+  // miniz addresses the output by absolute offset and rewrites local headers.
+  WriteSink sink{&out, static_cast<std::streamoff>(out.tellp())};
+
   mz_zip_archive archive{};
-  archive.m_pIO_opaque = &out;
-  archive.m_pWrite = [](void *opaque, std::uint64_t /*offset*/,
+  archive.m_pIO_opaque = &sink;
+  archive.m_pWrite = [](void *opaque, const std::uint64_t offset,
                         const void *buffer, const std::size_t size) {
-    const auto o = static_cast<std::ostream *>(opaque);
-    o->write(static_cast<const char *>(buffer),
-             static_cast<std::streamsize>(size));
+    const auto s = static_cast<WriteSink *>(opaque);
+    std::ostream &o = *s->out;
+    const std::streamoff position =
+        s->base + static_cast<std::streamoff>(offset);
+    if (static_cast<std::streamoff>(o.tellp()) != position) {
+      o.seekp(position);
+    }
+    o.write(static_cast<const char *>(buffer),
+            static_cast<std::streamsize>(size));
     // A short write has to surface, or the archive is silently truncated.
-    return o->good() ? size : std::size_t{0};
+    return o.good() ? size : std::size_t{0};
   };
   state = mz_zip_writer_init(&archive, 0);
   if (!state) {

src/odr/internal/zip/zip_archive.hpp

@@ -24,6 +24,7 @@ class ZipArchive final : public abstract::Archive {
   [[nodiscard]] std::shared_ptr<abstract::Filesystem>
   as_filesystem() const override;
 
+  /// `out` has to be seekable — local headers are rewritten with the size.
   void save(std::ostream &out) const override;
 
   class Entry;

src/odr/internal/zip/zip_util.cpp

@@ -229,9 +229,12 @@ bool util::append_file(mz_zip_archive &archive, const std::string &path,
     return in->gcount();
   };
 
+  // Without the flag the size only lands in a trailing data descriptor, which
+  // LibreOffice rejects on a stored entry.
   return mz_zip_writer_add_read_buf_callback(
       &archive, path.c_str(), read_callback, &istream, size, &time,
-      comment.c_str(), comment.size(), level_and_flags, "", 0, "", 0);
+      comment.c_str(), comment.size(),
+      level_and_flags | MZ_ZIP_FLAG_WRITE_HEADER_SET_SIZE, "", 0, "", 0);
 }
 
 } // namespace odr::internal::zip

src/odr/internal/zip/zip_util.hpp

@@ -110,6 +110,8 @@ class Archive final : public std::enable_shared_from_this<Archive> {
 void open_from_file(mz_zip_archive &archive, const abstract::File &file,
                     std::istream &stream);
 
+/// `archive`'s write callback has to honour the offset it is given — local
+/// headers are rewritten with the entry size.
 bool append_file(mz_zip_archive &archive, const std::string &path,
                  std::istream &istream, std::size_t size,
                  const std::time_t &time, const std::string &comment,

test/src/internal/zip/zip_archive_test.cpp

@@ -9,6 +9,7 @@
 
 #include <gtest/gtest.h>
 
+#include <cstdint>
 #include <filesystem>
 #include <fstream>
 #include <memory>
@@ -51,7 +52,7 @@ TEST(ZipArchive, create_and_save) {
                   std::make_shared<MemoryFile>("hello world!"));
   zip.insert_directory(std::end(zip), RelPath("b"));
 
-  std::ofstream out("test.zip");
+  std::ofstream out("test.zip", std::ios::binary);
   zip.save(out);
 }
 
@@ -76,7 +77,7 @@ TEST(ZipArchive, create) {
     zip.insert_file(std::end(zip), RelPath("./notempty/four.txt"),
                     std::make_shared<MemoryFile>("1234"));
 
-    std::ofstream out(path);
+    std::ofstream out(path, std::ios::binary);
     zip.save(out);
   }
 
@@ -113,7 +114,7 @@ TEST(ZipArchive, create_order) {
                       std::make_shared<MemoryFile>(""));
     }
 
-    std::ofstream out(path);
+    std::ofstream out(path, std::ios::binary);
     zip.save(out);
   }
 
@@ -127,3 +128,63 @@ TEST(ZipArchive, create_order) {
     EXPECT_EQ(actual, entries);
   }
 }
+
+namespace {
+
+std::uint32_t read_le(const std::string &data, const std::size_t offset,
+                      const std::size_t size) {
+  std::uint32_t value = 0;
+  for (std::size_t i = 0; i < size; ++i) {
+    value |=
+        static_cast<std::uint32_t>(static_cast<std::uint8_t>(data[offset + i]))
+        << (8 * i);
+  }
+  return value;
+}
+
+} // namespace
+
+/// Walking the local headers by the sizes they carry only terminates if they
+/// carry them.
+TEST(ZipArchive, save_sizes_local_headers) {
+  const std::string path =
+      (std::filesystem::current_path() / "local_headers.zip").string();
+
+  {
+    ZipArchive zip;
+
+    zip.insert_file(std::end(zip), RelPath("stored"),
+                    std::make_shared<MemoryFile>("stored, like a mimetype"), 0);
+    zip.insert_file(std::end(zip), RelPath("deflated"),
+                    std::make_shared<MemoryFile>(std::string(1000, 'x')));
+    zip.insert_directory(std::end(zip), RelPath("dir"));
+
+    std::ofstream out(path, std::ios::binary);
+    zip.save(out);
+  }
+
+  std::ifstream in(path, std::ios::binary);
+  const std::string data{std::istreambuf_iterator<char>(in),
+                         std::istreambuf_iterator<char>()};
+
+  std::size_t offset = 0;
+  std::size_t entries = 0;
+  while (data.compare(offset, 4, "PK\x03\x04") == 0) {
+    const std::uint32_t flag = read_le(data, offset + 6, 2);
+    const std::uint32_t crc = read_le(data, offset + 14, 4);
+    const std::uint32_t compressed_size = read_le(data, offset + 18, 4);
+    const std::uint32_t path_size = read_le(data, offset + 26, 2);
+    const std::uint32_t extra_size = read_le(data, offset + 28, 2);
+
+    EXPECT_EQ(0, flag & 0x08);
+    if (data.compare(offset + 30, path_size, "dir/") != 0) {
+      EXPECT_NE(0, crc);
+      EXPECT_NE(0, compressed_size);
+    }
+
+    offset += 30 + path_size + extra_size + compressed_size;
+    ++entries;
+  }
+
+  EXPECT_EQ(3, entries);
+}