RFR 7191662: JCE providers should be located via ServiceLoader

[Mandy Chung]

JDK-8066492

Mandy

On 06/08/2015 04:44 PM, Valerie Peng wrote:
>
> What is the bug id for the image refresh change? Just so I can keep a 
> watch and hold my changes for the time being.
>
> My webrev has a new regression test which compares the list of 
> providers found by ServiceLoader and Security.getProviders() call. So, 
> if the META-INF/services/java.security.Provider file content is not 
> correct, the new test would fail and it's a clear indication that 
> ServiceLoader can't find one or more of the providers.
>
> Thanks,
> Valerie
>
> On 6/5/2015 10:43 PM, Mandy Chung wrote:
>>> On Jun 5, 2015, at 4:32 PM, Valerie Peng<valerie.peng at oracle.com>  
>>> wrote:
>>>
>>>
>>> I don't know image builder well enough to answer your question.
>> The current implementation of the image builder sorts the modules by 
>> their name that are mapped to the same class loader. That explains 
>> why java.naming is the first one containing 
>> META-INF/services/java.security.Provider in your current list.
>>
>> There is no guarantee in what particular order a module is processed 
>> first.   I don’t know if the jimage refresh work will change the 
>> ordering either.  Since this is temporary, I’m okay if you want to 
>> depend on the image build implementation as long as you have a test 
>> to catch it and verify 
>> java.naming/META-INF/services/java.security.Provider file content.   
>> The existing security tests may also catch it but I think it’s not 
>> obvious to indicate that the security tests fail because of the issue 
>> of the merged service configuration file.
>>
>> Please also hold off until the image refresh change goes into 
>> jdk9/dev so that you can verify if your build change still works.
>>
>> If you want to push earlier, another alternative we discussed earlier 
>> is to separate the META-INF/services file and make change and 
>> java.security to keep using classname.  That can be pushed that in a 
>> second phase with a proper image builder support.
>>
>> Mandy
>>
>>> Based on my own experiment, it seems to pick up the one from 
>>> java.naming when duplication occurs, so that's why I saved the 
>>> merged result to there and named the Gensrc makefile with 
>>> java.naming. The result build work fine.
>>>
>>> Does this explain what I am trying to do here? If you have better 
>>> ways to get this done, I am certainly open to that idea.
>>> Thanks,
>>> Valerie
>>>
>>> On 6/5/2015 12:21 AM, Erik Joelsson wrote:
>>>> Hello Valerie,
>>>>
>>>> The merging seems ok, but I thought there was non determinism in 
>>>> the image builder regarding which provider would get picked up. Is 
>>>> that resolved or do you really need to override all of those 
>>>> providers with your generated file in gensrc? I can assist in 
>>>> writing that makefile logic if needed.
>>>>
>>>> /Erik
>>>>
>>>> On 2015-06-04 23:58, Valerie Peng wrote:
>>>>> Build experts,
>>>>>
>>>>> Can you please review the make file related change, i.e. the new 
>>>>> file make/gensrc/Gensrc-java.naming.gmk, in the following webrev: 
>>>>> http://cr.openjdk.java.net/~valeriep/7191662/webrev.03
>>>>>
>>>>> This is for merging the java.security.Provider file from various 
>>>>> providers and use the (merged) result for the final image build.
>>>>>
>>>>> Thanks,
>>>>> Valerie
>>>>>
>>>>> On 6/3/2015 10:27 AM, Valerie (Yu-Ching) Peng wrote:
>>>>>> Correct, if the makefile related changes are removed then no need 
>>>>>> for build team to review 7191662 webrev anymore.
>>>>>> There are other discussions ongoing and we should be able to 
>>>>>> reach a decision in a day or two.
>>>>>> Will update the list again.
>>>>>> Thanks,
>>>>>> Valerie
>>>>>>
>>>>>> On 06/01/15 16:39, Magnus Ihse Bursie wrote:
>>>>>>> On 2015-05-29 00:10, Valerie Peng wrote:
>>>>>>>> Please find comments in line...
>>>>>>>>
>>>>>>>> On 5/27/2015 3:42 PM, Mandy Chung wrote:
>>>>>>>>> Valerie,
>>>>>>>>>
>>>>>>>>> Did you see my comment yesterday?
>>>>>>>>> http://mail.openjdk.java.net/pipermail/security-dev/2015-May/012254.html 
>>>>>>>>>
>>>>>>>> Yes, we exchanged emails after this above one. I will follow up 
>>>>>>>> your latest one later today.
>>>>>>>>
>>>>>>>>> Since you have reverted the java.security to keep the 
>>>>>>>>> classname, to avoid causing merge conflict to jimage refresh, 
>>>>>>>>> let’s remove the META-INF files in the first push and the 
>>>>>>>>> build change.
>>>>>>>>>
>>>>>>>>> The security providers will be loaded via the fallback 
>>>>>>>>> mechanism (i.e. ProviderLoader.legacyLoad method).  You should 
>>>>>>>>> keep the ProviderLoader.load method to take the provider name 
>>>>>>>>> instead of classname.
>>>>>>>> Sure, I can remove the META-INF files so the providers are 
>>>>>>>> loaded through the legacyLoad().
>>>>>>>> Hmm, the ProviderLoader.load() method is used by java.security 
>>>>>>>> file provider loading. Since the current list still uses class 
>>>>>>>> name, it should take class name when checking for matches while 
>>>>>>>> iterating through the list returned by ServiceLoader.
>>>>>>>> This way, when changes are sync'ed into Jake, no extra change 
>>>>>>>> required and the providers will be loaded through 
>>>>>>>> ProviderLoader.load() automatically with the current list.
>>>>>>>>
>>>>>>>>> I’ll file a bug to follow up to change java.security to list 
>>>>>>>>> the provider name.  This will wait after the jimage refresh 
>>>>>>>>> goes into jdk9/dev
>>>>>>>> Ok.
>>>>>>>> Thanks,
>>>>>>>> Valerie
>>>>>>> I'm not sure I followed completely here were this landed. Does 
>>>>>>> this mean that there's currently no need for a build system code 
>>>>>>> review on 
>>>>>>> http://cr.openjdk.java.net/~valeriep/7191662/webrev.01/, and 
>>>>>>> that a new webrev will be posted instead?
>>>>>>>
>>>>>>> /Magnus
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>> .
>>>>>>>>>
>>>>>>>>> Mandy
>>>>>>>>>
>>>>>>>>>> On May 27, 2015, at 3:29 PM, Valerie 
>>>>>>>>>> Peng<valerie.peng at oracle.com>   wrote:
>>>>>>>>>>
>>>>>>>>>> Hi, build experts,
>>>>>>>>>>
>>>>>>>>>> Can you please review the make file related change, i.e. the 
>>>>>>>>>> new file make/gensrc/Gensrc-java.naming.gmk, in the following 
>>>>>>>>>> webrev:
>>>>>>>>>> http://cr.openjdk.java.net/~valeriep/7191662/webrev.01/
>>>>>>>>>>
>>>>>>>>>> This is for merging the java.security.Provider file from 
>>>>>>>>>> various providers and use the (merged) result for the final 
>>>>>>>>>> image build.
>>>>>>>>>>
>>>>>>>>>> The rest of source code changes are reviewed by my team already.
>>>>>>>>>> Thanks,
>>>>>>>>>> Valerie
>>>>>>>>>> (Java Security Team)