[JDK-6341887] Patch: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

[David Lloyd]

On Sun, Feb 18, 2018 at 3:33 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> Thanks for bringing this one up again

Thanks for taking the time to review.

> I see Sherman is looking at implementation so I'll stick with the javadoc
> for now. At some point it will need a security review to ensure that there
> no possibility of tricking the implementation to access memory outside of
> the input/output. That is, we have to assume the user of
> setInput(ByteBuffer) is evil and will change the position/limit during
> deflate operations. I see the patch computes clamps the remainder before
> accessing memory, it will just need a closer look to make sure there are no
> issues. The patch will also need adjustments to make it consistent with the
> existing code but that can come later.

I did write the code with this in mind: that the address should always
be within the bounds of the buffer (be it heap or direct) at the time
of the call, and that the offset into the buffer should not be beyond
its end (or beginning).  So the effect could be a thrown exception but
escaping the buffer _should_ be impossible (by intent anyway; more
eyes are always better for noticing mistakes of course).

> On the APIs then the inflate and deflate methods look okay, I'll less sure
> that  setDcitionary(ByteBuffer) is really needed. Are you adding for
> setDcitionary(ByteBuffer) for consistency?

Yes; it was easy enough to add it so I did.

> The javadoc doesn't currently specify how it works with the buffer, e.g.
> inflate(ByteBuffer) doesn't specify that adds it bytes to the buffer
> starting at its position, it doesn't say if the position is adjusted. The
> javadoc will also need to set expectations on behavior when
> DataFormatException is thrown, is the position guaranteed to be unchanged or
> is it unspecified?

I intended for it to work similarly to the old code.  But I'll go
through the zlib docs and just make sure all the assumptions are still
correct, and the next version will have more concise docs in this
regard and also with regards to the disposition of the buffer in each
case.

-- 
- DML