On Fri, 16 Aug 2024 16:13:17 GMT, Weijun Wang <wei...@openjdk.org> wrote:

>> Francisco Ferrari Bihurriet has updated the pull request incrementally with 
>> one additional commit since the last revision:
>> 
>>   Code review and additional changes
>>   
>>   Throw an IllegalArgumentException exception if Security.getProperty() is
>>   invoked with "include" as key, also add a check in the test case.
>>   
>>   Initialize java.security.Security::props in a single place, and make it
>>   final.
>>   
>>   Make sun.security.util.PropertyExpander::expandNonStrict() throw
>>   AssertionError instead of RuntimeException.
>>   
>>   Remove "this file" reference in java.security.
>>   
>>   Additionally, use java.security.Security::check() to check the
>>   'getProperty.<key>' permission, as with 'setProperty.<key>'.
>>   
>>   Co-authored-by: Martin Balao <mba...@redhat.com>
>>   Co-authored-by: Francisco Ferrari Bihurriet <fferr...@redhat.com>
>
> I don't like the silent mode. If no one uses that key name, then everything 
> is fine anyway. Otherwise, if someone really sets it, it's very likely they 
> will want to read it somewhere and expect a non `null` value.
> 
> Can we just support it as a real security property? i.e. if it appears in 
> `java.security`, besides loading the content, we can also read it as a 
> security property. And then we also allow it in `setProperty` and 
> `getProperty`. I can see some behavior change:
> 
> 1. If it's in `java.security` and not point to a real file, there will be an 
> exception.
> 2. If it appears in an included file, the value will overwrite the previous 
> one.
> 
> For 1, I don't think you want to be silent about the missing file. For 2, we 
> can ignore it as a security property, but this becomes a little complex.

I'm ok with proceeding to integrate. @wangweij do you have any further comments 
or concerns?

If Weijun is ok with proceeding, you will need to finalize the CSR before you 
can integrate. You also need to write a release note, unless I missed that.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/16483#issuecomment-2332563047

Reply via email to