RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized

[Valerie Peng]

Hmm, I like the idea of expanding null to cover both cases.
I will explore it more and see.
Thanks for the feedback,
Valerie

On 7/13/2018 6:56 AM, Sean Mullan wrote:
> On 7/12/18 10:23 PM, Weijun Wang wrote:
>>
>>
>>> On Jul 13, 2018, at 10:01 AM, Valerie Peng <valerie.peng at oracle.com> 
>>> wrote:
>>>
>>> Hi Max,
>>>
>>> The javadoc is for Signature.getParameters(), so null can be 
>>> returned for signature algorithms which do not use parameters, e.g. 
>>> SHA256withDSA. As Signature class covers all signature algorithms, I 
>>> am not sure about mentioning specific algorithm names as it may be 
>>> lengthy and even misleading unless we list out all applicable 
>>> algorithms.
>>
>> Sure.
>>
>>>
>>> The part of "default and randomly generated" is inherited from 
>>> existing javadoc. I think "default" in the aforementioned sentence 
>>> means "hardcoded values". For example, something like salt length 
>>> will likely have a fixed default value. Since we have no control 
>>> over 3rd party providers, I think we may have to keep this for 
>>> backward compatibility reason. For RSASSA-PSS sig algorithm, it 
>>> errors out if the required parameter is not given. Thus, I added the 
>>> sentence "If there are no provider-specific default values, the 
>>> underlying signature implementation may also fail".
>>
>>
>> OK, now I understand "a combination of default and randomly 
>> generated" means some part of the parameter is hardcoded and some is 
>> random. Anyway, let's keep it unchanged.
>>
>> Then, how about simply "If there are no provider-specific values" 
>> which covers both hardcoded and random values?
>>
>> "the underlying signature implementation may also fail" is still not 
>> clear to me. If I had not read the CSR I would thought an exception 
>> will be thrown when update/sign is called.
>>
>>>
>>> As for @throws, I debated about it. The main reason for not adding 
>>> one is consistency. Many (or should I say most) security classes do 
>>> not have @throws for ProviderException. If we were to add @throws 
>>> ProviderException here, we should do it across the board. Besides, 
>>> it is recommended to NOT document runtime exceptions which callers 
>>> are not prepared to handle.
>>
>> I assume other getParameters() had not added it is because it 
>> happened they can return one.
>>
>> While people does not catch runtime exceptions but my understanding 
>> is that if you mentioned "fail" in the spec maybe it's better to add 
>> a @throws for it.
>>
>> For example, @throws SecurityException for File/Files operations.
>
> Thinking more about this, I would be more inclined to recommend that 
> you change the meaning of null as the return value to cover both cases:
>
> @return the parameters used with this signature, or null if this 
> signature does not use any parameters or does not support default or 
> randomly generated parameter values
>
> I don't think it is critical to make a distinction between these 2 
> cases, because if the programmer doesn't initialize it with parameters 
> it will get a SignatureException anyway when it tries to call sign or 
> verify.
>
> It's not perfect, but probably the best you can do working within the 
> constraints of that API and minimizing compatibility risk.
>
> --Sean
>
>>
>> Thanks
>> Max
>>
>>>
>>> Thanks,
>>> Valerie
>>>
>>> On 7/10/2018 7:16 PM, Weijun Wang wrote:
>>>> Hi Valerie
>>>>
>>>> About "it *may* return", do you mean it could also return null? My 
>>>> understanding is no.
>>>>
>>>> Is it better to clarify when the implementation "may also fail"? 
>>>> From the CSR, it's this method. Can you add a @throws spec to this 
>>>> method then?
>>>>
>>>> Also, I am a little confused by "default and randomly generated". 
>>>> Does this actually mean "default (might be randomly generated)"? 
>>>> The "it may" sentence mentions "default and randomly generated" but 
>>>> the "if there" only says "default", which sounds like the the 
>>>> "randomly generated" case could be different.
>>>>
>>>> Thanks
>>>> Max
>>>>
>>>>
>>>>> On Jul 11, 2018, at 5:12 AM, Valerie Peng 
>>>>> <valerie.peng at oracle.com> wrote:
>>>>>
>>>>> Hi Brad,
>>>>>
>>>>> Would you have time to review the fix for JDK-8206171: 
>>>>> Signature#getParameters for RSASSA-PSS throws ProviderException 
>>>>> when not initialized?
>>>>> No source code changes, but just updating javadoc to mention the 
>>>>> possible failure case.
>>>>> Otherwise, JCK team expects a parameter object or null being 
>>>>> returned.
>>>>> I filed a CSR to track the javadoc clarification.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8206171
>>>>> Webrev: http://cr.openjdk.java.net/~valeriep/8206171/webrev.00/
>>>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8206864
>>>>>
>>>>> Thanks,
>>>>> Valerie
>>>>>
>>>>>
>>>>>
>>>
>>