The premise, corrected
There is only one ZIP container implementation, and it is already shared:
zip/ (7,219 lines), imported by 18 packages. docx2html/zip looks like a
second one but is a 426-line domain adapter — zero ZIP header parsing, it calls
@mbtzip.read and wraps the result with DOCX path handling.
testutil/zip_fixture builds adversarial fixtures. Nothing else parses ZIP
structures.
The duplication is one level down: two independent DEFLATE codecs.
|
zip/deflate.mbt (1,044 ln) |
pdflite/flate (4,427 ln) |
| Module |
bobzhang/mbtexcel |
bobzhang/pdflite |
| Encoder |
one fixed-Huffman block, greedy matching, double-pass sizing |
miniz tdefl port — lazy matching, dynamic/static/stored selection, multi-block |
| Decoder |
stored/fixed/dynamic |
stored/fixed/dynamic |
| Kraft / oversubscription check |
yes |
no |
| All-zero distance tree (RFC 1951 §3.2.7) |
fixed in #422 |
fixed in #419 |
| Bounded output |
yes |
yes |
| Test emphasis |
corrupt corpus (1,010 ln), fuzz, preserve |
differential vs the C oracle |
They share no code.
Why this is worth doing: the fork has already cost us the same bug twice
pdf_flate_build_huffman and build_huffman are structurally the same
function — same flat lookup table, same reversed-code fill — with
complementary gaps. Neither is a superset of the other.
1. All-zero distance tree. A dynamic block coding no matches must still
declare a distance tree with every length zero. Both decoders rejected it as
malformed. Fixed in pdflite in #419; the identical defect was still live in
zip and is fixed in #422. It had to be found and fixed twice.
2. Oversubscribed Huffman table. The reverse gap. Feeding one malformed
stream (literal tree with three codes of length 1, Kraft = 1.5) to three
decoders:
| decoder |
verdict |
| zlib |
rejects — invalid literal/lengths set |
zip |
rejects — oversubscribed huffman table |
pdflite |
accepts, returns 0 bytes |
pdflite silently returns wrong output where it should raise — the wrong
failure mode for parsing untrusted PDFs. zip/deflate.mbt:280-291 has the
check and explains why; pdflite/flate/pdf_flate_huffman.mbt:149 omits it.
(Not memory-unsafe: reversed stays under 1 << length, so the flat table is
never indexed out of bounds. Colliding codes just overwrite each other.)
The other payoff: smaller output
zip's encoder emits a single fixed-Huffman block and never a dynamic one. On
the parts of excelize/test/Book1.xlsx, fixed-Huffman costs 10.6% over
dynamic (67,052 raw → 19,427 fixed vs 17,361 dynamic), and that understates it
because the measurement's fixed-Huffman baseline still used lazy matching where
ours is greedy.
Consolidating on the miniz port means every .xlsx and .docx we write gets
roughly a tenth smaller, and the writer gains dynamic-Huffman selection,
multi-block emission and lazy matching for free.
It would also have closed the gap that hid the #422 bug: zip's round-trip and
fuzz properties compress with our encoder and decompress with our decoder, and
since that encoder never emits a dynamic block, the dynamic decode path was
unreachable from them. A shared codec whose encoder does emit dynamic blocks
exercises its own decoder's dynamic path.
Shape
The codecs live in different modules and neither should depend on the other — a
spreadsheet library should not pull in a PDF library, or vice versa. So this
wants a small standalone flate module that both import, published so other
libraries can use it directly rather than reaching into mbtexcel or pdflite.
Contents: the miniz-identical tdefl encoder, one decoder carrying the union of
both hardenings (Kraft check + all-zero distance tree + bounded output +
cancellation), CRC-32, and the merged test corpora — the corrupt/fuzz suite from
zip plus the differential-against-C suite from pdflite.
Steps
Blocked-on / sequencing
Not urgent relative to the mutation gap (N0b4 #234). #422 fixes the live
correctness bug, so this is consolidation and output-size work rather than a
defect fix — with the exception of the pdflite oversubscription gap above,
which could be fixed on its own first if we want the correctness part sooner.
🤖 Generated with Claude Code
The premise, corrected
There is only one ZIP container implementation, and it is already shared:
zip/(7,219 lines), imported by 18 packages.docx2html/ziplooks like asecond one but is a 426-line domain adapter — zero ZIP header parsing, it calls
@mbtzip.readand wraps the result with DOCX path handling.testutil/zip_fixturebuilds adversarial fixtures. Nothing else parses ZIPstructures.
The duplication is one level down: two independent DEFLATE codecs.
zip/deflate.mbt(1,044 ln)pdflite/flate(4,427 ln)bobzhang/mbtexcelbobzhang/pdflitetdeflport — lazy matching, dynamic/static/stored selection, multi-blockThey share no code.
Why this is worth doing: the fork has already cost us the same bug twice
pdf_flate_build_huffmanandbuild_huffmanare structurally the samefunction — same flat lookup table, same reversed-code fill — with
complementary gaps. Neither is a superset of the other.
1. All-zero distance tree. A dynamic block coding no matches must still
declare a distance tree with every length zero. Both decoders rejected it as
malformed. Fixed in
pdflitein #419; the identical defect was still live inzipand is fixed in #422. It had to be found and fixed twice.2. Oversubscribed Huffman table. The reverse gap. Feeding one malformed
stream (literal tree with three codes of length 1, Kraft = 1.5) to three
decoders:
invalid literal/lengths setzipoversubscribed huffman tablepdflitepdflitesilently returns wrong output where it should raise — the wrongfailure mode for parsing untrusted PDFs.
zip/deflate.mbt:280-291has thecheck and explains why;
pdflite/flate/pdf_flate_huffman.mbt:149omits it.(Not memory-unsafe:
reversedstays under1 << length, so the flat table isnever indexed out of bounds. Colliding codes just overwrite each other.)
The other payoff: smaller output
zip's encoder emits a single fixed-Huffman block and never a dynamic one. Onthe parts of
excelize/test/Book1.xlsx, fixed-Huffman costs 10.6% overdynamic (67,052 raw → 19,427 fixed vs 17,361 dynamic), and that understates it
because the measurement's fixed-Huffman baseline still used lazy matching where
ours is greedy.
Consolidating on the miniz port means every
.xlsxand.docxwe write getsroughly a tenth smaller, and the writer gains dynamic-Huffman selection,
multi-block emission and lazy matching for free.
It would also have closed the gap that hid the #422 bug:
zip's round-trip andfuzz properties compress with our encoder and decompress with our decoder, and
since that encoder never emits a dynamic block, the dynamic decode path was
unreachable from them. A shared codec whose encoder does emit dynamic blocks
exercises its own decoder's dynamic path.
Shape
The codecs live in different modules and neither should depend on the other — a
spreadsheet library should not pull in a PDF library, or vice versa. So this
wants a small standalone
flatemodule that both import, published so otherlibraries can use it directly rather than reaching into
mbtexcelorpdflite.Contents: the miniz-identical
tdeflencoder, one decoder carrying the union ofboth hardenings (Kraft check + all-zero distance tree + bounded output +
cancellation), CRC-32, and the merged test corpora — the corrupt/fuzz suite from
zipplus the differential-against-C suite frompdflite.Steps
bobzhang/flatewith the miniztdeflencoder and the merged decoderzip's Kraft check into the shared decoder; add a regression vector for the oversubscribed stream abovezip/deflate.mbtat it, delete the fixed-Huffman-only encoderpdflite/flateat it, keep the PDF-specific zlib-wrapper handlingBlocked-on / sequencing
Not urgent relative to the mutation gap (N0b4 #234). #422 fixes the live
correctness bug, so this is consolidation and output-size work rather than a
defect fix — with the exception of the
pdfliteoversubscription gap above,which could be fixed on its own first if we want the correctness part sooner.
🤖 Generated with Claude Code