Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
Ok, I included Max's description for the special PBE parameter handling. Should be enough details... Filed: https://bugs.openjdk.java.net/browse/JDK-8209038 Thanks, Valerie On 8/6/2018 7:16 AM, Sean Mullan wrote: On 8/3/18 8:19 PM, Valerie Peng wrote: I can file a follow-on issue. However, I want to clarify what we want to do before filing it. Based on current exchanges: We want to update Cipher.getParameters()/CipherSpi.engineGetParameters() with similar format/wording as Signature.getParameters(), e.g. expanding the meaning when null is returned. Yes. In addition, we also need to change the part of "same parameters" due to this special PBE parameter handling behavior. Right? I think so, but can you add a bit more details on the "special PBE parameter handling behavior"? --Sean Thanks, Valerie On 7/31/2018 12:56 PM, Sean Mullan wrote: On 7/24/18 9:38 PM, Weijun Wang wrote: Something related. Cipher has a similar init(..,params) and getParameters() structure and the spec is also similar. * 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? Yes, we should update the Cipher API to be consistent with Signature. I think this can wait until JDK 12 though. Valerie, can you file a follow-on issue? Thanks, Sean
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
On 8/3/18 8:19 PM, Valerie Peng wrote: I can file a follow-on issue. However, I want to clarify what we want to do before filing it. Based on current exchanges: We want to update Cipher.getParameters()/CipherSpi.engineGetParameters() with similar format/wording as Signature.getParameters(), e.g. expanding the meaning when null is returned. Yes. In addition, we also need to change the part of "same parameters" due to this special PBE parameter handling behavior. Right? I think so, but can you add a bit more details on the "special PBE parameter handling behavior"? --Sean Thanks, Valerie On 7/31/2018 12:56 PM, Sean Mullan wrote: On 7/24/18 9:38 PM, Weijun Wang wrote: Something related. Cipher has a similar init(..,params) and getParameters() structure and the spec is also similar. * 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? Yes, we should update the Cipher API to be consistent with Signature. I think this can wait until JDK 12 though. Valerie, can you file a follow-on issue? Thanks, Sean
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
I can file a follow-on issue. However, I want to clarify what we want to do before filing it. Based on current exchanges: We want to update Cipher.getParameters()/CipherSpi.engineGetParameters() with similar format/wording as Signature.getParameters(), e.g. expanding the meaning when null is returned. In addition, we also need to change the part of "same parameters" due to this special PBE parameter handling behavior. Right? Thanks, Valerie On 7/31/2018 12:56 PM, Sean Mullan wrote: On 7/24/18 9:38 PM, Weijun Wang wrote: Something related. Cipher has a similar init(..,params) and getParameters() structure and the spec is also similar. * 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? Yes, we should update the Cipher API to be consistent with Signature. I think this can wait until JDK 12 though. Valerie, can you file a follow-on issue? Thanks, Sean
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
On 7/24/18 9:38 PM, Weijun Wang wrote: Something related. Cipher has a similar init(..,params) and getParameters() structure and the spec is also similar. * 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? Yes, we should update the Cipher API to be consistent with Signature. I think this can wait until JDK 12 though. Valerie, can you file a follow-on issue? Thanks, Sean
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
Something related.
Cipher has a similar init(..,params) and getParameters() structure and the spec
is also similar.
* 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
> 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 写道：
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.
> *
> * 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
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
>> 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
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
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 写道：
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.
*
* 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
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
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
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
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
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 写道：
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.
*
* 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 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 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 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 wrote:
No issues found and it seems ok to return null if no parameters is set. Thus, I
have updated the webrev and
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
> 在 2018年7月20日，05:18，Valerie Peng 写道：
>
> 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.
>> *
>> * 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 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
>>> 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
> 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
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
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.
*
* 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
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
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
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
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.
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
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 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 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 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 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 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,
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
Is it better to append the new lines to the 2nd paragraph?
Thanks
Max
> On Jul 18, 2018, at 9:46 AM, Valerie Peng 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 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 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
>>> 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
>>> 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
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
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 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 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 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 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
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
Ok, I will try add it back. Thanks, Valerie On 7/17/2018 1:14 PM, Sean Mullan wrote: On 7/16/18 9:46 PM, Valerie Peng wrote: As for the part about "randomly generated", I am leaning toward not having it as I don't see a value of specifying this. Hmm, I think it is important to continue to document the case where an implementation may return default/generated parameters even if it was not explicitly initialized with any. Maybe we don't need the "randomly generated" wording, but since that wording has been there for many releases, I would be hesitant to remove it. --Sean
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
> On Jul 18, 2018, at 8:23 AM, Valerie Peng 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 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 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
> 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.
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
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?
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.
Thanks,
Valerie
Everything else looks fine.
Thanks
Max
On Jul 17, 2018, at 9:46 AM, Valerie Peng 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 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 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
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
On 7/16/18 9:46 PM, Valerie Peng wrote: As for the part about "randomly generated", I am leaning toward not having it as I don't see a value of specifying this. Hmm, I think it is important to continue to document the case where an implementation may return default/generated parameters even if it was not explicitly initialized with any. Maybe we don't need the "randomly generated" wording, but since that wording has been there for many releases, I would be hesitant to remove it. --Sean
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
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".
- null may be returned
How about "{@code null} must be returned"?
Everything else looks fine.
Thanks
Max
> On Jul 17, 2018, at 9:46 AM, Valerie Peng 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 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
>>> 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
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
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 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 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 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.
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
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 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 > 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 >>> wrote:
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
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 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 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
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
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 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 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
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
On 7/12/18 10:23 PM, Weijun Wang wrote: On Jul 13, 2018, at 10:01 AM, Valerie Peng 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 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
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
> On Jul 13, 2018, at 10:01 AM, Valerie Peng 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. 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 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 >>> >>> >>> >
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
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. 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". 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. 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 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
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
Yes, I think if an implementation can throw an exception in this case, we should add that as an @throws. For example, something like the following: @throws ProviderException if this signature engine requires parameters but does not support default or randomly generated parameter values --Sean On 7/10/18 10: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 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
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
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 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 > > >
