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

[Lance Andersen]

On Fri, 11 Oct 2024 08:11:37 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

> Please review this cleanup PR which removes overrides of `InflaterInputStream.fill` in `ZipFileInflaterInputStream` and `ZipFileSystem::getInputStream`. Associated boolean fields used to track `eof` are also removed.
> 
> These overrides exist to provide zlib with an extra dummy byte at the end of raw compressed streams (no wrapping):
> 
> 
> // Override fill() method to provide an extra "dummy" byte
> // at the end of the input stream. This is required when
> // using the "nowrap" Inflater option.
> protected void fill() throws IOException {
> 
> However, zlib has not required such an extra dummy byte since 2003. 
> 
> 
> Changes in 1.2.0 (9 March 2003)
> - New and improved inflate code
>     - Raw inflate no longer needs an extra dummy byte at end
> ``` 
> 
> The code in these overrides is effectively dead and removing it cleans up our code and reclaims weirdness dollars for our budget.
> 
> Risk: I cannot imagine anyone is building OpenJDK with a 21 year old ZLIB.  Please advise if this is the case or if any zlib fork in use still has this limitation.
> 
> Testing: ZIP and ZIPFS tests run green locally. GHA results pending. No tests are added or modified, this is a cleanup-only PR. Manually verified that the code is dead by injecting AssertionErrors.

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

 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, 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.

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

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

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

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