On Mon, 2 Nov 2020 19:31:14 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> I wrote a whole new tests to exercise all the buffers with and without >> offsets. Hopefully that covers the concern. The test found 4 bugs.. > >> * I do not understand where the corruption comes from. The user >> provides a buffer that output is suppose to be placed into, what could it be >> corrupting? > Existing tests may not catch/check all error cases. Especially, in the past, > all calls w/ ByteBuffer inputs are converted to calls w/ byte[] inputs. Thus, > testing is focused on byte[] scenarios. In addition, since for decryption, > internal buffering is done, there may be no test checking if padding bytes > are written to the output buffer. Please see my comment follows. The > corruption that I refer to is about the padding bytes which are now written > into the output buffer with this change. > * It was my expectation that checkOutputCapacity() is making sure all the > inOfs ==<> outOfs are going to work. Does that not catch all cases? checkOutputCapacity() as the name has, only changes that the output buffer is large enough. > * outWithPadding" is excessive because it doubles the allocation for > every operation followed by a copy to the original buffer, even if the > original buffer was adequate. I'm ok with doing the extra alloc & copy in > certain situations, but not everytime. Can you be more specific what things > may fail that we don't already check for in the regression tests? For example, 1) various input/output offset combinations, e.g. inOfs <,=,> outOfs, 2) Given a output buffer of 200-byte and outOfs == 0, assuming the returned decrypted data length is 160 bytes, the bytes from index 160 and onward should not be overwritten. GCM has no padding, so you may not notice any difference if this is what you are testing. This is generic CipherCore code and changes impact all ciphers. ------------- PR: https://git.openjdk.java.net/jdk/pull/411
