Hi Sean,
Thanks for the suggestion, I like it.
Max, any objection or concern before I update the webrev and CSR?
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
