On Thu, 17 Feb 2022 20:58:54 GMT, Lance Andersen <[email protected]> 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

Reply via email to