RFR: 8319332: Security properties files inclusion [v17]

Martin Balao mbalao at openjdk.org
Tue Aug 6 23:21:36 UTC 2024


On Tue, 6 Aug 2024 21:02:50 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Francisco Ferrari Bihurriet has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Ensure Security::setProperty() cannot issue an include
>>    
>>    Co-authored-by: Martin Balao <mbalao at redhat.com>
>>    Co-authored-by: Francisco Ferrari Bihurriet <fferrari at redhat.com>
>>  - Simplify PropertyExpander::expandNonStrict()
>>    
>>    Co-authored-by: Martin Balao <mbalao at redhat.com>
>>    Co-authored-by: Francisco Ferrari Bihurriet <fferrari at redhat.com>
>
> src/java.base/share/classes/java/security/Security.java line 901:
> 
>> 899:         if (SecPropLoader.isInclude(key)) {
>> 900:             return;
>> 901:         }
> 
> Don't you want to throw an exception here?

May be. This is a public API that only documents `SecurityException` for cases in which there is a Security Manager and `NullPointerException` for cases in which either the key or the value are `null`. Wouldn't be the exact case here, unless we stretch it a bit and document a new type of unchecked exception. I was thinking of `IllegalArgumentException`. What do you think?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16483#discussion_r1706206008



More information about the security-dev mailing list