On 4/20/2016 9:17 AM, Wang Weijun wrote:
>
>> On Apr 20, 2016, at 7:41 AM, Xuelei Fan <[email protected]> wrote:
>>
>> On 4/19/2016 11:41 PM, Wang Weijun wrote:
>>>>>>> http://cr.openjdk.java.net/~weijun/8051408/webrev.10/
>>>>>
>>>>> Please update copyright dates.
>>>>>
>>>>> src/java.base/share/classes/java/security/Provider.java
>>>>> -------------------------------------------------------
>>>>> 145-151: Looks like the comment are not correct. There are
>>>>> getInstance(alg,params) since JDK 1.4 (See CertStore)
>>> Yes, but in those cases, there is either getInstance(alg) or
>>> getInstance(alg,params), but not both. In SecureRandom, we are now
>>> supporting both, and a fallback is needed for those implementations that
>>> does not override new SecureRandomSpi(SRP).
>>>
>>>>>
>>>>> 156-157: Fallback change the behavior significantly. Construct with or
>>>>> without parameter can be very differently. For example, LDAP cert store
>>>>> get requested, but the CertStore.getInstane(String,
>>>>> LDAPCertStoreParameters) may not return an LDAP cert store. Can you
>>>>> make more comment why the fallback is OK?
>>> For CertStore (and Configuration and Policy), all of its implementations
>>> only support the with-arg constructor,
>> I think the constructor is not of CertStore, but of CertStoreSpi
>> implementation.
>
> Yes.
>
>> The provider implementation may support any kind of
>> constructor (implicit or explicit, intended or not). The constructor
>> should be unknown to Provider.
>
> Not sure what you mean, but Provider uses reflection to get the constructor
> and create new instances. By convention, when something is created via
> getInstance(alg), the SPI class would provide the arg-less constructor; when
> via getInstance(alg,params), it would provide the with-arg constructor.
>
Not sure what you mean.
public MyCertStore extends CertStoreSpi {
public MyCertStore() {
// whatever
// ;-) Don't ask me why this construct is necessary.
}
public MyCertStore(XXX params) {
// throws NoSuchMethodException
// ;-) Don't ask me why throw this exception.
}
}
newInstanceUtil(MyCertStore, ...)
The MyCertStore() would get called, unexpectly. Am I missing something?
>>
>>> so NoSuchMethodException will never be thrown. Again, this fallback is only
>>> for SecureRandom now.
>>>
>>>
>> Please add more comment and control so that this only apply to SecureRandom.
>
> It already had "This is to support the enhanced SecureRandom".
>
My understanding is: this is to support the enhanced SecureRandom, and
it now applies to non-SecureRandom too.
If it does not apply to non-SecureRandom, please add comment and update
the code accordingly.
Xuelei
