RFR: 8341944: The zlib library no longer requires dummy byte for raw inflate

[Eirik Bjørsnøs]

On Fri, 11 Oct 2024 14:05:40 GMT, Lance Andersen <lancea at openjdk.org> wrote:

> I think we need to be a bit cautious with the proposed change while I understand the desire to remove potentially unneeded code.

I understand the need to be cautious in this area. But I would also like to avoid cargo-culting this for another 21 years :-)  

> I have not yet found a specific issue for the for the original need for the overload of fill, but have not looked to hard either

If we trust the zlib change log, the original need retired with 1.2.0. Any of the performance-motivated forks like chromium-zlib or cloudflare-zlib should have been forked long after this. Perhaps @simonis can confirm. 

>, and the constructor in Inflater.java also makes reference to gzip for which we do not have as much test coverage as we do for Zip.

It's interesting that `GZIPInputStream` does _not_ override `InflaterInputStream.fill`, but _does_ use `nowrap`.

The extra dummy byte note in the Inflater constructor is now misleading and should probably go too. Would require a CSR. 

The `@param nowrap` note is perhaps also misleading in that it is not actually specific to GZIP. It just tells zlib to expect raw deflated data, without any wrapping, GZIP or zlib. I guess since the only wrapping supported by the JDK is GZIP, the note is maybe okay. But if we do a CSR, I think we should relax this note to not reference GZIP.  

> So we might want to hold off on this PR for JDK 24 while we try to do some more archaeological digging.

That is fair.

> As far as which zlib with the openJDK, currently the openjdk project bundles zlib with the windows builds and relies on the zlib bundled with the various OS's
> 
> I would be surprised to find any vendor using a zlib below zlib 1.2.13 or 1.3.1

Yes, that would indeed be awkward.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/21467#issuecomment-2407549193