RFR 8054536: sun.security.x509.Extension object may throw NPE for hashCode and equals method

Svetlana Nikandrova svetlana.nikandrova at oracle.com
Mon Jul 25 14:59:01 UTC 2016


Hi Max,

yes, comment states that this constructor is only for sub-classes so 
I've made it protected. Compilation didn't broke (at least in jdk and 
tests).
AFAIK extensionId should never be null, as for extensionValue there are 
extensions without value (e.g. OCSPNocheck) but in that case value is 
set to empty byte array.
I've reverted checks in equal and hashCode to let NPE be thrown:

http://cr.openjdk.java.net/~snikandrova/8054536/webrev.03/ 
<http://cr.openjdk.java.net/%7Esnikandrova/8054536/webrev.03/>

BTW, should I rename bug and review? Current title seems to be irrelevant.

Thank you,
Svetlana

On 18.07.2016 19:09, Wang Weijun wrote:
> If the constructor is only meant to be called by a child class, can we just change it to protected?
>
> Also, if extensionId and extensionValue should never be null, we should focus on not to create an Extension that has one of 2 fields as null. Inside this class, we can add Objects::requireNonNull checks in the 3-arg constructor and the static newExtension method.
>
> In case we cannot guarantee them to be non-null (For example, a child class has not fill in these 2 fields), I'd rather fail loudly instead of returning null or "null". In my humble opinion, throwing NPE in hashCode and equals is not really a bad thing.
>
> Thanks
> Max
>
>> On Jul 18, 2016, at 11:40 PM, Svetlana Nikandrova <svetlana.nikandrova at oracle.com> wrote:
>>
>> Artem,
>>
>> please see updated review:
>> http://cr.openjdk.java.net/~snikandrova/8054536/webrev.02/ <http://cr.openjdk.java.net/%7Esnikandrova/8054536/webrev.02/>
>>
>> Thanks,
>> Svetlana




More information about the security-dev mailing list