JDK-8284851 Update javax.crypto files to use proper javadoc for mentioned classes by mcpowers · Pull Request #9282 · openjdk/jdk

mcpowers · 2022-06-24T16:48:46Z

https://bugs.openjdk.org/browse/JDK-8284851

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8284851: Update javax.crypto files to use proper javadoc for mentioned classes

Reviewers

Weijun Wang (@wangweij - Reviewer) ⚠️ Review applies to 2f494fcb
Valerie Peng (@valeriepeng - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/9282/head:pull/9282
$ git checkout pull/9282

Update a local copy of the PR:
$ git checkout pull/9282
$ git pull https://git.openjdk.org/jdk pull/9282/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 9282

View PR using the GUI difftool:
$ git pr show -t 9282

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/9282.diff

bridgekeeper · 2022-06-24T16:49:28Z

👋 Welcome back mcpowers! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2022-06-24T16:50:54Z

@mcpowers The following label will be automatically applied to this pull request:

security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

TheShermanTanker · 2022-06-28T11:12:29Z

@mcpowers Seems like Skara didn't correctly detect and link the JBS entry. Try renaming your title to just "8284851" and see if that works (It may take a while)

mlbridge · 2022-06-28T14:28:09Z

Webrevs

mcpowers · 2022-06-28T14:30:21Z

Missing '-': changed "JDK 8284851" to JDK-8284851.

…

________________________________ From: TheShermanTanker ***@***.***> Sent: Tuesday, June 28, 2022 4:12 AM To: openjdk/jdk ***@***.***> Cc: Mark Powers ***@***.***>; Mention ***@***.***> Subject: [External] : Re: [openjdk/jdk] JDK 8284851 Update javax.crypto files to use proper javadoc for mentioned classes (PR #9282) @mcpowers<https://urldefense.com/v3/__https://github.com/mcpowers__;!!ACWV5N9M2RV99hQ!J4f6TfiCEgjKE5PychE69_uOAeR5Jj8_vsJ3yevJkPu-KnxHbEH6OXpgeQwmZeqhuLvlb_QC9jcFOHE7QQiI9W3dwg$> Seems like Skara didn't correctly detect and link the JBS entry. Try renaming your title to just "8284851" and see if that works (It may take a while) — Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/9282*issuecomment-1168584208__;Iw!!ACWV5N9M2RV99hQ!J4f6TfiCEgjKE5PychE69_uOAeR5Jj8_vsJ3yevJkPu-KnxHbEH6OXpgeQwmZeqhuLvlb_QC9jcFOHE7QQiKpQMjYw$>, or unsubscribe<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AXVTFJQVNTVQ36CCP32XCS3VRLM2TANCNFSM5ZYM4SZA__;!!ACWV5N9M2RV99hQ!J4f6TfiCEgjKE5PychE69_uOAeR5Jj8_vsJ3yevJkPu-KnxHbEH6OXpgeQwmZeqhuLvlb_QC9jcFOHE7QQiutUTZgQ$>. You are receiving this because you were mentioned.Message ID: ***@***.***>

valeriepeng · 2022-06-28T19:31:54Z

src/java.base/share/classes/java/security/AccessControlContext.java

- * it in an AccessControlContext object, which it returns. A sample call is
- * the following:
+ * it in an {@code AccessControlContext} object, which it returns.
+ A sample call is the following:


Missing "*"?

Yes. Good catch.

valeriepeng · 2022-06-28T20:16:37Z

src/java.base/share/classes/javax/crypto/AEADBadTagException.java


    /**
-     * Constructs a AEADBadTagException with no detail message.
+     * Constructs an <code>AEADBadTagException</code> with no detail message.


Why not using the {@code } as in other files?

I started with java.crypto first, and the first few files I looked at had <code> so I used <code>. I later realized there was a mix of styles. I tried to continue with whatever style a file was using.

using {@code }

valeriepeng · 2022-06-28T20:17:23Z

src/java.base/share/classes/javax/crypto/AEADBadTagException.java


    /**
-     * Constructs a AEADBadTagException with the specified
+     * Constructs an <code>AEADBadTagException</code> with the specified


Same as above, use {@code }?

using {@code }

valeriepeng · 2022-06-28T20:18:34Z

src/java.base/share/classes/javax/crypto/BadPaddingException.java


    /**
-     * Constructs a BadPaddingException with no detail
+     * Constructs a <code>BadPaddingException</code> with no detail


Use {@code }?

using {@code }

valeriepeng · 2022-06-28T20:19:15Z

src/java.base/share/classes/javax/crypto/BadPaddingException.java


    /**
-     * Constructs a BadPaddingException with the specified
+     * Constructs a <code>BadPaddingException</code> with the specified


Use {@code }?

using {@code }

valeriepeng · 2022-06-28T20:21:34Z

src/java.base/share/classes/javax/crypto/Cipher.java


    /**
-     * Constant used to initialize cipher to encryption mode.
+     * Constant used to initialize {@code Cipher} to encryption mode.


If it's lower case, we normally just leave it? Not sure what is the criteria for changing this. Is it because these are public static constants, so you prefer more formatting? But then, some have "object" following the {@code Cipher}, and some aren't which looks a bit inconsistent.

Personally, I'd against making this change, i.e. changing all "cipher" occurrences to "{@code Cipher}". Can we just stick to fixing what's broken?

In this case the sentence was about initializing a cipher object, so I changed it to {@code Cipher}. If a sentence says "cipher block size" then I tried to leave it alone because it's referring to an algorithm rather than a class or object. I could add "object" after {@code Cipher} if it's not present. That would make it more consistent.

Ok, add object afterwards would make it more consistent.

fixed: {@code Cipher} object

valeriepeng · 2022-06-28T20:34:45Z

src/java.base/share/classes/javax/crypto/CipherInputStream.java

- * underlying InputStream but have been additionally processed by the
- * Cipher.  The Cipher must be fully initialized before being used by
- * a CipherInputStream.
+ * A <code>CipherInputStream</code> is composed of an <code>InputStream</code>


Use {@code }? Applies to the rest of changes of this file.

using {@code }

valeriepeng · 2022-06-28T20:49:38Z

src/java.base/share/classes/javax/crypto/CipherOutputStream.java

- * that write() methods first process the data before writing them out
- * to the underlying OutputStream.  The cipher must be fully
- * initialized before being used by a CipherOutputStream.
+ * A <code>CipherOutputStream</code> is composed of an <code>OutputStream</code>


Use {@code } for all of the javadoc in this class?

using {@code }

valeriepeng · 2022-06-28T20:57:45Z

src/java.base/share/classes/javax/crypto/CipherOutputStream.java

+     * <code>OutputStream</code> and a <code>Cipher</code>.
     * <br>Note: if the specified output stream or cipher is
-     * null, a NullPointerException may be thrown later when
+     * <code>null</code>, a NullPointerException may be thrown later when


Add {@code } to NullPointerException also

using {@code }

valeriepeng · 2022-06-28T20:59:05Z

src/java.base/share/classes/javax/crypto/CipherOutputStream.java

+     * This has the effect of constructing a <code>CipherOutputStream</code>
+     * using a <code>NullCipher</code>.
+     * <br>Note: if the specified output stream is <code>null</code>, a
     * NullPointerException may be thrown later when it is used.


Add {@code } to NullPointerException also.

using {@code }

valeriepeng · 2022-06-28T21:04:42Z

src/java.base/share/classes/javax/crypto/CipherSpi.java


    /**
-     * Sets the mode of this cipher.
+     * Sets the mode of this {@code Cipher}.


Same comment as in the one in Cipher.java, prefer to leave cipher alone.

I believe we're doing {@code Cipher} object.

valeriepeng · 2022-06-28T21:06:23Z

src/java.base/share/classes/javax/crypto/CryptoAllPermission.java

     *
     * @return true if the specified permission is an
-     * instance of CryptoPermission.
+     * instance of <code>CryptoPermission</code>.


Use {@code } for all of the javadoc in this class?

using {@code }

wangweij

This is the java.security part. It spends quite some time and you can see my commenting style keeps changing. Unless debatable, no need to reply.

wangweij · 2022-06-28T19:59:24Z

src/java.base/share/classes/java/security/AccessController.java

@@ -980,21 +982,22 @@ public static <T> T doPrivilegedWithCombiner(PrivilegedExceptionAction<T> action
    /**
     * Returns the "inherited" AccessControl context. This is the context


Not sure what "AccessControl context" is. Either "access control context" or "{@code AccessControlContext}".

using {@code AccessControlContext}

wangweij · 2022-06-28T19:59:57Z

src/java.base/share/classes/java/security/AccessController.java

     * This method takes a "snapshot" of the current calling context, which
-     * includes the current Thread's inherited AccessControlContext and any
-     * limited privilege scope, and places it in an AccessControlContext object.
+     * includes the current Thread's inherited {@code AccessControlContext}


"Thread" to "thread".

Fixed in this file and a couple of others.

wangweij · 2022-06-28T20:03:02Z

src/java.base/share/classes/java/security/AccessController.java

-     * getPermission method of the AccessControlException returns the
-     * {@code perm} Permission object instance.
+     * {@code getPermission} method of the AccessControlException returns the
+     * {@code perm} {@code Permission} object instance.


Two "{@code" together. Maybe move "{@code perm}" to the end?

Moved to end.

wangweij · 2022-06-28T20:03:24Z

src/java.base/share/classes/java/security/AccessController.java

-     * the current AccessControlContext and security policy.
+     * the current {@code AccessControlContext} and security policy.
     * This method quietly returns if the access request
     * is permitted, or throws an AccessControlException otherwise. The


Want to code-ify "AccessControlException" as well?

wangweij · 2022-06-28T20:17:00Z

src/java.base/share/classes/java/security/AlgorithmParameterGenerator.java

-     * AlgorithmParameterGeneratorSpi implementation from the first
+     * A new {@code AlgorithmParameterGenerator} object encapsulating the
+     * {@code AlgorithmParameterGeneratorSpi} implementation from the first
     * Provider that supports the specified algorithm is returned.


"Provider" -> "provider".

wangweij · 2022-06-28T21:06:52Z

src/java.base/share/classes/java/security/Security.java

@@ -1031,7 +1031,7 @@ static String[] getFilterComponents(String filterKey, String filterValue) {
     * algorithms or types for the specified Java cryptographic service
     * (e.g., Signature, MessageDigest, Cipher, Mac, KeyStore). Returns
     * an empty Set if there is no provider that supports the


"set" is OK.

wangweij · 2022-06-28T21:07:30Z

src/java.base/share/classes/java/security/Security.java

     * (e.g., Signature, MessageDigest, Cipher, Mac, KeyStore). Returns
     * an empty Set if there is no provider that supports the
-     * specified service or if serviceName is null. For a complete list
+     * specified service or if serviceName is {@code null}. For a complete list


{@code serviceName}.

wangweij · 2022-06-28T21:08:06Z

src/java.base/share/classes/java/security/Security.java

@@ -1031,7 +1031,7 @@ static String[] getFilterComponents(String filterKey, String filterValue) {
     * algorithms or types for the specified Java cryptographic service
     * (e.g., Signature, MessageDigest, Cipher, Mac, KeyStore). Returns


All these Java cryptographic service type names.

using {@code }

wangweij · 2022-06-28T21:10:04Z

src/java.base/share/classes/java/security/UnresolvedPermission.java

+ * Thus, an {@code UnresolvedPermission} is essentially a "placeholder"
 * containing information about the permission.
 *
 * <p>Later, when code calls AccessController.checkPermission


AccessController.checkPermission. Maybe make it a link.

made it a link

wangweij · 2022-06-28T21:10:48Z

src/java.base/share/classes/java/security/UnresolvedPermission.java

     * @param obj the object we are testing for equality with this object.
     *
-     * @return true if obj is an UnresolvedPermission, and has the same
+     * @return true if obj is an {@code UnresolvedPermission}, and has the same


{@code obj}.

valeriepeng · 2022-07-06T23:11:15Z