RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

[Valerie Peng]

Max,

Please find comments in line.

On 6/29/2016 8:18 PM, Wang Weijun wrote:
>> On Jun 30, 2016, at 9:39 AM, Valerie Peng <valerie.peng at oracle.com> wrote:
>>
>> Hi Max,
>>
>> Changes look fine. Just some comments/questions as below.
>>
>> <src/java.base/share/classes/sun/security/tools/keytool/Resources.java>
>> - line 46, fix this {0} as well?
> I don't see {0} in keytool/Resources.java.
>
> There is one in jarsigner/Resources.java:
>
>     46         {"signerClass.is.not.a.signing.mechanism", "{0} is not a signing mechanism"},
>
> You mean it's useless now?
Right, it's under the jarsigner directory. My eyes were crossed. ;)
Is this used still? I didn't find reference to this one.
>> <test/sun/security/tools/keytool/i18n.html>
>> - line 53, better to use -providerClass?
> Both should work.
>
> -addprovider works because SUN is already a loaded provider.
> -providerclass works because sun.security.provider.Sun is a public class in the same module.
>
> I prefer -addprovider because -providerclass is only for legacy providers loaded with reflection.
>
> In fact, I noticed that SUN is also not in ServiceLoader.load(Provider.class), which means it is not a service. Is that why you suggest the test load it with -providerclass?
I only saw -provider not -addprovider?
Correct, SUN provider is not exported as a provider for ServiceLoader. I 
don't have preference to use -providerClass over -addprovider. Whatever 
is the expected usage is fine with me.

Thanks,
Valerie
> Thanks
> Max
>
>> Thanks,
>> Valerie
>>
>> On 6/28/2016 6:09 PM, Wang Weijun wrote:
>>> Ping again to security-dev. Anyone can approve it?
>>>
>>> The latest webrev is at
>>>
>>>     http://cr.openjdk.java.net/~weijun/8130302/webrev.06
>>>
>>> Change from webrev.05 [1] is tiny.
>>>
>>> Thanks
>>> Max
>>>
>>> [1] http://cr.openjdk.java.net/~weijun/8130302/webrev.06/interdiff.patch.html
>>>
>>>> On Jun 16, 2016, at 9:33 AM, Wang Weijun <weijun.wang at oracle.com> wrote:
>>>>
>>>>
>>>>> On Jun 16, 2016, at 7:50 AM, Valerie Peng <valerie.peng at oracle.com> wrote:
>>>>>
>>>>> No big difference to me.
>>>> Good, I'll remove the cast.
>>>>
>>>> @security-dev, can someone approve the whole webrev.05?
>>>>
>>>>    http://cr.openjdk.java.net/~weijun/8130302/webrev.05
>>>>
>>>> Thanks
>>>> Max
>>>>
>>>>> Valerie
>>>>>
>>>>> On 6/15/2016 8:40 AM, Wang Weijun wrote:
>>>>>>> On Jun 15, 2016, at 10:57 PM, Mandy Chung<mandy.chung at oracle.com>  wrote:
>>>>>>>
>>>>>>>>> 241             throw (InvalidParameterException)
>>>>>>>>>
>>>>>>>>> This cast should not be needed?
>>>>>>>>>
>>>>>>>> } catch (UcryptoException ue) {
>>>>>>>>   throw (InvalidParameterException)
>>>>>>>>       new InvalidParameterException("Error using " + configArg).
>>>>>>>>           initCause(ue.getCause());
>>>>>>>> }
>>>>>>>>
>>>>>>>> initCause() returns Throwable but the method's signature throws InvalidParameterException.
>>>>>>>>
>>>>>>> Perhaps have a local variable for InvalidParameterException exception.
>>>>>> Valerie, are you OK with this?
>>>>>>
>>>>>> --Max
>>>>>>
>>>>>>> Mandy