On 16/02/2018 22:13, David Lloyd wrote:
It would be convenient to be able to inflate/deflate a direct or heap
byte buffer without having to copy it through an array first. For my
Friday mini-project this week, I've decided to take a crack at this.
The attached patch is the result. Would anyone be interested in
reviewing and maybe sponsoring this change? It would be really great
to get it in to JDK 11.
The patch includes a modification to FlaterTest to run through
permutations of using direct and heap byte buffers. Though I couldn't
get jtreg working, I did compile and run the test by hand and it seems
to pass; the build also works fine with the new code.
Extra thanks to Martin Balao for pointing me towards mapfile-vers when
I couldn't figure out why I was getting UnsatisfiedLinkError. That
one was not obvious.
Thanks for bringing this one up again, I think the last time that it was
discussed here was in 2012 when Martin Kirst proposed a patch to add
these methods. If you go through the archives then you'll see there were
several issues with that proposal before the discussion petered out.
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.
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?
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?
-Alan
