On Mon, 11 Jan 2021 21:41:56 GMT, Hai-May Chao <hc...@openjdk.org> wrote:
> This enhancement adds support for the nonce extension in OCSP request > extensions by system property jdk.security.certpath.ocspNonce. > > Please review the CSR at: > https://bugs.openjdk.java.net/browse/JDK-8257766 In general it looks pretty good. Just a couple small 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" 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 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. ------------- PR: https://git.openjdk.java.net/jdk/pull/2039
