RFR: JDK-8291509 Minor cleanup could be done in sun.security [v3]

Weijun Wang weijun at openjdk.org
Tue Aug 30 21:10:31 UTC 2022


On Mon, 29 Aug 2022 21:48:18 GMT, Mark Powers <mpowers at openjdk.org> wrote:

>> https://bugs.openjdk.org/browse/JDK-8291509
>
> Mark Powers has updated the pull request incrementally with one additional commit since the last revision:
> 
>   white space

Files before (and equals to) `X509Factory`.

src/java.base/share/classes/sun/security/jca/ProviderList.java line 672:

> 670:             "SHA3-384", "SHA3-512" };
> 671:     private static final String[] HmacSHA3Group = { "HmacSHA3-224",
> 672:             "HmacSHA3-256", "HmacSHA3-384", "HmacSHA3-512"};

Shall I change these `static final` fields to UPPERCASE?

src/java.base/share/classes/sun/security/pkcs/PKCS7.java line 216:

> 214: 
> 215:     private void parseNetscapeCertChain(DerValue val)
> 216:     throws IOException {

No need for a newline here.

src/java.base/share/classes/sun/security/pkcs/PKCS7.java line 263:

> 261:     //     signerInfos SignerInfos }
> 262:     private void parseSignedData(DerValue val)
> 263:         throws IOException {

No need for a newline here.

src/java.base/share/classes/sun/security/pkcs/PKCS7.java line 389:

> 387:      */
> 388:     private void parseOldSignedData(DerValue val)
> 389:         throws IOException

No need for a newline here.

src/java.base/share/classes/sun/security/pkcs/SignerInfo.java line 106:

> 104:      */
> 105:     public SignerInfo(DerInputStream derin)
> 106:         throws IOException {

No need for a newline here.

src/java.base/share/classes/sun/security/pkcs/SignerInfo.java line 121:

> 119:      */
> 120:     public SignerInfo(DerInputStream derin, boolean oldStyle)
> 121:         throws IOException {

Indent 4 more spaces.

src/java.base/share/classes/sun/security/pkcs10/PKCS10.java line 369:

> 367:     private final PKCS10Attributes    attributeSet;
> 368:     private byte[]              encoded;        // signed
> 369: }

Either realign all the names or none.

src/java.base/share/classes/sun/security/pkcs10/PKCS10Attributes.java line 184:

> 182:             return false;
> 183:         PKCS10Attribute thisAttr, otherAttr;
> 184:         String key;

This whole `toString` method can be enhanced. Can we just compare `this.map.equals(that.map)`?

src/java.base/share/classes/sun/security/pkcs10/PKCS10Attributes.java line 216:

> 214:      */
> 215:     public String toString() {
> 216:         return map.size() + "\n" + map.toString();

`toString` not needed.

src/java.base/share/classes/sun/security/pkcs12/MacData.java line 47:

> 45: 
> 46:     private String digestAlgorithmName;
> 47:     private AlgorithmParameters digestAlgorithmParams;

Yes, this field is not used now. But one day we might see one that has parameters. I'd rather keep them and maybe fix `getEncoded` by then.

src/java.base/share/classes/sun/security/pkcs12/MacData.java line 58:

> 56:      */
> 57:     MacData(DerInputStream derin)
> 58:         throws IOException {

No need for a newline here.

src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 1723:

> 1721:             if (entry instanceof PrivateKeyEntry keyEntry) {
> 1722:                 certs = Objects.requireNonNullElseGet(keyEntry.chain, () ->
> 1723:                     new Certificate[0]);

I'd rather use `?:`.

src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 1809:

> 1807:      */
> 1808:     private byte[] createSafeContent()
> 1809:         throws IOException {

No need for a newline here.

src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 2213:

> 2211:             if (entry.keyId != null) {
> 2212:                 ArrayList<X509Certificate> chain =
> 2213:                         new ArrayList<>();

No need for a newline here.

src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 2389:

> 2387:     private void loadSafeContents(DerInputStream stream)
> 2388:         throws IOException, CertificateException
> 2389:     {

Put brace to previous line.

src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 2554:

> 2552:                                 Long.parseLong(keyIdStr.substring(5)));
> 2553:                     } catch (Exception e) {
> 2554:                         date = null;

Maybe add a comment?

src/java.base/share/classes/sun/security/provider/DomainKeyStore.java line 564:

> 562:         }
> 563:         return new AbstractMap.SimpleEntry<>("",
> 564:                 Collections.emptyList());

No need for a newline here.

src/java.base/share/classes/sun/security/provider/PolicyParser.java line 576:

> 574:         }
> 575: 
> 576:         return (ignoreEntry == true) ? null : e;

Remove definition of `ignoreEntry`.

src/java.base/share/classes/sun/security/provider/PolicyParser.java line 956:

> 954:             ge.principals = new LinkedList<>(this.principals);
> 955:             ge.permissionEntries =
> 956:                         new Vector<>(this.permissionEntries);

No need for a newline here.

src/java.base/share/classes/sun/security/provider/SecureRandom.java line 235:

> 233:         if (r > 0) {
> 234:             // How many bytes?
> 235:             todo = Math.min((result.length - index), (DIGEST_SIZE - r));

No need for so many parentheses.

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

PR: https://git.openjdk.org/jdk/pull/9972



More information about the security-dev mailing list