You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Every zip we read or write goes through miniz
(miniz/3.0.2, conanfile.py). It works, but it is a C library with a 1990s
API, and the whole of src/odr/internal/zip/ (~700 lines) exists to paper over
that. Worth evaluating whether a more modern library — ideally a C++ one — buys
us a smaller wrapper, better errors, and the zip features we currently do not
support.
What miniz costs us today
Everything is a bool.mz_zip_writer_init, append_file, finalize_archive, writer_end each return true/false, so ZipArchive::save (src/odr/internal/zip/zip_archive.cpp:82) is a ladder of if (!state) throw MinizSaveError(archive), where the error has to be fished
back out of the archive struct afterwards. That leaked far enough that odr/exceptions.hpp:63 — a public header — documents internal::zip::MinizSaveError, and zip_exceptions.hpp includes <miniz.h> to hold an mz_zip_error.
No thread safety, so we serialise everything.util::Archive carries a mutable std::mutex and every single operation — is_file, path, method, size, opening a stream, and each 4 KiB underflow — takes it
(zip_util.cpp:36-141). Two entries of the same document can never be read
in parallel, which is exactly the thing we would want when rendering gets
parallelised.
Streaming reads are hand-rolled and lossy on error.ReaderBuffer wraps mz_zip_reader_extract_iter_* in a std::streambuf, and because miniz
reports a failed inflate as a short read, zip_util.cpp:44 has to treat result == 0 as EOF — a corrupt entry silently truncates instead of
throwing.
Writing needs a seekable sink and a magic flag.save has to keep a WriteSink with a base offset because miniz addresses output by absolute
offset and rewrites local headers; ZipArchive::save's contract is therefore
"out has to be seekable" (zip_archive.hpp:29). And fix(zip): write an entry's size into its local header #755 was exactly the
kind of bug this invites: without MZ_ZIP_FLAG_WRITE_HEADER_SET_SIZE the
size only lands in a trailing data descriptor and LibreOffice rejects the
file.
Round-tripping loses compression settings.ZipArchive's ctor can only
recover "stored" vs "deflate", and guesses level 6 for the latter
(zip_archive.cpp:48), because that is all mz_zip_archive_file_stat gives
back.
No zip64, no encrypted entries. Encrypted entries are detected and
rejected (zip_util.cpp:100). Fine today — ODF/OOXML encryption is handled
above us in odf_crypto.cpp / ooxml_file.cpp — but it means WinZip-AES
archives opened as plain zips are simply unreadable.
We test the dependency itself.test/src/internal/zip/miniz_test.cpp
drives raw mz_zip_* calls, which is a fair sign of how little the library
gives us on its own.
Also relevant: #312 (SIGSEGV in zip_util.cpp, still open, no repro) and #762
(memory blowup) both live in this neighbourhood.
The blast radius is small
miniz is used only inside src/odr/internal/zip/ — nothing else in the
tree calls mz_* or uses its raw deflate. Outside consumers touch only ZipFile / ZipArchive / abstract::Archive
(open_strategy.cpp, odf_crypto.cpp, odf_document.cpp, ooxml_file.cpp, ooxml_text_document.cpp). So a swap is: the 8 files under src/odr/internal/zip/, one line in conanfile.py, two in CMakeLists.txt,
and miniz_test.cpp disappears. zip_archive_test.cpp is API-level and should
pass unchanged.
What a replacement must actually do
Non-negotiable, because ODF/OOXML break otherwise:
Read from a custom source. Our input is an abstract::File (memory or
disk), consumed through a std::istream and a read callback
(zip_util.cpp:203). No "give me a path" -only API.
Builds everywhere we ship: linux/macos/windows, android, ios, and wasm/emscripten — the last one has killed dependencies for us before.
Conan Center recipe strongly preferred.
Nice to have: concurrent entry reads, zip64, WinZip-AES, exceptions or expected-style errors instead of bool, RAII instead of mz_zip_end in a destructor, and a non-seekable write path.
Candidates
Available on Conan Center today:
libzip/1.11.4 — C, mature, actively maintained, zip64 + AES, zip_source covers requirement 1 cleanly, good error reporting. Still C, so
we would keep a wrapper — but a much thinner one.
libzippp/7.1-1.10.1 — C++ wrapper over libzip. Closest thing to "a
modern C++ zip library" that is packaged; needs a look at whether its API
reaches libzip's custom sources and streaming, or only the convenient subset.
minizip-ng/4.2.1 — C, actively maintained, zip64, AES, zstd/lzma,
callback IO. Feature-rich; API is still very C.
libarchive/3.8.7 — C, broadest format support (would also cover
reading other containers). Heavier, and its streaming model is a poor fit for
the random-access reads we do into a zip.
Not packaged / probably out: bit7z (no Conan Center recipe, needs the 7-Zip
shared library), kuba--/zip (a thin wrapper over miniz — no gain),
miniz-cpp and ZipLib (unmaintained).
Proposal
Prototype against libzip and libzippp behind the existing abstract::Archive / ZipFile interfaces — the interfaces are already the
right seam, so this is a drop-in experiment, not a refactor.
Check requirement 5 first (wasm + android + ios builds), it is the cheapest
way to eliminate a candidate.
Compare on: wrapper size, whether the mutex can go away, error quality, and
round-trip fidelity against LibreOffice (soffice --convert-to on a saved
package).
Keep miniz if nothing clears the bar — "it works and it is small" is a
legitimate outcome. But the current wrapper's list of workarounds suggests
there is something better.
Every zip we read or write goes through miniz
(
miniz/3.0.2,conanfile.py). It works, but it is a C library with a 1990sAPI, and the whole of
src/odr/internal/zip/(~700 lines) exists to paper overthat. Worth evaluating whether a more modern library — ideally a C++ one — buys
us a smaller wrapper, better errors, and the zip features we currently do not
support.
What miniz costs us today
bool.mz_zip_writer_init,append_file,finalize_archive,writer_endeach return true/false, soZipArchive::save(src/odr/internal/zip/zip_archive.cpp:82) is a ladder ofif (!state) throw MinizSaveError(archive), where the error has to be fishedback out of the archive struct afterwards. That leaked far enough that
odr/exceptions.hpp:63— a public header — documentsinternal::zip::MinizSaveError, andzip_exceptions.hppincludes<miniz.h>to hold anmz_zip_error.util::Archivecarries amutable std::mutexand every single operation —is_file,path,method,size, opening a stream, and each 4 KiBunderflow— takes it(
zip_util.cpp:36-141). Two entries of the same document can never be readin parallel, which is exactly the thing we would want when rendering gets
parallelised.
ReaderBufferwrapsmz_zip_reader_extract_iter_*in astd::streambuf, and because minizreports a failed inflate as a short read,
zip_util.cpp:44has to treatresult == 0as EOF — a corrupt entry silently truncates instead ofthrowing.
savehas to keep aWriteSinkwith a base offset because miniz addresses output by absoluteoffset and rewrites local headers;
ZipArchive::save's contract is therefore"
outhas to be seekable" (zip_archive.hpp:29). And fix(zip): write an entry's size into its local header #755 was exactly thekind of bug this invites: without
MZ_ZIP_FLAG_WRITE_HEADER_SET_SIZEthesize only lands in a trailing data descriptor and LibreOffice rejects the
file.
ZipArchive's ctor can onlyrecover "stored" vs "deflate", and guesses level 6 for the latter
(
zip_archive.cpp:48), because that is allmz_zip_archive_file_statgivesback.
rejected (
zip_util.cpp:100). Fine today — ODF/OOXML encryption is handledabove us in
odf_crypto.cpp/ooxml_file.cpp— but it means WinZip-AESarchives opened as plain zips are simply unreadable.
test/src/internal/zip/miniz_test.cppdrives raw
mz_zip_*calls, which is a fair sign of how little the librarygives us on its own.
Also relevant: #312 (SIGSEGV in
zip_util.cpp, still open, no repro) and #762(memory blowup) both live in this neighbourhood.
The blast radius is small
miniz is used only inside
src/odr/internal/zip/— nothing else in thetree calls
mz_*or uses its raw deflate. Outside consumers touch onlyZipFile/ZipArchive/abstract::Archive(
open_strategy.cpp,odf_crypto.cpp,odf_document.cpp,ooxml_file.cpp,ooxml_text_document.cpp). So a swap is: the 8 files undersrc/odr/internal/zip/, one line inconanfile.py, two inCMakeLists.txt,and
miniz_test.cppdisappears.zip_archive_test.cppis API-level and shouldpass unchanged.
What a replacement must actually do
Non-negotiable, because ODF/OOXML break otherwise:
abstract::File(memory ordisk), consumed through a
std::istreamand a read callback(
zip_util.cpp:203). No "give me a path" -only API.see A 1.4 MB ods grows past 1.2 GB in Html::translate and the OS kills the app #762.
MZ_ZIP_FLAG_DO_NOT_SORT_CENTRAL_DIRECTORY): ODF requiresmimetypefirstand stored.
local header (the fix(zip): write an entry's size into its local header #755 fix).
wasm/emscripten — the last one has killed dependencies for us before.
Conan Center recipe strongly preferred.
Nice to have: concurrent entry reads, zip64, WinZip-AES, exceptions or
expected-style errors instead ofbool, RAII instead ofmz_zip_endin a destructor, and a non-seekable write path.Candidates
Available on Conan Center today:
libzip/1.11.4— C, mature, actively maintained, zip64 + AES,zip_sourcecovers requirement 1 cleanly, good error reporting. Still C, sowe would keep a wrapper — but a much thinner one.
libzippp/7.1-1.10.1— C++ wrapper over libzip. Closest thing to "amodern C++ zip library" that is packaged; needs a look at whether its API
reaches libzip's custom sources and streaming, or only the convenient subset.
minizip-ng/4.2.1— C, actively maintained, zip64, AES, zstd/lzma,callback IO. Feature-rich; API is still very C.
libarchive/3.8.7— C, broadest format support (would also coverreading other containers). Heavier, and its streaming model is a poor fit for
the random-access reads we do into a zip.
Not packaged / probably out:
bit7z(no Conan Center recipe, needs the 7-Zipshared library),
kuba--/zip(a thin wrapper over miniz — no gain),miniz-cpp and ZipLib (unmaintained).
Proposal
libzipandlibzipppbehind the existingabstract::Archive/ZipFileinterfaces — the interfaces are already theright seam, so this is a drop-in experiment, not a refactor.
way to eliminate a candidate.
round-trip fidelity against LibreOffice (
soffice --convert-toon a savedpackage).
legitimate outcome. But the current wrapper's list of workarounds suggests
there is something better.