RFR: 8281962: Avoid unnecessary native calls in InflaterInputStream [v2]
Volker Simonis
simonis at openjdk.java.net
Fri Feb 18 08:49:55 UTC 2022
On Thu, 17 Feb 2022 20:58:54 GMT, Lance Andersen <lancea at openjdk.org> wrote:
> The change looks innocuous so it is probably OK. I would like to kick of our Mach5 runs to see if it shakes out any potential issues.
>
Thanks Lance! Much appreciated.
> From reading the 3rd party problem reports, it appears that the problem is with the 3rd party zlib implementations. Hopefully this change will not mask other issues
The problem arises from a difference in the specification of zlib's inflate function and Java's Input stream. Basically, both take a buffer and the *length* of that buffer as input and return the number of bytes (i.e. payload) written into that buffer (which can be smaller than *length*). However, zlib doesn't specify that bytes between the *returned length* and the the *buffer length* can't be modified while Java does.
Mark Adler's original zlib version never wrote more bytes into the buffer than it returned as *length* value and users of his implementation started to more or less rely on this implementation detail. But newer and improved versions of zlib might write more bytes into the output buffer than they return as *length* value (e.g. because they use vector instructions for writes). According to zlib's inflate specification this is fine as long as they don't overwrite the end of the buffer and as long as they return the correct *length* of inflated bytes.
In order to make this behavior more evident, Chromium's zlib version puts some extra padding bytes after the last inflated byte if there's enough space left in the buffer (and this happens even if zero bytes were inflated). This behavior becomes particularly harmful if Java's InflaterInputStream unnecessarily calls the native inflate() function just to find out that there's no data left to inflate. With Chromium's zlib this will still write the padding bytes to the beginning of the output buffer and potentially overwrite the inflated output from the last call to InflaterInputStream::read.
So to cut a long story short, there's no problem with Chromium's zlib implementation. It behaves correctly according to the zlib specification. This change (besides the performance improvements) helps using other zlib implementations which behave correctly but slightly different from the original zlib implementation into Java.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7492
More information about the core-libs-dev
mailing list