On 4/21/2016 11:10 PM, Wang Weijun wrote:
>
>> On Apr 21, 2016, at 9:44 PM, Xuelei Fan <xuelei....@oracle.com> wrote:
>>
>>>> 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?
>>>
>>> Probably not, unless you call getInstance(arg, null). I am not sure this
>>> null will trigger some other exception along the way.
>>>
>>> OK, I admit there is a side effect here: If you design
>>> getInstance(alg,params) but params is always null, then you can only
>>> implement a constructor with no params.
>>>
>>> This is stupid and useless, but not really harmful.
>>>
>> Can you explain more here?
>
> The code change looks like this
>
> private static Object newInstanceUtil(final Class<?> clazz,
> final Class<?> ctrParamClz, final Object ctorParamObj)
> throws Exception {
> if (ctrParamClz == null) {
> Constructor<?> con = clazz.getConstructor();
> return con.newInstance();
> } else {
> - Constructor<?> con = clazz.getConstructor(ctrParamClz);
> - return con.newInstance(ctorParamObj);
> + try {
> + Constructor<?> con = clazz.getConstructor(ctrParamClz);
> + return con.newInstance(ctorParamObj);
> + } catch (NoSuchMethodException nsme) {
> + if (ctorParamObj == null) {
> + try {
> + Constructor<?> con = clazz.getConstructor();
> + return con.newInstance();
> + } catch (NoSuchMethodException nsme2) {
> + nsme.addSuppressed(nsme2);
> + throw nsme;
> + }
> + } else {
> + throw nsme;
> + }
> + }
> }
> }
>
> So in order for the arg-less constructor to be called, you will need
>
> 1. ctrParamClz != null, i.e. there is a getInstance(arg,params) API.
>
> 2. ctorParamObj == null, i.e. someone calls it with getInstance(arg) or
> getInstance(arg,null).
>
> 3. nsme caught, i.e. the implementation has not provided a constructor with
> args
>
> This matches the "otherwise" part of what I described in the @implSpec of
> SecureRandomSpi, which I don't suggest new implementation doing it, and is
> not what all non-SecureRandom implementations are doing now (they always have
> a constructor with args).
>
Got it.
Constructor-Fall-back is not common in general. I think this is only
apply to SecureRandom. No problem for known engines.
In the new update, there are a few description about SecureRandom, but
no control code related to SecureRandom explicitly. A reader like me
would have to ask a lot questions before I can understand the logic.
For safe, what do you think about adding a check to make sure it is for
SecureRandom only (instanceof, etc), or a note so that we don't forgot
to revise if thing get changed in the future?
A simple update may be:
- // For other primitives using params, ctorParamObj should not
+ // For other known primitives using params, ctorParamObj should not
// be null and nsme is thrown, just like before.
Thanks,
Xuelei
