Something related. Cipher has a similar init(..,params) and getParameters() structure and the spec is also similar.
* <p>The returned parameters may be the same that were used to initialize * this cipher, or may contain a combination of default and random * parameter values used by the underlying cipher implementation if this * cipher requires algorithm parameters but was not initialized with any. However, one can supply an incomplete parameters object in init() and getParameters() will fill in default/random values to make it complete. For example, in PBE-based Cipher, one can only include salt and iteration count in the init params, and init() will add in a random IV, and the IV can be retrieved with getParameters(). Is this something we need to clarify? Thanks Max > On Jul 25, 2018, at 7:59 AM, Bradford Wetmore <bradford.wetm...@oracle.com> > wrote: > > Returning to this briefly, looks good to me too. > > Brad > > > > On 7/19/2018 6:23 PM, Valerie Peng wrote: >> Thanks Max and Sean for the review and suggestions. I have updated the >> webrev accordingly and finalized CSR. >> Webrev: http://cr.openjdk.java.net/~valeriep/8206171/webrev.04/ >> CSR: https://bugs.openjdk.java.net/browse/JDK-8206864 >> Valerie >> On 7/19/2018 3:13 PM, Wang Weijun wrote: >>> >>>> 在 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 >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>
