RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol… [v2]

Hai-May Chao hchao at openjdk.java.net
Tue Jan 12 19:18:20 UTC 2021


On Tue, 12 Jan 2021 16:26:11 GMT, Jamil Nimeh <jnimeh at openjdk.org> wrote:

>> Hai-May Chao has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   update to use List.of() and typo changes
>
> In general it looks pretty good.  Just a couple small comments.

Thanks for the review, Jamil. I've updated with all of your comments.

> src/java.base/share/classes/sun/security/provider/certpath/OCSPNonceExtension.java line 126:
> 
>> 124:      *      is set for this extension
>> 125:      * @param incomingNonce The nonce data to be set for the extension.  This
>> 126:      *      must be a non-null array of at least one byte long and can be upto
> 
> typo: "upto" -> "up to"

fixed.

> src/java.base/share/classes/sun/security/provider/certpath/OCSPNonceExtension.java line 143:
> 
>> 141:         // RFC 8954, section 2.1: the length of the nonce MUST be at least 1 octet
>> 142:         // and can be up to 32 octets.
>> 143:         if (incomingNonce.length > 0 && incomingNonce.length <=32) {
> 
> nit: space after the <= to be consistent with style elsewhere

fixed.

> src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java line 118:
> 
>> 116:                 tmpExtensions = new ArrayList<Extension>();
>> 117:                 tmpExtensions.add(nonceExt);
>> 118:                 setOcspExtensions(tmpExtensions);
> 
> It seems like you could collapse 113 - 118 into something like:
> setOcspExtensions(List.of(new OCSPNonceExtension(DEFAULT_NONCE_BYTES)));
> 
> At the very least, it looks like you could do away with 113, since you immediately change the value of the tmpExtensions reference on 116.

collapsing done.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2039



More information about the security-dev mailing list