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

Sean Mullan sean.mullan at oracle.com
Wed Mar 11 14:38:33 UTC 2015


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