diff --git a/CHANGELOG.md b/CHANGELOG.md index 01597886b..c116171b0 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/odr/internal/zip/zip_archive.cpp b/src/odr/internal/zip/zip_archive.cpp index fbdf6cc2d..2e424d94c 100644 --- a/src/odr/internal/zip/zip_archive.cpp +++ b/src/odr/internal/zip/zip_archive.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -72,20 +73,38 @@ std::shared_ptr 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(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(opaque); - o->write(static_cast(buffer), - static_cast(size)); + const auto s = static_cast(opaque); + std::ostream &o = *s->out; + const std::streamoff position = + s->base + static_cast(offset); + if (static_cast(o.tellp()) != position) { + o.seekp(position); + } + o.write(static_cast(buffer), + static_cast(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) { diff --git a/src/odr/internal/zip/zip_archive.hpp b/src/odr/internal/zip/zip_archive.hpp index 6fee803b6..2b7b5db0e 100644 --- a/src/odr/internal/zip/zip_archive.hpp +++ b/src/odr/internal/zip/zip_archive.hpp @@ -24,6 +24,7 @@ class ZipArchive final : public abstract::Archive { [[nodiscard]] std::shared_ptr 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; diff --git a/src/odr/internal/zip/zip_util.cpp b/src/odr/internal/zip/zip_util.cpp index 428d7bb85..2fa6d2c15 100644 --- a/src/odr/internal/zip/zip_util.cpp +++ b/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 diff --git a/src/odr/internal/zip/zip_util.hpp b/src/odr/internal/zip/zip_util.hpp index 09f00ad4f..329d13e54 100644 --- a/src/odr/internal/zip/zip_util.hpp +++ b/src/odr/internal/zip/zip_util.hpp @@ -110,6 +110,8 @@ class Archive final : public std::enable_shared_from_this { 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, diff --git a/test/src/internal/zip/zip_archive_test.cpp b/test/src/internal/zip/zip_archive_test.cpp index 5305b033c..9b0187df8 100644 --- a/test/src/internal/zip/zip_archive_test.cpp +++ b/test/src/internal/zip/zip_archive_test.cpp @@ -9,6 +9,7 @@ #include +#include #include #include #include @@ -51,7 +52,7 @@ TEST(ZipArchive, create_and_save) { std::make_shared("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("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("")); } - 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(static_cast(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("stored, like a mimetype"), 0); + zip.insert_file(std::end(zip), RelPath("deflated"), + std::make_shared(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(in), + std::istreambuf_iterator()}; + + 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); +}