On Wed, 13 Apr 2022 06:31:51 GMT, Bradford Wetmore <[email protected]> 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
