Review Request: loading JCE providers using ServiceLoader

[Sean Mullan]

On 08/10/2012 07:46 PM, Mandy Chung wrote:
> Hi Sean,
>
> On 8/9/2012 8:14 AM, Sean Mullan wrote:
>
>> Please review the 2nd webrev for my code changes to enable JCE
>> providers to be
>> registered and loaded as services using the ServiceLoader API:
>>
>> http://cr.openjdk.java.net/~mullan/jigsaw/webrevs/jce-service/webrev.01/
> This looks good and cleaner with the refactoring you have done.
> I agree we should move forward with this and address the open issues
> next. One issue to add to the list is about the "java.security"
> config file that lists the security provider classname.  As Alan points
> out, they are (likely) not exported in modules.

Right, but just listing the classnames in the java.security file doesn't 
cause any problems. It is only a problem if an application instantiates 
them directly, ex:

MessageDigest md = MessageDigest.getInstance("SHA-256", new 
sun.security.provider.SUN());

There are probably some applications doing that, although it is 
discouraged in our docs, and we advise using the provider's String name 
instead if you want to use a specific provider:

MessageDigest md = MessageDigest.getInstance("SHA-256", "SUN");

> Also all the Providers
> available as services should be automatically added to the list of
> registered providers (I think your patch only registers the ones
> listed in java.security).  These are related to the configured preference
> order.

I thought about doing that, just adding them on to the end. But there 
are a few other subtle issues. Right now, if you want to use a JCE 
provider that isn't registered, you instantiate it directly (as I showed 
above). Or you could also instantiate it and use the 
Security.addProvider or insertProviderAt methods to add it to the list 
of registered providers. There are apps doing that today. That code may 
break (addProvider will return -1) if we automatically registered all 
JCE providers that were found by ServiceLoader. So I would rather hold 
off on this and address it later with the ordering of providers. We may 
need to adjust what it actually means for a JCE provider to be installed:

http://docs.oracle.com/javase/7/docs/api/java/security/Security.html#getProviders%28%29

Right now "installed" means as configured in the java.security file.

> make/modules/modules.group
>    jdk.depconfig is intended for auto-generation of these
>    service dependency and implementation providing services
>    but META-INF/services/java.security.Provider doesn't exist.
>    That's why you need to declare these "provides service"
>    explicitly.  Since we're going to check in module-info.java
>    in the source tree, I'm okay with this workaround.
>    It might worth adding a comment for that.
>
> Most of the changes except Providers.ModuleProviderLoader are
> refactoring.  It's good to get that non-module-specific change
> into jdk8 to reduce the number of changes in jigsaw/jigsaw to
> carry that would help avoid merge issues.  It's fine to push
> that to jdk8 after this gets into jigsaw/jigsaw - whichever
> is convenient.

Ok, I will do that after I push it into jigsaw.

Thanks,
Sean