RFR: 8207775: Better management of CipherCore buffers

[Anthony Scarpino]

Looks good. Only comment is update the copyright year. No need to regenerate a webrev. Assuming it passes all the tests I say it’s ready to push. 

Tony

> On Aug 2, 2018, at 5:15 AM, Seán Coffey <sean.coffey at oracle.com> wrote:
> 
> Thanks Tony. I was asked off thread to add some comments to help code maintenance also.
> 
> webrev updated. Hope we're nearly ready to push now.
> 
> http://cr.openjdk.java.net/~coffeys/webrev.8207775.v4/webrev/
> 
> regards,
> Sean.
> 
>> On 01/08/2018 18:41, Anthony Scarpino wrote:
>> That looks fine to me.
>> 
>> Tony
>> 
>>> On 08/01/2018 08:39 AM, Seán Coffey wrote:
>>> Thanks again for review Tony. I think you raise a good point and should give some performance gain.
>>> 
>>> for line 676: maybe this :
>>> 
>>>                   byte[] copy = Arrays.copyOf(output, len);
>>>                   if (decrypting) {
>>>                       Arrays.fill(output, (byte) 0x00);
>>>                   }
>>>                   return copy;
>>> 
>>> 
>>> perhaps the same again for around line 852 :
>>>                   byte[] copy = Arrays.copyOf(output, len);
>>>                   if (decrypting) {
>>>                       Arrays.fill(output, (byte) 0x00);
>>>                   }
>>>                   return copy;
>>> 
>>> ==
>>> 
>>> Any other reviews/comments from folks before I submit final build/test & webrev ?
>>> 
>>> Regards,
>>> Sean.
>>> 
>>>> On 01/08/18 15:55, Anthony Scarpino wrote:
>>>> 
>>>> 
>>>> My only comment is if it makes sense to have the change at 676 to also only null out on decrypt?
>>>> 
>>>> Otherwise I'm fine with the changes
>>>> 
>>>> Tony
>>>> 
>>>>> On 07/31/2018 02:04 AM, Seán Coffey wrote:
>>>>> Thanks for review Tony. Comments inline..
>>>>> 
>>>>>> On 27/07/18 21:02, Anthony Scarpino wrote:
>>>>>> If we are going to add more, here are two more ton consider:
>>>>>> 
>>>>>> - It looks like there is another Arrays.copyOf() on doFinal() line 851
>>>>> Good point.
>>>>>> - doFinal()
>>>>>> at line 897 there might be something that should be done with 'buffer'.  In particular as a result of line 963's arraycopy().
>>>>> Yes - I've identified two areas where we can be proactive about nulling out 'buffer' contents. That's around the same time where we reset 'buffered' to 0. See lines 777 and 967
>>>>> 
>>>>> http://cr.openjdk.java.net/~coffeys/webrev.8207775.v3/webrev/
>>>>> 
>>>>> regards,
>>>>> Sean.
>>>>>> 
>>>>>> Tony
>>>>>> 
>>>>>> 
>>>>>>> On 07/27/2018 08:29 AM, Seán Coffey wrote:
>>>>>>> Thanks Tony. If it's alright with you, I'd like to make one more edit for this change.
>>>>>>> 
>>>>>>> http://cr.openjdk.java.net/~coffeys/webrev.8207775.v2/webrev/
>>>>>>> 
>>>>>>> There's a condition where we can null out an array early if we're returning a copy. See lines 671-683
>>>>>>> 
>>>>>>> Regards,
>>>>>>> Sean.
>>>>>>> 
>>>>>>>> On 26/07/18 17:42, Anthony Scarpino wrote:
>>>>>>>>> On 07/26/2018 07:36 AM, Seán Coffey wrote:
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8207775
>>>>>>>>> 
>>>>>>>>> Simple enough fix to null out some internal buffers once they're no longer required.
>>>>>>>>> 
>>>>>>>>> webrev : http://cr.openjdk.java.net/~coffeys/webrev.8207775/webrev/
>>>>>>>>> 
>>>>>>>>> regards,
>>>>>>>>> Sean.
>>>>>>>>> 
>>>>>>>> 
>>>>>>>> that looks fine..
>>>>>>>> 
>>>>>>>> Tony
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>