On Tue, 26 Apr 2022 17:51:45 GMT, Valerie Peng <valer...@openjdk.org> wrote:

>> This is to update the method javadoc of 
>> java.security.Signature.getParameters() with the missing `@throws 
>> UnsupportedOperationException`. In addition, the wording on the returned 
>> parameters are updated to match those in Cipher and CipherSpi classes. 
>> 
>> CSR will be filed later.
>> 
>> Thanks,
>> Valerie
>
> Valerie Peng has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Undo un-intentional changes.

src/java.base/share/classes/java/security/Signature.java line 1012:

> 1010:      * values used by the underlying signature implementation if the 
> underlying
> 1011:      * signature implementation supports returning the parameters as
> 1012:      * {@code AlgorithmParameters}. If the required

"... if the underlying signature implementation supports returning the 
parameters a {@code AlgorithmParameters}", it looks this part is duplicated and 
may be removed.

src/java.base/share/classes/java/security/Signature.java line 1014:

> 1012:      * {@code AlgorithmParameters}. If the required
> 1013:      * parameters were not supplied and the underlying signature 
> implementation
> 1014:      * can generate the parameter values, it will be returned. 
> Otherwise,

> If the required parameters were not supplied and the underlying signature 
> implementation can generate the parameter values, it will be returned.

What does it refer to with 'it'? Is 'it' refer to the implementation generated 
parameter values?

> If the required parameters were not supplied and the underlying signature 
> implementation can generate the parameter values, it will be returned. 
> Otherwise, {@code null} is returned.

The logic looks like

    if (A & B) {
        // it will be returned
    } else {
        // {@code null} is returned.
    }

If I read it correctly, the behavior may look like:
1. If the required parameters were supplied, {@code null} is returned; (if !A)
2. if the underlying signature implementation cannot generate the parameter 
values, {@code null} is returned; (if !B)
3. if not 1 and 2, ... (if A & B)

It does not look like right.  The expected behavior may be:

    if (A) {
        if (B) {
            // it will be returned
        } else {
            // {@code null} is returned.
        }
    }


Maybe, the confusion could be mitigated by re-org the logic:

     if (A & !B) {
            // {@code null} is returned.
     } // Otherwise, refer to 1st sentence.


"The returned parameters may be the same that were used to initialize this 
signature, or may contain additional default or random parameter values used by 
the underlying signature implementation.   {@code null} is returned if the 
required parameters were not supplied and the underlying signature 
implementation cannot generate the parameter values."

Similar comment to [PR 8117](https://github.com/openjdk/jdk/pull/8117), if you 
want to use similar description there as well.

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

PR: https://git.openjdk.java.net/jdk/pull/8396

Reply via email to