On Tue, 12 Jan 2021 16:26:11 GMT, Jamil Nimeh <jni...@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