# [Patch] paszlib: inflate() rejects a nil next_in when avail_in=0, unlike deflate() `zinflate.pas:344` starts `inflate()` with ```pascal function inflate(var z : z_stream; f : integer) : integer; begin if (z.state=nil) or (z.next_in=nil) then begin inflate := Z_STREAM_ERROR; exit; end; ``` so a nil `next_in` is refused whether or not there is anything to read. zlib refuses it only when there is: ```c /* current zlib */ if (inflateStateCheck(strm) || strm->next_out == Z_NULL || (strm->next_in == Z_NULL && strm->avail_in != 0)) return Z_STREAM_ERROR; ``` The unconditional form is what zlib 1.1.4's `inflate.c` did. zlib 1.2.0 (March 2003) changed it to the conditional form and added the `next_out` check. zlib 1.1.4's `deflate.c` already had the conditional form, so the asymmetry comes from zlib 1.1.x itself; paszlib inherited it and still has it, because `zdeflate.pas:875-877` reads ```pascal if (strm.next_out=nil) or ((strm.next_in=nil) and (strm.avail_in<>0)) or ``` while `zinflate.pas` never checks `next_out` and checks `next_in` too strictly. ## Why it matters A wrapper that borrows the caller's buffer has to drop the pointer once the input is consumed, because the buffer does not outlive the call: ```pascal z.next_in := @caller_buffer; z.avail_in := len; repeat inflate(z, Z_NO_FLUSH); ... until z.avail_in = 0; z.next_in := nil; // the caller's buffer must not be referenced any more z.avail_in := 0; ... inflate(z, Z_FINISH); ``` Against zlib the last call returns whatever the stream state calls for - `Z_STREAM_END` for a stream that is already complete, `Z_BUF_ERROR` for one that still needs input. Against paszlib it returns `Z_STREAM_ERROR` in both cases, purely because `next_in` is nil. This is how mORMot 2's `TSynZipDecompressor` is written, and it is why decompression fails on every target where mORMot selects paszlib over a static zlib. ## Reproducer `inflatenil.pas` (attached) checks three calls. Each expected value is what zlib returns for the same call: | call | zlib | paszlib | |---|---|---| | `deflate(Z_FINISH)`, input consumed, `next_in=nil` | `Z_STREAM_END` | `Z_STREAM_END` | | `inflate(Z_FINISH)`, fresh stream, no input | `Z_BUF_ERROR` | `Z_STREAM_ERROR` | | `inflate(Z_FINISH)`, input consumed, `next_in=nil` | `Z_STREAM_END` | `Z_STREAM_ERROR` | The deflate line is a control, not an oracle: it shows the intended contract already holds in the sibling implementation of the same package. ``` fpc inflatenil.pas && ./inflatenil ``` ``` FAIL inflate(Z_FINISH), fresh stream, no input: got -2, expected -5 FAIL inflate(Z_FINISH), next_in=nil avail_in=0: got -2, expected 1 2 failure(s), Z_STREAM_ERROR is -2 ``` The decompressed bytes and `total_out` are verified before the decisive call, so a no-op implementation cannot pass. Measured on FPC 3.3.1 (trunk 416b51be87) aarch64-win64 and on FPC 3.2.2 aarch64-linux, with identical output. The failing entry guard is unconditional source, with no target-dependent branch. ## Suggested fix Give `inflate()` the same guard `deflate()` and zlib already use: ```diff --- a/packages/paszlib/src/zinflate.pas +++ b/packages/paszlib/src/zinflate.pas @@ - if (z.state=nil) or (z.next_in=nil) then + if (z.state=nil) or + (z.next_out=nil) or + ((z.next_in=nil) and (z.avail_in<>0)) then begin inflate := Z_STREAM_ERROR; exit; end; ``` The `next_out` half is not cosmetic: today a nil `next_out` with a positive `avail_out` is not rejected and is eventually written through. Verified: with that hunk applied the reproducer exits 0, and all three calls return the zlib values in the table above. ## Regression test `tpaszlib1.pp` (attached) is the same three checks in testsuite form. It exits 1 on the current sources and 0 with the hunk above. It is pure Pascal and needs no external shared library, unlike `tests/test/packages/zlib/tzlib1.pp`, which is `%norun`. It compiles in the default mode without a warning, note or hint, and carries no `%` directives; add a `%skiptarget` if paszlib is not expected to build on the 16-bit targets that `tzlib1.pp` skips. `tests/test/packages/paszlib/` is a new directory, and `TESTPACKAGESDIRS` in `tests/Makefile.fpc:175` is a hardcoded list that does not include `paszlib`, so the test also needs ```diff --- a/tests/Makefile.fpc +++ b/tests/Makefile.fpc @@ -TESTPACKAGESDIRS=bzip2 cocoaint fcl-base fcl-db fcl-image fcl-registry fcl-xml hash rtl-objpas univint webtbs win-base zlib +TESTPACKAGESDIRS=bzip2 cocoaint fcl-base fcl-db fcl-image fcl-registry fcl-xml hash paszlib rtl-objpas univint webtbs win-base zlib ``` `tests/Makefile` is what a normal test run consumes and still carries the old list, so it has to be regenerated: from `tests/`, `make fpc_makefile` (i.e. `fpcmake -w -T<target> Makefile.fpc`), and the result committed. No `fpmake.pp` or directory-level Makefile is needed. Without that step the file is never picked up. If you would rather keep the test next to the existing package tests in `packages/paszlib/tests/`, that directory is not wired into `TESTPACKAGESDIRECTDIRS` either. [fpc-paszlib-fix.patch](/uploads/be384df9b0b7e8cf384e87eadd699053/fpc-paszlib-fix.patch) [fpc-paszlib-repro.zip](/uploads/04ced731bef17114cef5bfa0946ebf23/fpc-paszlib-repro.zip)