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