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

[Mark Powers]

On Tue, 30 Aug 2022 20:12:19 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Mark Powers has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   white space
>
> 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.

Fixed.

> 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.

Fixed.

> 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.

Fixed.

> 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.

Fixed.

> 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.

Fixed.

> 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.

Fixed.

> 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.

Fixed.

> 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.

I put the field back.

> 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.

Fixed.

> 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 `?:`.

Fixed.

> 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.

Fixed.

> 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.

Fixed.

> 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.

Fixed.

> 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?

`// date has been initialized to null`

> 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.

Fixed.

> src/java.base/share/classes/sun/security/provider/PolicyParser.java line 576:
> 
>> 574:         }
>> 575: 
>> 576:         return (ignoreEntry == true) ? null : e;
> 
> Remove definition of `ignoreEntry`.

I don't see how that can be done.

> 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.

Fixed.

> 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.

Fixed.

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

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