[RFR] 8243424: Signature and SignatureSpi get parameter methods may throw null when unsupported

[Anthony Scarpino]

On 6/2/20 10:22 AM, Sean Mullan wrote:
> On 5/27/20 4:22 PM, Anthony Scarpino wrote:
>> Hi,
>>
>> I need a javadoc review for a Signature/SignatureSpi parameters change 
>> that is needed to the spec, which affects EdDSA.  The existing spec 
>> was very strict in the way set and get parameters were handled.  This 
>> modifies that slightly by allowing null to be returned by the get 
>> methods when it is unsupported after the parameters have been set. 
>> Since null was an allowed return value, this minimizes the backwards 
>> compatibility chances.
>>
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8243425
>> webrev: https://cr.openjdk.java.net/~ascarpino/8243424/webrev/
> 
> Here are a few wording/formatting comments on Signature. The same 
> comments apply to SignatureSpi since it contains similar text.
> 
> 1015      * (by calling the {@code setParameter} method) and the underlying
> 
> You should distinguish between the two overloaded setParameter methods 
> so it is clear which one you are talking about. I recommend using @link 
> instead of @code: {@link #setParameter(AlgorithmParameterSpec)}
> 
> 1016      * implementation supports returning the parameters as 
> AlgorithmParameters,
> 
> I would say "underlying signature implementation" to be consistent with 
> the previous text.
> 
> Also, change "AlgorithmParameters" (the one above and below) to "{@code 
> AlgorithmParameters}".
> 
> 1017      * this method returns the same parameters.  If parameters are 
> set using
> 1018      * {@code setParameter}, this method may return those parameters
> 
> Use "{@link #setParameter(String, Object)}" instead of "{@code 
> setParameter}"
> 
> 1019      * from the provider as AlgorithmParameters, if supported.
> 
> The first two sentences are very similar. How about combining them as:
> 
> "If this signature has been previously initialized with parameters
> (by calling {@link #setParameter(AlgorithmParameterSpec)} or {@link 
> #setParameter(String, Object)}) and the underlying signature
> implementation supports returning the parameters as {@code 
> AlgorithmParameters}, this method returns the same parameters."

If we are going to reexamine this, then I would say "previously 
initialized" is the wrong wording.  it makes it sound like it only 
applies when the signature is reset().   It should be replaced with 
"set", regardless if it is the first op or a reset op.

Tony

> 
> 
> 1020      * parameters were not set, this method may return a combination
> 1021      * of default and randomly generated parameter values if the
> 1022      * underlying signature implementation supports it and can 
> successfully
> 1023      * generate them. Otherwise, {@code null} is returned.
> 
> --Sean