(10) RFR of JDK-8184165: sun.security.provider.PolicyFile$PolicyEntry.toString() throws MissingResourceException

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

(10) RFR of JDK-8184165: sun.security.provider.PolicyFile$PolicyEntry.toString() throws MissingResourceException

Hamlin Li
Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8184165

webrev: http://cr.openjdk.java.net/~mli/8184165/webrev.00/


Thank you

-Hamlin

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: (10) RFR of JDK-8184165: sun.security.provider.PolicyFile$PolicyEntry.toString() throws MissingResourceException

Weijun Wang
Change looks fine.

Please remember to add a noreg-trivial label.

Also, can you do some more investigation when this starts to happen? The bug says affected versions are 9 and 10 but PolicyFile.java has been there long long ago. Was there a regression?

Thanks
Max

> On Jul 12, 2017, at 10:28 AM, Hamlin Li <[hidden email]> wrote:
>
> Would you please review the below patch?
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8184165
>
> webrev: http://cr.openjdk.java.net/~mli/8184165/webrev.00/
>
>
> Thank you
>
> -Hamlin
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: (10) RFR of JDK-8184165: sun.security.provider.PolicyFile$PolicyEntry.toString() throws MissingResourceException

Hamlin Li
Hi Max,

On 2017/7/12 10:50, Weijun Wang wrote:
> Change looks fine.
>
> Please remember to add a noreg-trivial label.
Added the label, and will push the change.
> Also, can you do some more investigation when this starts to happen? The bug says affected versions are 9 and 10 but PolicyFile.java has been there long long ago. Was there a regression?
The same code is there since jdk6
(http://hg.openjdk.java.net/jdk6/jdk6/jdk) , I did not check jdk5...
I don't think it's a regression, it should be just a missing resource,
as the failure only occurs when accessing very details of
sun.security.provider.PolicyFile by reflection, I guess people seldom do
that.

Thank you
-Hamlin

>
> Thanks
> Max
>
>> On Jul 12, 2017, at 10:28 AM, Hamlin Li <[hidden email]> wrote:
>>
>> Would you please review the below patch?
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8184165
>>
>> webrev: http://cr.openjdk.java.net/~mli/8184165/webrev.00/
>>
>>
>> Thank you
>>
>> -Hamlin
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: (10) RFR of JDK-8184165: sun.security.provider.PolicyFile$PolicyEntry.toString() throws MissingResourceException

Weijun Wang

> On Jul 12, 2017, at 11:03 AM, Hamlin Li <[hidden email]> wrote:
>
> Hi Max,
>
> On 2017/7/12 10:50, Weijun Wang wrote:
>> Change looks fine.
>>
>> Please remember to add a noreg-trivial label.
> Added the label, and will push the change.
>> Also, can you do some more investigation when this starts to happen? The bug says affected versions are 9 and 10 but PolicyFile.java has been there long long ago. Was there a regression?
> The same code is there since jdk6 (http://hg.openjdk.java.net/jdk6/jdk6/jdk) , I did not check jdk5...
> I don't think it's a regression, it should be just a missing resource, as the failure only occurs when accessing very details of sun.security.provider.PolicyFile by reflection, I guess people seldom do that.

Is it easy to verify?

I asked if it’s a regression because I remember some time last year there is some rearrangement of codes in this area.

--Max

>
> Thank you
> -Hamlin
>>
>> Thanks
>> Max
>>
>>> On Jul 12, 2017, at 10:28 AM, Hamlin Li <[hidden email]> wrote:
>>>
>>> Would you please review the below patch?
>>>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8184165
>>>
>>> webrev: http://cr.openjdk.java.net/~mli/8184165/webrev.00/
>>>
>>>
>>> Thank you
>>>
>>> -Hamlin
>>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: (10) RFR of JDK-8184165: sun.security.provider.PolicyFile$PolicyEntry.toString() throws MissingResourceException

Hamlin Li


On 2017/7/12 11:06, Weijun Wang wrote:

>> On Jul 12, 2017, at 11:03 AM, Hamlin Li <[hidden email]> wrote:
>>
>> Hi Max,
>>
>> On 2017/7/12 10:50, Weijun Wang wrote:
>>> Change looks fine.
>>>
>>> Please remember to add a noreg-trivial label.
>> Added the label, and will push the change.
>>> Also, can you do some more investigation when this starts to happen? The bug says affected versions are 9 and 10 but PolicyFile.java has been there long long ago. Was there a regression?
>> The same code is there since jdk6 (http://hg.openjdk.java.net/jdk6/jdk6/jdk) , I did not check jdk5...
>> I don't think it's a regression, it should be just a missing resource, as the failure only occurs when accessing very details of sun.security.provider.PolicyFile by reflection, I guess people seldom do that.
> Is it easy to verify?
>
> I asked if it’s a regression because I remember some time last year there is some rearrangement of codes in this area.
I just checked jdk6, 7, 8. You're right, there is a regression in jdk 8.
In summary,
   in jdk6, the issue exists;
   in jdk7 some resources were added which I believe fixed this issue;
(too many, I can not list it here.)
   in jdk8 the added resources were totally removed, and another 2
resources are added:
 >         {"duplicate.keystore.domain.name","duplicate keystore domain
name: {0}"},
 >         {"duplicate.keystore.name","duplicate keystore name: {0}"},

But currently I don't know how big the change impacts, as there are many
resource usage under sun/security which use
sun/security/util/[Resources|AuthResources].java

sun/security/util/Resources in jdk6,
http://hg.openjdk.java.net/jdk6/jdk6/jdk/file/62df9772b849/src/share/classes/sun/security/util/Resources.java
sun/security/util/Resources in jdk7,
http://hg.openjdk.java.net/jdk7/jdk7/jdk/file/9b8c96f96a0f/src/share/classes/sun/security/util/Resources.java
sun/security/util/Resources in jdk8,
http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/sun/security/util/Resources.java

Thank you
-Hamlin

>
> --Max
>
>> Thank you
>> -Hamlin
>>> Thanks
>>> Max
>>>
>>>> On Jul 12, 2017, at 10:28 AM, Hamlin Li <[hidden email]> wrote:
>>>>
>>>> Would you please review the below patch?
>>>>
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8184165
>>>>
>>>> webrev: http://cr.openjdk.java.net/~mli/8184165/webrev.00/
>>>>
>>>>
>>>> Thank you
>>>>
>>>> -Hamlin
>>>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: (10) RFR of JDK-8184165: sun.security.provider.PolicyFile$PolicyEntry.toString() throws MissingResourceException

Hamlin Li
I manually checked *Resources.java under sun/security and usage of them,
my investigation shows that overall quality of "static" resource usage
is good (by "static" I mean it uses hard code resource string like
rb.getString("xxxx") rather than rb.getString(aStringVariable) ), except
of this issue and another new issue:
https://bugs.openjdk.java.net/browse/JDK-8184234, in which similar issue
in sun.security.provider.AuthPolicyFile$PolicyEntry is reported.

For new issue JDK-8184234, it existed since jdk6 to 10.

Thank you

-Hamlin


On 2017/7/12 11:36, Hamlin Li wrote:

>
>
> On 2017/7/12 11:06, Weijun Wang wrote:
>>> On Jul 12, 2017, at 11:03 AM, Hamlin Li <[hidden email]> wrote:
>>>
>>> Hi Max,
>>>
>>> On 2017/7/12 10:50, Weijun Wang wrote:
>>>> Change looks fine.
>>>>
>>>> Please remember to add a noreg-trivial label.
>>> Added the label, and will push the change.
>>>> Also, can you do some more investigation when this starts to
>>>> happen? The bug says affected versions are 9 and 10 but
>>>> PolicyFile.java has been there long long ago. Was there a regression?
>>> The same code is there since jdk6
>>> (http://hg.openjdk.java.net/jdk6/jdk6/jdk) , I did not check jdk5...
>>> I don't think it's a regression, it should be just a missing
>>> resource, as the failure only occurs when accessing very details of
>>> sun.security.provider.PolicyFile by reflection, I guess people
>>> seldom do that.
>> Is it easy to verify?
>>
>> I asked if it’s a regression because I remember some time last year
>> there is some rearrangement of codes in this area.
> I just checked jdk6, 7, 8. You're right, there is a regression in jdk 8.
> In summary,
>   in jdk6, the issue exists;
>   in jdk7 some resources were added which I believe fixed this issue;
> (too many, I can not list it here.)
>   in jdk8 the added resources were totally removed, and another 2
> resources are added:
> >         {"duplicate.keystore.domain.name","duplicate keystore domain
> name: {0}"},
> >         {"duplicate.keystore.name","duplicate keystore name: {0}"},
>
> But currently I don't know how big the change impacts, as there are
> many resource usage under sun/security which use
> sun/security/util/[Resources|AuthResources].java
>
> sun/security/util/Resources in jdk6,
> http://hg.openjdk.java.net/jdk6/jdk6/jdk/file/62df9772b849/src/share/classes/sun/security/util/Resources.java
> sun/security/util/Resources in jdk7,
> http://hg.openjdk.java.net/jdk7/jdk7/jdk/file/9b8c96f96a0f/src/share/classes/sun/security/util/Resources.java
> sun/security/util/Resources in jdk8,
> http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/sun/security/util/Resources.java
>
> Thank you
> -Hamlin
>>
>> --Max
>>
>>> Thank you
>>> -Hamlin
>>>> Thanks
>>>> Max
>>>>
>>>>> On Jul 12, 2017, at 10:28 AM, Hamlin Li <[hidden email]>
>>>>> wrote:
>>>>>
>>>>> Would you please review the below patch?
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8184165
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~mli/8184165/webrev.00/
>>>>>
>>>>>
>>>>> Thank you
>>>>>
>>>>> -Hamlin
>>>>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: (10) RFR of JDK-8184165: sun.security.provider.PolicyFile$PolicyEntry.toString() throws MissingResourceException

Weijun Wang
Great work! Thanks for going deep on this issue.

Is it possible to change your manual check into an automatic test? I know sources might not be available when running a test, but if something like ${test.src}/../../../src/java.base/share/classes/sun/security/util/ exists it can be a good hint that source codes are available.

Feel free to ignore my suggestion if this is useless. I don’t actually know if the src directory is there in mach5 and JPRT tests.

--Max

> On Jul 12, 2017, at 2:07 PM, Hamlin Li <[hidden email]> wrote:
>
> I manually checked *Resources.java under sun/security and usage of them, my investigation shows that overall quality of "static" resource usage is good (by "static" I mean it uses hard code resource string like rb.getString("xxxx") rather than rb.getString(aStringVariable) ), except of this issue and another new issue: https://bugs.openjdk.java.net/browse/JDK-8184234, in which similar issue in sun.security.provider.AuthPolicyFile$PolicyEntry is reported.
>
> For new issue JDK-8184234, it existed since jdk6 to 10.
>
> Thank you
>
> -Hamlin
>
>
> On 2017/7/12 11:36, Hamlin Li wrote:
>>
>>
>> On 2017/7/12 11:06, Weijun Wang wrote:
>>>> On Jul 12, 2017, at 11:03 AM, Hamlin Li <[hidden email]> wrote:
>>>>
>>>> Hi Max,
>>>>
>>>> On 2017/7/12 10:50, Weijun Wang wrote:
>>>>> Change looks fine.
>>>>>
>>>>> Please remember to add a noreg-trivial label.
>>>> Added the label, and will push the change.
>>>>> Also, can you do some more investigation when this starts to happen? The bug says affected versions are 9 and 10 but PolicyFile.java has been there long long ago. Was there a regression?
>>>> The same code is there since jdk6 (http://hg.openjdk.java.net/jdk6/jdk6/jdk) , I did not check jdk5...
>>>> I don't think it's a regression, it should be just a missing resource, as the failure only occurs when accessing very details of sun.security.provider.PolicyFile by reflection, I guess people seldom do that.
>>> Is it easy to verify?
>>>
>>> I asked if it’s a regression because I remember some time last year there is some rearrangement of codes in this area.
>> I just checked jdk6, 7, 8. You're right, there is a regression in jdk 8.
>> In summary,
>>  in jdk6, the issue exists;
>>  in jdk7 some resources were added which I believe fixed this issue; (too many, I can not list it here.)
>>  in jdk8 the added resources were totally removed, and another 2 resources are added:
>> >         {"duplicate.keystore.domain.name","duplicate keystore domain name: {0}"},
>> >         {"duplicate.keystore.name","duplicate keystore name: {0}"},
>>
>> But currently I don't know how big the change impacts, as there are many resource usage under sun/security which use sun/security/util/[Resources|AuthResources].java
>>
>> sun/security/util/Resources in jdk6, http://hg.openjdk.java.net/jdk6/jdk6/jdk/file/62df9772b849/src/share/classes/sun/security/util/Resources.java
>> sun/security/util/Resources in jdk7, http://hg.openjdk.java.net/jdk7/jdk7/jdk/file/9b8c96f96a0f/src/share/classes/sun/security/util/Resources.java
>> sun/security/util/Resources in jdk8, http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/sun/security/util/Resources.java
>>
>> Thank you
>> -Hamlin
>>>
>>> --Max
>>>
>>>> Thank you
>>>> -Hamlin
>>>>> Thanks
>>>>> Max
>>>>>
>>>>>> On Jul 12, 2017, at 10:28 AM, Hamlin Li <[hidden email]> wrote:
>>>>>>
>>>>>> Would you please review the below patch?
>>>>>>
>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8184165
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~mli/8184165/webrev.00/
>>>>>>
>>>>>>
>>>>>> Thank you
>>>>>>
>>>>>> -Hamlin
>>>>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: (10) RFR of JDK-8184165: sun.security.provider.PolicyFile$PolicyEntry.toString() throws MissingResourceException

Hamlin Li
I'm afraid it's not worth to build such a automatic tool.

Different resource clients uses different resource classes which have
different resource items, so such a tool must understand which resource
client is using which resource class, which is not easy or even
possible. Or we can hard code this relationship in the tool, but this
will make the tool prone to error in case any code changes.

So I suggest to check it manually every time we change any code in this
area, of course manual check can be partially automated by some script
but I don't think it's worth to develop a dedicated tool to do it as you
still need manual check any way.

Thank you

-Hamlin


On 2017/7/12 15:59, Weijun Wang wrote:

> Great work! Thanks for going deep on this issue.
>
> Is it possible to change your manual check into an automatic test? I know sources might not be available when running a test, but if something like ${test.src}/../../../src/java.base/share/classes/sun/security/util/ exists it can be a good hint that source codes are available.
>
> Feel free to ignore my suggestion if this is useless. I don’t actually know if the src directory is there in mach5 and JPRT tests.
>
> --Max
>
>> On Jul 12, 2017, at 2:07 PM, Hamlin Li <[hidden email]> wrote:
>>
>> I manually checked *Resources.java under sun/security and usage of them, my investigation shows that overall quality of "static" resource usage is good (by "static" I mean it uses hard code resource string like rb.getString("xxxx") rather than rb.getString(aStringVariable) ), except of this issue and another new issue: https://bugs.openjdk.java.net/browse/JDK-8184234, in which similar issue in sun.security.provider.AuthPolicyFile$PolicyEntry is reported.
>>
>> For new issue JDK-8184234, it existed since jdk6 to 10.
>>
>> Thank you
>>
>> -Hamlin
>>
>>
>> On 2017/7/12 11:36, Hamlin Li wrote:
>>>
>>> On 2017/7/12 11:06, Weijun Wang wrote:
>>>>> On Jul 12, 2017, at 11:03 AM, Hamlin Li <[hidden email]> wrote:
>>>>>
>>>>> Hi Max,
>>>>>
>>>>> On 2017/7/12 10:50, Weijun Wang wrote:
>>>>>> Change looks fine.
>>>>>>
>>>>>> Please remember to add a noreg-trivial label.
>>>>> Added the label, and will push the change.
>>>>>> Also, can you do some more investigation when this starts to happen? The bug says affected versions are 9 and 10 but PolicyFile.java has been there long long ago. Was there a regression?
>>>>> The same code is there since jdk6 (http://hg.openjdk.java.net/jdk6/jdk6/jdk) , I did not check jdk5...
>>>>> I don't think it's a regression, it should be just a missing resource, as the failure only occurs when accessing very details of sun.security.provider.PolicyFile by reflection, I guess people seldom do that.
>>>> Is it easy to verify?
>>>>
>>>> I asked if it’s a regression because I remember some time last year there is some rearrangement of codes in this area.
>>> I just checked jdk6, 7, 8. You're right, there is a regression in jdk 8.
>>> In summary,
>>>   in jdk6, the issue exists;
>>>   in jdk7 some resources were added which I believe fixed this issue; (too many, I can not list it here.)
>>>   in jdk8 the added resources were totally removed, and another 2 resources are added:
>>>>          {"duplicate.keystore.domain.name","duplicate keystore domain name: {0}"},
>>>>          {"duplicate.keystore.name","duplicate keystore name: {0}"},
>>> But currently I don't know how big the change impacts, as there are many resource usage under sun/security which use sun/security/util/[Resources|AuthResources].java
>>>
>>> sun/security/util/Resources in jdk6, http://hg.openjdk.java.net/jdk6/jdk6/jdk/file/62df9772b849/src/share/classes/sun/security/util/Resources.java
>>> sun/security/util/Resources in jdk7, http://hg.openjdk.java.net/jdk7/jdk7/jdk/file/9b8c96f96a0f/src/share/classes/sun/security/util/Resources.java
>>> sun/security/util/Resources in jdk8, http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/sun/security/util/Resources.java
>>>
>>> Thank you
>>> -Hamlin
>>>> --Max
>>>>
>>>>> Thank you
>>>>> -Hamlin
>>>>>> Thanks
>>>>>> Max
>>>>>>
>>>>>>> On Jul 12, 2017, at 10:28 AM, Hamlin Li <[hidden email]> wrote:
>>>>>>>
>>>>>>> Would you please review the below patch?
>>>>>>>
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8184165
>>>>>>>
>>>>>>> webrev: http://cr.openjdk.java.net/~mli/8184165/webrev.00/
>>>>>>>
>>>>>>>
>>>>>>> Thank you
>>>>>>>
>>>>>>> -Hamlin
>>>>>>>

Loading...