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