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

[Svetlana Nikandrova]

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

On 15.07.2016 21:32, Artem Smotrakov wrote:
> 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
>>>
>>
>