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

Jamil Nimeh jamil.j.nimeh at oracle.com
Wed Mar 11 22:10:18 UTC 2015


Okay, an updated webrev has been posted that addresses Sean's comments 
(thanks, BTW).

http://cr.openjdk.java.net/~jnimeh/reviews/8074064/webrev.04/

--Jamil

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");
>     }
>
> 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)) {
>
> --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
>>>>
>>>
>>




More information about the security-dev mailing list