RFR: 8319332: Security properties files inclusion [v18]

[Francisco Ferrari Bihurriet]

On Wed, 7 Aug 2024 19:13:32 GMT, Martin Balao <mbalao at openjdk.org> wrote:

>> src/java.base/share/classes/java/security/Security.java line 211:
>> 
>>> 209:         }
>>> 210: 
>>> 211:         private static void reset(LoadingMode mode) {
>> 
>> The method here looks like there is a chance that `props` does not get assigned. I know when the main file is loaded the mode is OVERRIDE, so this will not actually happen. Do you want to add a comment on this?
>> 
>> Or, maybe we can assign `props` at the beginning and use APPEND mode when loading the main file?
>
> Ok, we will change this to initialize `props` statically and then, if needed because of an OVERRIDE, do a `clear` of the properties map.

Done in 580d34a63b5d3b6c7d2323413f338527db2d9acd.

>> src/java.base/share/classes/sun/security/util/PropertyExpander.java line 72:
>> 
>>> 70:         } catch (ExpandException e) {
>>> 71:             // should not happen
>>> 72:             throw new RuntimeException("unexpected expansion error: when " +
>> 
>> Is an `AssertionError` better here?
>
> Yes, we can use an `AssertionError` here

Done in 580d34a63b5d3b6c7d2323413f338527db2d9acd.

>> src/java.base/share/conf/security/java.security line 33:
>> 
>>> 31: #
>>> 32: # The special "include" property can be defined one or multiple times in
>>> 33: # this file with a filesystem path value. The effect of each definition
>> 
>> "this file". Is this precise? As described below, It can also appear in included files and the file "java.security.properties" points to.
>> 
>> Maybe not an issue since no one will be confused. You decide whether to make any update.
>
> I don't see "this file" as strictly necessary given the context so we can remove it.

Done in 580d34a63b5d3b6c7d2323413f338527db2d9acd.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16483#discussion_r1707804517
PR Review Comment: https://git.openjdk.org/jdk/pull/16483#discussion_r1707807676
PR Review Comment: https://git.openjdk.org/jdk/pull/16483#discussion_r1707807978