RFR 8168075: Custom system class loader + security manager + malformed policy file = recursive initialization

[Adam Petcher]

Thanks Sean and Mandy for the feedback. I believe I incorporated all 
feedback into the latest patch, with one exception (Sean see below).

http://cr.openjdk.java.net/~apetcher/8168075/webrev.02/

On 1/23/2017 12:15 PM, Sean Mullan wrote:
> On 1/19/17 10:28 AM, Adam Petcher wrote:
>> My last attempt to solve this problem didn't work because some classes
>> needed for string formatting were not loaded by init level 3 in some
>> cases. So I had to backtrack and try a different approach.
>>
>> This patch avoids localization and message formatting when the VM is not
>> booted. In this case, non-localized messages are printed, and simplified
>> message formatting code is used. Once the VM is loaded, messages are
>> localized and formatted in the usual way.
>>
>> http://cr.openjdk.java.net/~apetcher/8168075/webrev.01/
>
> Looks good, just a couple of comments:
>
> - PolicyUtil.getLocalizedMessage
>
> Don't think you need this method, since 
> LocalizedMessage.getLocalizedString is public.

Removed. I also renamed LocalizedMessage.getLocalizedString to 
"getMessage" to remove some redundancy and make the expressions that 
reference it shorter.

>
> - LocalizedMessage.java
>
> Not sure I see the need for the constructor or toLocalizedString 
> method, as I think you can just call getLocalizedString, ex:
>
>     LocalizedMessage localizedMsg = new LocalizedMessage
>         ("alias.name.not.provided.pe.name.");
>     Object[] source = {pe.name};
>     throw new Exception(localizedMsg.toLocalizedString(source));
>
> becomes:
>
>     throw new 
> Exception(LocalizedMessage.getLocalizedString("alias.name.not.provided.pe.name.", 
> source));

In PolicyParser.ParsingException, there is a comment stating that the 
MessageFormat is stored in the exception, and it is only formatted when 
getLocalizedMessage() is called on the exception. This arrangement 
avoids unnecessary formatting and permission checks in cases where the 
message is never localized and formatted. I kept this arrangement after 
I replaced MessageFormat with LocalizedMessage. If there is no value to 
delaying/avoiding the message formatting, then I can remove the 
constructor and non-static method as you suggest.

Here are the changes I made based on this feedback:
1) I changed the name of LocalizedMessage.toLocalizedString to "format" 
to make it more clear that I was trying to mirror the behavior of 
MessageFormat (and also to shorten the name and remove redundancy).
2) I added a comment to the constructor describing how it can be used to 
delay/avoid message formatting.
3) I changed all references to LocalizedMessage that don't involve 
ParsingException to use only the static method of LocalizedMessage, as 
you suggested.

>
> (saves creating an extra object).
>
> - MessageFormatting.java
>
> Minor nit: please use "java.security.policy==error.policy" instead of 
> "policy=error.policy" The java.security.policy is a newer jtreg option 
> that matches the syntax of the java.security.policy option. I'd like 
> to discourage use of the policy option going forward.
>
> Thanks,
> Sean
>
>
>>
>>
>> On 1/11/2017 8:34 AM, Adam Petcher wrote:
>>> Please review the following bug fix:
>>>
>>> http://cr.openjdk.java.net/~apetcher/8168075/webrev.00/
>>>
>>> This fixes a bug in which a permission check would try to load
>>> resources while the system class loader is being initialized.
>>> Resources cannot be loaded at this time, so this change ensures that
>>> the resources are loaded earlier.
>>>
>>