On Wed, 13 Apr 2022 06:31:51 GMT, Bradford Wetmore <wetm...@openjdk.org> wrote:
>> JDK-8284112 Minor cleanup could be done in javax.crypto > > src/java.base/share/classes/javax/crypto/CipherInputStream.java line 159: > >> 157: try { >> 158: ofinish = cipher.update(ibuffer, 0, readin, obuffer, >> ostart); >> 159: } catch (IllegalStateException e) { > > This is another one of those I would probably leave alone, just so it's clear > what should be done. Keeping the original. > src/java.base/share/classes/javax/crypto/CipherSpi.java line 29: > >> 27: >> 28: import java.nio.ByteBuffer; >> 29: import java.security.*; > > This is another one that some people will complain about. I personally > prefer this style, others prefer everything written out as long as it's not > "too many." Keeping the change for now. > src/java.base/share/classes/javax/crypto/CryptoPermission.java line 437: > >> 435: // may be the best try. >> 436: return this.algParamSpec.equals(algParamSpec); >> 437: } else return !this.checkParam; > > Please use the standard coding format. > > } else { > return !this.checkParam; > } > > or > > } > > return !this.checkParam; Using the first format. > src/java.base/share/classes/javax/crypto/CryptoPermissions.java line 158: > >> 156: >> 157: /** >> 158: * Checks if this object's PermissionCollection for permissions > > Just FYI and not for this review, but this class should really be updated to > use proper javadoc comment style, which is to tag all objects with < code > >PermissionCollection< /code > No change required. > src/java.base/share/classes/javax/crypto/ExemptionMechanism.java line 142: > >> 140: * @see java.security.Provider >> 141: */ >> 142: public static ExemptionMechanism getInstance(String algorithm) > > Others might disagree with this comment, but I would encourage you not to > make these changes throughout your PR. (@jddarcy ?) > > Even though a static method is implicitly final and IJ is correct, the final > keyword does prevent subclasses from inadvertently using the same method > signature. > > Once this is resolved, I can approve. Keeping the original here and elsewhere. > src/java.base/share/classes/javax/crypto/SealedObject.java line 449: > >> 447: final class extObjectInputStream extends ObjectInputStream { >> 448: extObjectInputStream(InputStream in) >> 449: throws IOException { > > This is another "I probably wouldn't do that..." > > It's nice to know what kind of IOExceptions you might get, so leaving > StreamCorruptedException is ok. Keeping the original. Still, why would IJ suggest removing this unless it had already figured out the exception could never happen? > src/java.base/share/classes/javax/crypto/spec/DESKeySpec.java line 49: > >> 47: * Weak/semi-weak keys copied from FIPS 74. >> 48: * >> 49: * "...The first 6 keys have duals different from themselves, hence > > Suggest you leave this alone as this is a direct quote from FIPS 74 (page 10). Keeping the original. ------------- PR: https://git.openjdk.java.net/jdk/pull/8214