RFR JDK-8134364: Add defensive copies to get/set methods for OCSPNonceExtension

Jamil Nimeh jamil.j.nimeh at oracle.com
Tue Aug 25 15:11:25 UTC 2015


Hi Sean,

Yes, I was just following Extension convention in terms of implementing 
CertAttrSet.  Within sun.security.x509, X509CertInfo uses the set 
methods from other objects implementing CertAttrSet. But 
OCSPNonceExtension really isn't a certificate attribute so it probably 
doesn't need to bring this interface in.

I'll refactor this and alter the tests to match.

--Jamil

On 08/25/2015 06:40 AM, Sean Mullan wrote:
> Is it necessary for the class to implement CertAttrSet? I realize all 
> of the internal X.509 extensions implement that, but that was done a 
> long time ago and not something we really want to carry forward if not 
> needed. I think it would be cleaner not to do that as it would more 
> easily allow you to make the class immutable, and would allow you to 
> remove the set method.
>
> Thanks,
> Sean
>
> On 08/25/2015 04:14 AM, Xuelei Fan wrote:
>> OCSPNonceExtension.java
>> =======================
>> -      nonceData = (byte[])obj;
>> +      nonceData = ((byte[])obj).clone();
>>
>> Do you want to check null obj?
>>
>> -      return nonceData;
>> +      return (nonceData != null ? nonceData.clone() : null);
>>
>> I think you may want to enclose the "!=" operator as:
>>
>> +      return (nonceData != null) ? nonceData.clone() : null;
>>
>>
>> Xuelei
>>
>> On 8/25/2015 12:55 PM, Jamil Nimeh wrote:
>>> Hi all,
>>>
>>> This is a quick fix to the OCSPNonceExtension class to add a couple
>>> defensive copies to public get/set methods.
>>>
>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8134364
>>> Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8134364/webrev.00
>>>
>>> Thanks,
>>> --Jamil
>>



More information about the security-dev mailing list