RFR: 8319332: Security properties files inclusion [v7]

[Martin Balao]

On Fri, 19 Apr 2024 19:40:53 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Francisco Ferrari Bihurriet has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 11 commits:
>> 
>>  - Merge 'openjdk/master' into JDK-8319332
>>  - Merge 'openjdk/master' into JDK-8319332
>>    
>>    Conflict in ConfigFileTest.java solved by keeping our file, which had
>>    been previously adjusted.
>>    
>>    Commands:
>>      git merge upstream/master
>>      git restore --ours -- test/jdk/java/security/Security/ConfigFileTest.java
>>      git add test/jdk/java/security/Security/ConfigFileTest.java
>>      git merge --continue
>>  - 8319332: Adjust code for JDK-8319673 changes
>>    
>>    JDK-8319673: Few security tests ignore VM flags
>>    
>>    Next, we will merge the openjdk/master branch and ignore the conflict in
>>    this file.
>>    
>>    Co-authored-by: Martin Balao <mbalao at redhat.com>
>>    Co-authored-by: Francisco Ferrari Bihurriet <fferrari at redhat.com>
>>  - 8319332: Update copyright and ConfigFileTest.java.
>>    
>>    Bump copyright year to 2024 in all the modified files.
>>    
>>    Remove leaked host name from children JVMs debug command.
>>    
>>    Extract Executor::addSystemPropertiesAsJvmArgs from Executor::execute
>>    and rename 'allJvmArgs' to 'command'. Also split class name and
>>    RUNNER_ARG addition to 'command' as two separated command.add() calls.
>>    
>>    Co-authored-by: Martin Balao <mbalao at redhat.com>
>>    Co-authored-by: Francisco Ferrari Bihurriet <fferrari at redhat.com>
>>  - Merge 'openjdk/master' into JDK-8319332
>>  - 8319332: Fix corner-case regression with bash pipe
>>    
>>    Extra properties files provided through bash pipes used to work before
>>    this enhancement, restore their behaviour.
>>    
>>    Also take advantage to use Files::isRegularFile, Files::isDirectory and
>>    Files::exists APIs instead of converting from Path to File.
>>    
>>    Linux reproducers (sub-shell, stdin, and combination of both):
>>    
>>    java -XshowSettings:security:properties                      \
>>         -Djava.security.properties==<(echo name=value)          \
>>         -Djava.security.debug=properties -version
>>    
>>    echo name=value | java -XshowSettings:security:properties    \
>>         -Djava.security.properties==/dev/stdin                  \
>>         -Djava.security.debug=properties -version
>>    
>>    echo name=value | java -XshowSettings:security:properties    \
>>         -Djava.security.properties==<(echo include /dev/stdin)  \
>>         -Djava.security.debug=properties...
>
> Yes, I can see its effect on the relative include. Maybe use the deprecated URL constructor as a fallback when the other mechanisms fail?

Hi @wangweij ,

We have pushed a change to support malformed URLs as discussed before. We introduced changes to the ConfigFileTest test so the backward-compatible scenarios are asserted. This has been tested on both Windows and Linux. In summary, our tests show no regression compared to the previous `java.security.properties` behavior: `file:` works, `file:/` works and `file:///` works. `file://` does not work, because it tries to establish an FTP connection to a host with the empty string hostname. Notice that the latter behavior comes from `java.net.URL::openStream` and was there before.

We have also introduced the following changes to the [CSR](https://bugs.openjdk.org/browse/JDK-8319333):

1) Removed `file:///` note from _Compatibility Risk Description_

2) Added the discussion about empty string expansion of non-existent system properties in include paths. See section _Solution_ and subsections _Syntax_ and _Examples_ of _Specification_.

Looking forward to your thoughts.

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

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