Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v2]
On Wed, 30 Mar 2022 10:26:56 GMT, Lance Andersen wrote:
>>> Hello Volker, An additional thing that we might have to consider here is
>>> whether adding this javadoc change to `InflaterInputStream` is ever going
>>> to "show up" to end user applications. What I mean is, I think in many
>>> cases the end user applications won't even know they are dealing with an
>>> `InflaterInputStream`. For example, the following code:
>>>
>>> ```
>>> ZipFile zf = ...
>>> ZipEntry ze = zf.getEntry("some-file");
>>> InputStream is = zf.getInputStream(ze);
>>> ```
>>>
>>> As we see above, none of these APIs talk about `InflaterInputStream` (the
>>> return type of `ZipFile.getInpustream(...)` is an `InputStream`). So end
>>> users won't be able to view this spec change. Perhaps we should also add
>>> some note in the `ZipFile.getInpustream()` API to make a mention of
>>> this potential difference in behaviour of the returned stream?
>>
>> You are right with your observation and I'll be happy to add a corresponding
>> comment if @LanceAndersen and @AlanBateman agree. Please let me know what
>> you think?
>
>> > Hello Volker, An additional thing that we might have to consider here is
>> > whether adding this javadoc change to `InflaterInputStream` is ever going
>> > to "show up" to end user applications. What I mean is, I think in many
>> > cases the end user applications won't even know they are dealing with an
>> > `InflaterInputStream`. For example, the following code:
>> > ```
>> > ZipFile zf = ...
>> > ZipEntry ze = zf.getEntry("some-file");
>> > InputStream is = zf.getInputStream(ze);
>> > ```
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> > As we see above, none of these APIs talk about `InflaterInputStream` (the
>> > return type of `ZipFile.getInpustream(...)` is an `InputStream`). So end
>> > users won't be able to view this spec change. Perhaps we should also add
>> > some note in the `ZipFile.getInpustream()` API to make a mention of
>> > this potential difference in behaviour of the returned stream?
>>
>> You are right with your observation and I'll be happy to add a corresponding
>> comment if @LanceAndersen and @AlanBateman agree. Please let me know what
>> you think?
>
> Hi Volker,
>
> I believe Jai raises a valid point given these javadocs probably have had
> limited updates if any since the API was originally added.We should look
> at ZipInputStream and GZipInputStream as well if we decide to update the
> ZipFile::getInputStream(where we could borrow some wording from the
> ZipInputStream class description as a start to some word smithing).
>
> As Roger points out we will need a release note for this change as well.
> @LanceAndersen, @AlanBateman can you please comment on the
> [CSR](https://bugs.openjdk.java.net/browse/JDK-8283758) for this issue. We
> now circled back to the initial proposal in the
> [CSR](https://bugs.openjdk.java.net/browse/JDK-8283758) but @jddarcy would
> like to hear your opinion.
Have not forgotten about this. I think we are not quite there on the changes
to` Inputstream` but have not had a chance to give it some more thought.
Apologies and thank you for your patience
-
PR: https://git.openjdk.java.net/jdk/pull/7986
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v2]
On Wed, 30 Mar 2022 10:26:56 GMT, Lance Andersen wrote:
>>> Hello Volker, An additional thing that we might have to consider here is
>>> whether adding this javadoc change to `InflaterInputStream` is ever going
>>> to "show up" to end user applications. What I mean is, I think in many
>>> cases the end user applications won't even know they are dealing with an
>>> `InflaterInputStream`. For example, the following code:
>>>
>>> ```
>>> ZipFile zf = ...
>>> ZipEntry ze = zf.getEntry("some-file");
>>> InputStream is = zf.getInputStream(ze);
>>> ```
>>>
>>> As we see above, none of these APIs talk about `InflaterInputStream` (the
>>> return type of `ZipFile.getInpustream(...)` is an `InputStream`). So end
>>> users won't be able to view this spec change. Perhaps we should also add
>>> some note in the `ZipFile.getInpustream()` API to make a mention of
>>> this potential difference in behaviour of the returned stream?
>>
>> You are right with your observation and I'll be happy to add a corresponding
>> comment if @LanceAndersen and @AlanBateman agree. Please let me know what
>> you think?
>
>> > Hello Volker, An additional thing that we might have to consider here is
>> > whether adding this javadoc change to `InflaterInputStream` is ever going
>> > to "show up" to end user applications. What I mean is, I think in many
>> > cases the end user applications won't even know they are dealing with an
>> > `InflaterInputStream`. For example, the following code:
>> > ```
>> > ZipFile zf = ...
>> > ZipEntry ze = zf.getEntry("some-file");
>> > InputStream is = zf.getInputStream(ze);
>> > ```
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> > As we see above, none of these APIs talk about `InflaterInputStream` (the
>> > return type of `ZipFile.getInpustream(...)` is an `InputStream`). So end
>> > users won't be able to view this spec change. Perhaps we should also add
>> > some note in the `ZipFile.getInpustream()` API to make a mention of
>> > this potential difference in behaviour of the returned stream?
>>
>> You are right with your observation and I'll be happy to add a corresponding
>> comment if @LanceAndersen and @AlanBateman agree. Please let me know what
>> you think?
>
> Hi Volker,
>
> I believe Jai raises a valid point given these javadocs probably have had
> limited updates if any since the API was originally added.We should look
> at ZipInputStream and GZipInputStream as well if we decide to update the
> ZipFile::getInputStream(where we could borrow some wording from the
> ZipInputStream class description as a start to some word smithing).
>
> As Roger points out we will need a release note for this change as well.
@LanceAndersen, @AlanBateman can you please comment on the
[CSR](https://bugs.openjdk.java.net/browse/JDK-8283758) for this issue. We now
circled back to the initial proposal in the
[CSR](https://bugs.openjdk.java.net/browse/JDK-8283758) but @jddarcy would
like to hear your opinion.
-
PR: https://git.openjdk.java.net/jdk/pull/7986
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v2]
On Tue, 29 Mar 2022 12:45:51 GMT, Volker Simonis wrote:
> > Hello Volker, An additional thing that we might have to consider here is
> > whether adding this javadoc change to `InflaterInputStream` is ever going
> > to "show up" to end user applications. What I mean is, I think in many
> > cases the end user applications won't even know they are dealing with an
> > `InflaterInputStream`. For example, the following code:
> > ```
> > ZipFile zf = ...
> > ZipEntry ze = zf.getEntry("some-file");
> > InputStream is = zf.getInputStream(ze);
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > As we see above, none of these APIs talk about `InflaterInputStream` (the
> > return type of `ZipFile.getInpustream(...)` is an `InputStream`). So end
> > users won't be able to view this spec change. Perhaps we should also add
> > some note in the `ZipFile.getInpustream()` API to make a mention of
> > this potential difference in behaviour of the returned stream?
>
> You are right with your observation and I'll be happy to add a corresponding
> comment if @LanceAndersen and @AlanBateman agree. Please let me know what you
> think?
Hi Volker,
I believe Jai raises a valid point given these javadocs probably have had
limited updates if any since the API was originally added.We should look at
ZipInputStream and GZipInputStream as well if we decide to update the
ZipFile::getInputStream(where we could borrow some wording from the
ZipInputStream class description as a start to some word smithing).
As Roger points out we will need a release note for this change as well.
-
PR: https://git.openjdk.java.net/jdk/pull/7986
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v2]
On Tue, 29 Mar 2022 01:58:05 GMT, Jaikiran Pai wrote:
> Hello Volker, An additional thing that we might have to consider here is
> whether adding this javadoc change to `InflaterInputStream` is ever going to
> "show up" to end user applications. What I mean is, I think in many cases the
> end user applications won't even know they are dealing with an
> `InflaterInputStream`. For example, the following code:
>
> ```
> ZipFile zf = ...
> ZipEntry ze = zf.getEntry("some-file");
> InputStream is = zf.getInputStream(ze);
> ```
>
> As we see above, none of these APIs talk about `InflaterInputStream` (the
> return type of `ZipFile.getInpustream(...)` is an `InputStream`). So end
> users won't be able to view this spec change. Perhaps we should also add some
> note in the `ZipFile.getInpustream()` API to make a mention of this
> potential difference in behaviour of the returned stream?
You are right with your observation and I'll be happy to add a corresponding
comment if @LanceAndersen and @AlanBateman agree. Please let me know what you
think?
-
PR: https://git.openjdk.java.net/jdk/pull/7986
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v2]
On Mon, 28 Mar 2022 18:23:46 GMT, Lance Andersen wrote: >> I think this require a bit of word smithing. If the return value is 'n' then >> it should make it clear that the values in elements b[off + n] to b[off + >> len - 1] are undefined, their values may or may have been changed by the >> inflate/read operation. For completeness, the expectation for when inflate >> fails should be specified too, the contents of b[off] to b[off + len - 1] >> will be undefined. > > Hi Volker, > > I believe you are heading in the right direction. I agree with Alan and Jai > that we could use a bit more clarification Thanks for your comments. I've updated the API-doc according to your recommendations. Please feel free to propose new/better wording :) I'll wait with updating the CSR until we reach final agreement in this PR. - PR: https://git.openjdk.java.net/jdk/pull/7986
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v2]
On Mon, 28 Mar 2022 10:55:30 GMT, Volker Simonis wrote: >> Add an API note to `InflaterInputStream::read(byte[] b, int off, int len)` >> to highlight that it might write more bytes than the returned number of >> inflated bytes into the buffer `b`. >> >> The superclass `java.io.InputStream` specifies that `read(byte[] b, int off, >> int len)` will leave the content beyond the last read byte in the read >> buffer `b` unaffected. However, the overridden `read` method in >> `InflaterInputStream` passes the read buffer `b` to >> `Inflater::inflate(byte[] b, int off, int len)` which doesn't provide this >> guarantee. Depending on implementation details, `Inflater::inflate` might >> write more than the returned number of inflated bytes into the buffer `b`. >> >> ### TL;DR >> >> `java.util.zip.Inflater` is the Java wrapper class for zlib's inflater >> functionality. `Inflater::inflate(byte[] output, int off, int len)` >> currently calls zlib's native `inflate(..)` function and passes the address >> of `output[off]` and `len` to it via JNI. >> >> The specification of zlib's `inflate(..)` function (i.e. the [API >> documentation in the original zlib >> implementation](https://github.com/madler/zlib/blob/cacf7f1d4e3d44d871b605da3b647f07d718623f/zlib.h#L400)) >> doesn't give any guarantees with regard to usage of the output buffer. It >> only states that upon completion the function will return the number of >> bytes that have been written (i.e. "inflated") into the output buffer. >> >> The original zlib implementation only wrote as many bytes into the output >> buffer as it inflated. However, this is not a hard requirement and newer, >> more performant implementations of the zlib library like >> [zlib-chromium](https://chromium.googlesource.com/chromium/src/third_party/zlib/) >> or [zlib-cloudflare](https://github.com/cloudflare/zlib) can use more bytes >> of the output buffer than they actually inflate as a scratch buffer. See >> https://github.com/simonis/zlib-chromium for a more detailed description of >> their approach and its performance benefit. >> >> These new zlib versions can still be used transparently from Java (e.g. by >> putting them into the `LD_LIBRARY_PATH` or by using `LD_PRELOAD`), because >> they still fully comply to specification of `Inflater::inflate(..)`. >> However, we might run into problems when using the `Inflater` functionality >> from the `InflaterInputStream` class. `InflaterInputStream` is derived from >> from `InputStream` and as such, its `read(byte[] b, int off, int len)` >> method is quite constrained. It specifically specifies that if *k* bytes >> have been read, then "these bytes will be stored in elements `b[off]` >> through `b[off+`*k*`-1]`, leaving elements `b[off+`*k*`]` through >> `b[off+len-1]` **unaffected**". But `InflaterInputStream::read(byte[] b, int >> off, int len)` (which is constrained by `InputStream::read(..)`'s >> specification) calls `Inflater::inflate(byte[] b, int off, int len)` and >> directly passes its output buffer down to the native zlib `inflate(..)` >> method which is free to change the bytes beyond `b[off+` *k*`]` (where *k* is the number of inflated bytes). >> >> From a practical point of view, I don't see this as a big problem, because >> callers of `InflaterInputStream::read(byte[] b, int off, int len)` can never >> know how many bytes will be written into the output buffer `b` (and in fact >> its content can always be completely overwritten). It therefore makes no >> sense to depend on any data there being untouched after the call. Also, >> having used zlib-cloudflare productively for about two years, we haven't >> seen real-world issues because of this behavior yet. However, from a >> specification point of view it is easy to artificially construct a program >> which violates `InflaterInputStream::read(..)`'s postcondition if using one >> of the alterantive zlib implementations. A recently integrated JTreg test >> (test/jdk/jdk/nio/zipfs/ZipFSOutputStreamTest.java) "unintentionally" fails >> with zlib-chromium but can fixed easily to run with alternative >> implementations as well (see >> [JDK-8283756](https://bugs.openjdk.java.net/browse/JDK-8283756)). > > Volker Simonis has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains one new > commit since the last revision: > > 8282648: Problems due to conflicting specification of Inflater::inflate(..) > and InflaterInputStream::read(..) Hello Volker, An additional thing that we might have to consider here is whether adding this javadoc change to `InflaterInputStream` is ever going to "show up" to end user applications. What I mean is, I think in many cases the end user applications won't even know they are dealing with an `InflaterInputStream`. For example, the following code: ZipFi
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v2]
On Mon, 28 Mar 2022 17:21:40 GMT, Alan Bateman wrote:
>> src/java.base/share/classes/java/util/zip/InflaterInputStream.java line 133:
>>
>>> 131: * Unlike the {@link InputStream#read(byte[],int,int) overridden
>>> method}
>>> 132: * of {@code InputStream}, this method might write more bytes than
>>> the returned
>>> 133: * number of inflated bytes into the buffer {@code b}.
>>
>> Hello Volker, I think this comment should be a bit more clear and state what
>> exactly it means by writing more bytes than the returned value.
>> Specifically, I think it should clearly state that even though it might
>> write additional data, it will however not write/change data starting and
>> beyond `off + len` index in the passed array.
>
> I think this require a bit of word smithing. If the return value is 'n' then
> it should make it clear that the values in elements b[off + n] to b[off + len
> - 1] are undefined, their values may or may have been changed by the
> inflate/read operation. For completeness, the expectation for when inflate
> fails should be specified too, the contents of b[off] to b[off + len - 1]
> will be undefined.
Hi Volker,
I believe you are heading in the right direction. I agree with Alan and Jai
that we could use a bit more clarification
-
PR: https://git.openjdk.java.net/jdk/pull/7986
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v2]
On Mon, 28 Mar 2022 15:01:30 GMT, Jaikiran Pai wrote:
>> Volker Simonis has refreshed the contents of this pull request, and previous
>> commits have been removed. The incremental views will show differences
>> compared to the previous content of the PR. The pull request contains one
>> new commit since the last revision:
>>
>> 8282648: Problems due to conflicting specification of
>> Inflater::inflate(..) and InflaterInputStream::read(..)
>
> src/java.base/share/classes/java/util/zip/InflaterInputStream.java line 133:
>
>> 131: * Unlike the {@link InputStream#read(byte[],int,int) overridden
>> method}
>> 132: * of {@code InputStream}, this method might write more bytes than
>> the returned
>> 133: * number of inflated bytes into the buffer {@code b}.
>
> Hello Volker, I think this comment should be a bit more clear and state what
> exactly it means by writing more bytes than the returned value. Specifically,
> I think it should clearly state that even though it might write additional
> data, it will however not write/change data starting and beyond `off + len`
> index in the passed array.
I think this require a bit of word smithing. If the return value is 'n' then it
should make it clear that the values in elements b[off + n] to b[off + len - 1]
are undefined, their values may or may have been changed by the inflate/read
operation. For completeness, the expectation for when inflate fails should be
specified too, the contents of b[off] to b[off + len - 1] will be undefined.
-
PR: https://git.openjdk.java.net/jdk/pull/7986
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v2]
On Mon, 28 Mar 2022 10:55:30 GMT, Volker Simonis wrote:
>> Add an API note to `InflaterInputStream::read(byte[] b, int off, int len)`
>> to highlight that it might write more bytes than the returned number of
>> inflated bytes into the buffer `b`.
>>
>> The superclass `java.io.InputStream` specifies that `read(byte[] b, int off,
>> int len)` will leave the content beyond the last read byte in the read
>> buffer `b` unaffected. However, the overridden `read` method in
>> `InflaterInputStream` passes the read buffer `b` to
>> `Inflater::inflate(byte[] b, int off, int len)` which doesn't provide this
>> guarantee. Depending on implementation details, `Inflater::inflate` might
>> write more than the returned number of inflated bytes into the buffer `b`.
>>
>> ### TL;DR
>>
>> `java.util.zip.Inflater` is the Java wrapper class for zlib's inflater
>> functionality. `Inflater::inflate(byte[] output, int off, int len)`
>> currently calls zlib's native `inflate(..)` function and passes the address
>> of `output[off]` and `len` to it via JNI.
>>
>> The specification of zlib's `inflate(..)` function (i.e. the [API
>> documentation in the original zlib
>> implementation](https://github.com/madler/zlib/blob/cacf7f1d4e3d44d871b605da3b647f07d718623f/zlib.h#L400))
>> doesn't give any guarantees with regard to usage of the output buffer. It
>> only states that upon completion the function will return the number of
>> bytes that have been written (i.e. "inflated") into the output buffer.
>>
>> The original zlib implementation only wrote as many bytes into the output
>> buffer as it inflated. However, this is not a hard requirement and newer,
>> more performant implementations of the zlib library like
>> [zlib-chromium](https://chromium.googlesource.com/chromium/src/third_party/zlib/)
>> or [zlib-cloudflare](https://github.com/cloudflare/zlib) can use more bytes
>> of the output buffer than they actually inflate as a scratch buffer. See
>> https://github.com/simonis/zlib-chromium for a more detailed description of
>> their approach and its performance benefit.
>>
>> These new zlib versions can still be used transparently from Java (e.g. by
>> putting them into the `LD_LIBRARY_PATH` or by using `LD_PRELOAD`), because
>> they still fully comply to specification of `Inflater::inflate(..)`.
>> However, we might run into problems when using the `Inflater` functionality
>> from the `InflaterInputStream` class. `InflaterInputStream` is derived from
>> from `InputStream` and as such, its `read(byte[] b, int off, int len)`
>> method is quite constrained. It specifically specifies that if *k* bytes
>> have been read, then "these bytes will be stored in elements `b[off]`
>> through `b[off+`*k*`-1]`, leaving elements `b[off+`*k*`]` through
>> `b[off+len-1]` **unaffected**". But `InflaterInputStream::read(byte[] b, int
>> off, int len)` (which is constrained by `InputStream::read(..)`'s
>> specification) calls `Inflater::inflate(byte[] b, int off, int len)` and
>> directly passes its output buffer down to the native zlib `inflate(..)`
>> method which is free to change the bytes beyond `b[off+`
*k*`]` (where *k* is the number of inflated bytes).
>>
>> From a practical point of view, I don't see this as a big problem, because
>> callers of `InflaterInputStream::read(byte[] b, int off, int len)` can never
>> know how many bytes will be written into the output buffer `b` (and in fact
>> its content can always be completely overwritten). It therefore makes no
>> sense to depend on any data there being untouched after the call. Also,
>> having used zlib-cloudflare productively for about two years, we haven't
>> seen real-world issues because of this behavior yet. However, from a
>> specification point of view it is easy to artificially construct a program
>> which violates `InflaterInputStream::read(..)`'s postcondition if using one
>> of the alterantive zlib implementations. A recently integrated JTreg test
>> (test/jdk/jdk/nio/zipfs/ZipFSOutputStreamTest.java) "unintentionally" fails
>> with zlib-chromium but can fixed easily to run with alternative
>> implementations as well (see
>> [JDK-8283756](https://bugs.openjdk.java.net/browse/JDK-8283756)).
>
> Volker Simonis has refreshed the contents of this pull request, and previous
> commits have been removed. The incremental views will show differences
> compared to the previous content of the PR. The pull request contains one new
> commit since the last revision:
>
> 8282648: Problems due to conflicting specification of Inflater::inflate(..)
> and InflaterInputStream::read(..)
src/java.base/share/classes/java/util/zip/InflaterInputStream.java line 133:
> 131: * Unlike the {@link InputStream#read(byte[],int,int) overridden
> method}
> 132: * of {@code InputStream}, this method might write more bytes than
> the returned
> 133: * number of inflated bytes into the buffer {@code b}.
Hello Volker, I think this comm
Re: RFR: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) [v2]
> Add an API note to `InflaterInputStream::read(byte[] b, int off, int len)` to > highlight that it might write more bytes than the returned number of > inflated bytes into the buffer `b`. > > The superclass `java.io.InputStream` specifies that `read(byte[] b, int off, > int len)` will leave the content beyond the last read byte in the read buffer > `b` unaffected. However, the overridden `read` method in > `InflaterInputStream` passes the read buffer `b` to `Inflater::inflate(byte[] > b, int off, int len)` which doesn't provide this guarantee. Depending on > implementation details, `Inflater::inflate` might write more than the > returned number of inflated bytes into the buffer `b`. > > ### TL;DR > > `java.util.zip.Inflater` is the Java wrapper class for zlib's inflater > functionality. `Inflater::inflate(byte[] output, int off, int len)` currently > calls zlib's native `inflate(..)` function and passes the address of > `output[off]` and `len` to it via JNI. > > The specification of zlib's `inflate(..)` function (i.e. the [API > documentation in the original zlib > implementation](https://github.com/madler/zlib/blob/cacf7f1d4e3d44d871b605da3b647f07d718623f/zlib.h#L400)) > doesn't give any guarantees with regard to usage of the output buffer. It > only states that upon completion the function will return the number of bytes > that have been written (i.e. "inflated") into the output buffer. > > The original zlib implementation only wrote as many bytes into the output > buffer as it inflated. However, this is not a hard requirement and newer, > more performant implementations of the zlib library like > [zlib-chromium](https://chromium.googlesource.com/chromium/src/third_party/zlib/) > or [zlib-cloudflare](https://github.com/cloudflare/zlib) can use more bytes > of the output buffer than they actually inflate as a scratch buffer. See > https://github.com/simonis/zlib-chromium for a more detailed description of > their approach and its performance benefit. > > These new zlib versions can still be used transparently from Java (e.g. by > putting them into the `LD_LIBRARY_PATH` or by using `LD_PRELOAD`), because > they still fully comply to specification of `Inflater::inflate(..)`. However, > we might run into problems when using the `Inflater` functionality from the > `InflaterInputStream` class. `InflaterInputStream` is derived from from > `InputStream` and as such, its `read(byte[] b, int off, int len)` method is > quite constrained. It specifically specifies that if *k* bytes have been > read, then "these bytes will be stored in elements `b[off]` through > `b[off+`*k*`-1]`, leaving elements `b[off+`*k*`]` through `b[off+len-1]` > **unaffected**". But `InflaterInputStream::read(byte[] b, int off, int len)` > (which is constrained by `InputStream::read(..)`'s specification) calls > `Inflater::inflate(byte[] b, int off, int len)` and directly passes its > output buffer down to the native zlib `inflate(..)` method which is free to > change the bytes beyond `b[off+`* k*`]` (where *k* is the number of inflated bytes). > > From a practical point of view, I don't see this as a big problem, because > callers of `InflaterInputStream::read(byte[] b, int off, int len)` can never > know how many bytes will be written into the output buffer `b` (and in fact > its content can always be completely overwritten). It therefore makes no > sense to depend on any data there being untouched after the call. Also, > having used zlib-cloudflare productively for about two years, we haven't seen > real-world issues because of this behavior yet. However, from a specification > point of view it is easy to artificially construct a program which violates > `InflaterInputStream::read(..)`'s postcondition if using one of the > alterantive zlib implementations. A recently integrated JTreg test > (test/jdk/jdk/nio/zipfs/ZipFSOutputStreamTest.java) "unintentionally" fails > with zlib-chromium but can fixed easily to run with alternative > implementations as well (see > [JDK-8283756](https://bugs.openjdk.java.net/browse/JDK-8283756)). Volker Simonis has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8282648: Problems due to conflicting specification of Inflater::inflate(..) and InflaterInputStream::read(..) - Changes: - all: https://git.openjdk.java.net/jdk/pull/7986/files - new: https://git.openjdk.java.net/jdk/pull/7986/files/128166ff..b55fc332 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7986&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7986&range=00-01 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7986.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7986/head:pu
