RFR: [Update 2] 8074064 : OCSPResponse.SingleResponse objects do not parse singleExtensions

[Jamil Nimeh]

On 03/11/2015 07:38 AM, Sean Mullan wrote:
> Hi Jamil,
>
> Just a few comments, mostly minor.
>
> * OCSPResponse
>
> 768: there is an extra space in the indentation
>
> 811: use Collections.emptyMap instead of new HashMap<>() as that does 
> not allocate a new Map object.
>
> 813-15,861-63: Use Map.values instead of Map.keySet, ex:
>
>     for (Extension ext : singleExtensions.values()) {
>         sb.append("singleExtension: " + ext + "\n");
>     }
>
Easily fixed, thanks!
> 784-807: I think this code could be compressed a bit to avoid calling 
> parseExtensions twice. You can eliminate lines 786-92, if you do 
> something like this (I think), starting at line 784:
>
>     if (tmp.available() > 0) {
>         derVal = tmp.getDerValue();
>     } else {
>         derVal == null;
>     }
>
> then what is now line 794 becomes:
>
>     if (derVal != null && derVal.isContextSpecific((byte)1)) {
>
I'll go back and take a crack at refactoring this and see if we can make 
it a bit more elegant.  I had done something that used parseExtensions 
once initially using a loop but found that I mistakenly accepted a case 
where the two context-specific tags were both present, but reversed in 
their order.  Regardless, I'll take a whack at it and see what happens.

Thanks for the comments!

--Jamil
> --Sean
>
> On 03/04/2015 05:50 PM, Jamil Nimeh wrote:
>> One more round of updates.  Only the test code and the test inputs have
>> changed.  Test input is now Base64 format, with comment headers that
>> display the OCSP response pretty-print (or an asn1parse of the
>> BasicOCSPResponse for malformed response test cases).
>>
>> http://cr.openjdk.java.net/~jnimeh/reviews/8074064/webrev.03/index.html
>>
>> Thanks, Vinnie for the feedback and suggestions!
>> --Jamil
>>
>> On 03/03/2015 04:10 PM, Jamil Nimeh wrote:
>>> Okay, I've got an updated webrev for this issue:
>>> http://cr.openjdk.java.net/~jnimeh/reviews/8074064/webrev.02/index.html
>>>
>>> Thanks,
>>> --Jamil
>>>
>>> On 03/03/2015 02:18 PM, Jamil Nimeh wrote:
>>>> Hello all, I've come across a couple edge cases that this fix doesn't
>>>> cover properly.  I'll put out another webrev in a bit that should
>>>> tighten up the singleResponse parsing, particularly with the optional
>>>> fields. It will also include a couple other negative test input 
>>>> samples.
>>>>
>>>> Thanks,
>>>> --Jamil
>>>>
>>>> On 03/02/2015 04:05 PM, Jamil Nimeh wrote:
>>>>> Hello all, this review fixes an issue in OCSPResponse where it does
>>>>> not parse singleExtensions in the SingleResponse structure.  I also
>>>>> fixed two minor deviations from the OCSP.RevocationStatus interface
>>>>> when getRevocationTime and getRevocationReason are called on good
>>>>> responses.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8074064
>>>>> Review:
>>>>> http://cr.openjdk.java.net/~jnimeh/reviews/8074064/webrev.01/index.html 
>>>>>
>>>>>
>>>>> Thank you,
>>>>> --Jamil
>>>>
>>>
>>