RFR 7191662: JCE providers should be located via ServiceLoader
Mandy Chung
mandy.chung at oracle.com
Mon Jun 8 23:53:50 UTC 2015
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)
More information about the security-dev
mailing list