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

[Artem Smotrakov]

Hi Svetlana,

The webrev looks fine to me. Please see one more comment inline.

On 07/15/2016 11:12 AM, Svetlana Nikandrova wrote:
> Hi Artem,
>
> ok, lets get rid of possible NPE in other methods too:
> http://cr.openjdk.java.net/~snikandrova/8054536/webrev.01/ 
> <http://cr.openjdk.java.net/%7Esnikandrova/8054536/webrev.01/>
I also noticed that encode(DerOutputStream) checks extensionId and 
extensionValue for null, but encode(OutputStream) doesn't.

encode(OutputStream) might just call encode(DerOutputStream), so that it 
always checks those fields for null. It also would remove some duplicate 
code.
>
> As for unnecessary try-catch in test I'd prefer to have it to emphasis 
> that we are checking for NPE.
Okay.
> I'm not sure about "iff" but let it be, it seems like a right place to 
> use it.
Sure.

Artem
>
> Thank you,
> Svetlana
>
> On 14.07.2016 19:35, Artem Smotrakov wrote:
>> Hi Svetlana,
>>
>> I'll leave the main review to an official reviewer, but I have a 
>> couple of comments.
>>
>> There are a couple of other places in Extension.java where NPE may 
>> occur:
>> - line 255: I see that "extensionId" is checked for null in other 
>> methods, but not in getId()
>> - line 200: I see that "extensionValue" is checked for null in other 
>> methids, but not in getValue()
>>
>> Minor: try-catch is not really necessary.
>>
>> I am not sure, but "iff" might mean "if and only if", so it may be 
>> not a typo.
>>
>> You may want to add @Override to a couple of methods since you update 
>> Extension.java
>>
>> Artem
>>
>> On 07/14/2016 08:19 AM, Svetlana Nikandrova wrote:
>>> Hello,
>>>
>>> please review the fix for:
>>> https://bugs.openjdk.java.net/browse/JDK-8054536
>>> Webrev:
>>> http://cr.openjdk.java.net/~snikandrova/8054536/webrev.00/ 
>>> <http://cr.openjdk.java.net/%7Esnikandrova/8054536/webrev.00/>
>>>
>>> Description:
>>> Equals and hasCode methods of Extension use extensionId without 
>>> prior check for "null" (+ 1 mistype in equals javadoc).
>>>
>>> Thank you,
>>> Svetlana
>>
>