> 在 2018年7月20日，05:18，Valerie Peng <valerie.p...@oracle.com> 写道：
>
> Hi Sean,
>
> Thanks for the suggestion, I like it.
Me too.
>
> Max, any objection or concern before I update the webrev and CSR?
No.
Thanks
Max
>
> Valerie
>
>
>> On 7/19/2018 7:28 AM, Sean Mullan wrote:
>> Hi Valerie, Max -
>>
>> I took a fresh look at this issue this morning. I think we are getting
>> bogged down by trying to adjust within the current wording, which is
>> somewhat awkward and hard to understand. So, I think it might be better to
>> break up the wording into multiple sentences. Here's a cut at the rewording:
>>
>> /**
>> * Returns the parameters used with this signature object.
>> *
>> * <p> If this signature has been previously initialized with parameters
>> * (by calling the {@code setParameter} method), this method returns
>> * the same parameters. If this signature has not been initialized with
>> * parameters, this method may return a combination of default and
>> * randomly generated parameter values if the underlying
>> * signature implementation supports it and can successfully generate
>> * them. Otherwise, {@code null} is returned.
>> *
>> * @return the parameters used with this signature, or {@code null}
>> */
>>
>> In the first sentence of the 2nd paragraph above, I wanted to first list the
>> case where the signature is initialized with parameters, which is the most
>> clear-cut case of what the behavior will be. Then I described the case where
>> an implementation may return default/generated parameters -- and this is
>> clearly marked "may" since it is optional. All other cases other than those
>> two always return null, so I think this makes it easier to understand.
>>
>> --Sean
>>
>>> On 7/18/18 1:29 PM, Valerie Peng wrote:
>>> Sean,
>>>
>>> Where do you think that we should add the part about "null must be returned
>>> ..." paragraph? At the end of first or second paragraph?
>>>
>>> I will go with majority.
>>>
>>> Valerie
>>>
>>>> On 7/17/2018 8:38 PM, Weijun Wang wrote:
>>>> Is it better to append the new lines to the 2nd paragraph?
>>>>
>>>> Thanks
>>>> Max
>>>>
>>>>> On Jul 18, 2018, at 9:46 AM, Valerie Peng <valerie.p...@oracle.com> wrote:
>>>>>
>>>>>
>>>>> Ok, let's use "must" then. I have also added the part about default
>>>>> parameters.
>>>>> Hope that all is clear now.
>>>>>
>>>>> Latest webrev: http://cr.openjdk.java.net/~valeriep/8206171/webrev.03/
>>>>> Latest CSR: https://bugs.openjdk.java.net/browse/JDK-8206864
>>>>>
>>>>> Thanks,
>>>>> Valerie
>>>>>
>>>>> On 7/17/2018 5:50 PM, Weijun Wang wrote:
>>>>>>> On Jul 18, 2018, at 8:23 AM, Valerie Peng <valerie.p...@oracle.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>> Hi Max,
>>>>>>>
>>>>>>> Thanks for the suggestions. Please find comments inline.
>>>>>>>
>>>>>>>> On 7/16/2018 7:38 PM, Weijun Wang wrote:
>>>>>>>> CSR at https://bugs.openjdk.java.net/browse/JDK-8206864.
>>>>>>>>
>>>>>>>> - At the end of the 1st paragraph, you have now
>>>>>>>>
>>>>>>>>> However, for signature algorithm such as "RSASSA-PSS", it requires
>>>>>>>>> parameters and the one used for signing must be supplied for
>>>>>>>>> verification to succeed. It may be better to return null instead of
>>>>>>>>> returning provider-specific default parameters.
>>>>>>>> I suggest we don't talk about the reason (params must be the same for
>>>>>>>> signing and verification), we can just say something like "If there is
>>>>>>>> no provider-specific default parameters, this method should return
>>>>>>>> null before user sets one".
>>>>>>> Alright, I initially didn't put down the reason. But since Brad asked
>>>>>>> about it, I add it to the CSR in case Joe raise the same question. I
>>>>>>> will update the CSR per your suggestion.
>>>>>>>> - null may be returned
>>>>>>>>
>>>>>>>> How about "{@code null} must be returned"?
>>>>>>> How about "should"? Is there a guideline on when to use
>>>>>>> "may/should/must"? Anyone knows?
>>>>>> Even if there were guidelines on this for Java, I think we should just
>>>>>> stick to https://tools.ietf.org/html/rfc2119, except that the
>>>>>> capitalization is not necessary.
>>>>>>
>>>>>> "Must" is precise here.
>>>>>>
>>>>>>> I thought must is mostly used in mandating input arguments must not be
>>>>>>> null. But don't recall it being used much for return values.
>>>>>> "must return" appears 39 times in java/ and "should return" 19 (simple
>>>>>> grep, no line break).
>>>>>>
>>>>>> --Max
>>>>>>
>>>>>>> Thanks,
>>>>>>> Valerie
>>>>>>>
>>>>>>>> Everything else looks fine.
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Max
>>>>>>>>
>>>>>>>>> On Jul 17, 2018, at 9:46 AM, Valerie Peng <valerie.p...@oracle.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Hi Max,
>>>>>>>>>
>>>>>>>>> Good catch on the SignatureSpi class and SunMSCAPI provider, I will
>>>>>>>>> include them.
>>>>>>>>>
>>>>>>>>> As for the part about "randomly generated", I am leaning toward not
>>>>>>>>> having it as I don't see a value of specifying this.
>>>>>>>>>
>>>>>>>>> Webrev and CSR has been updated.
>>>>>>>>>
>>>>>>>>> New webrev: http://cr.openjdk.java.net/~valeriep/8206171/webrev.02/
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Valerie
>>>>>>>>>
>>>>>>>>>> On 7/16/2018 4:29 PM, Weijun Wang wrote:
>>>>>>>>>> Valerie
>>>>>>>>>>
>>>>>>>>>> Would you like to retain the word "randomly generated" somewhere?
>>>>>>>>>>
>>>>>>>>>> Also, the SignatureSpi class needs to be updated in the same way.
>>>>>>>>>>
>>>>>>>>>> Can you also update the implementation in the MSCAPI Signature
>>>>>>>>>> object?
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> Max
>>>>>>>>>>
>>>>>>>>>>> On Jul 17, 2018, at 6:16 AM, Valerie Peng <valerie.p...@oracle.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> No issues found and it seems ok to return null if no parameters is
>>>>>>>>>>> set. Thus, I have updated the webrev and CSR accordingly.
>>>>>>>>>>>
>>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8206171
>>>>>>>>>>> Webrev: http://cr.openjdk.java.net/~valeriep/8206171/webrev.01/
>>>>>>>>>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8206864
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Valerie
>>>>>>>>>>>
>>>>>>>>>>>> On 7/13/2018 11:05 AM, Valerie Peng wrote:
>>>>>>>>>>>> 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.p...@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.p...@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
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>
>
