On Mon, 22 Apr 2024 20:42:44 GMT, Francisco Ferrari Bihurriet <fferr...@openjdk.org> wrote:
>> The implementation of this proposal is based on the requirements, >> specification and design choices described in the [JDK-8319332] ticket and >> its respective CSR [JDK-8319333]. What follows are implementation notes >> organized per functional component, with the purpose of assisting to >> navigate the code changes in this pull-request. >> >> ## Security properties loading (overview) >> >> A new static class named `SecPropLoader` (nested within >> `java.security.Security`) is introduced to handle the loading of all >> security properties. Its method `loadAll` is the first one to be called, at >> `java.security.Security` static class initialization. The master security >> properties file is then loaded by `loadMaster`. When additional security >> properties files are allowed (the security property >> `security.overridePropertiesFile` is set to `true`) and the >> `java.security.properties` system property is passed, the method `loadExtra` >> handles the extra load. >> >> The master properties file is loaded in `OVERRIDE` mode, meaning that the >> map of properties is originally empty. Any failure occurred while loading >> these properties is considered fatal. The extra properties file >> (`java.security.properties`) may be loaded in `OVERRIDE` or `APPEND` mode. >> Any failure in this case is ignored. This behavior maintains compatibility >> with the previous implementation. >> >> While the `java.security.properties` system property is documented to accept >> an URL type of value, filesystem path values are supported in the same way >> that they were prior to this enhancement. Values are then interpreted as >> paths and, only if that fails, are considered URLs. In the latter case, >> there is one more attempt after opening the stream to check if there is a >> local file path underneath (e.g. the URL has the form of >> `file:///path/to/a/local/file`). The reason for preferring paths over URLs >> is to support relative path file inclusion in properties files. >> >> ## Loading security properties from paths (`loadFromPath` method) >> >> When loading a properties file from a path, the normalized file location is >> stored in the static field `currentPath`. This value is the current base to >> resolve any relative path encountered while handling an _include_ >> definition. Normalized paths are also saved in the `activePaths` set to >> detect recursive cycles. As we move down or up in the _includes_ stack, >> `currentPath` and `activePaths` values are updated. >> >> ## Loading security properties from URLs (`loadFromUrl` method) >> >> The extra properti... > > 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 13 commits: > > - Extend backward compatibility support > > Co-authored-by: Francisco Ferrari <fferr...@redhat.com> > Co-authored-by: Martin Balao <mba...@redhat.com> > - Merge 'openjdk/master' into JDK-8319332 > - 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 <mba...@redhat.com> > Co-authored-by: Francisco Ferrari Bihurriet <fferr...@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 <mba...@redhat.com> > Co-authored-by: Francisco Ferrari Bihurriet <fferr...@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.secu... Thanks. I really appreciate it. I’m on vacation this week and will read more when I’m back. On Apr 22, 2024, at 18:34, Martin Balao Alonso ***@***.***> wrote: Hi @wangweij<https://urldefense.com/v3/__https://github.com/wangweij__;!!ACWV5N9M2RV99hQ!PLfmz4Pj-Wb17DCRZva4wDWu3nWdbLz1skiojB-1WAGnJ8PgdU8Eg_Xv53oGhCOT8qu_cVpRYLGi0TuC61ayzndAFSo$> , 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. — Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/16483*issuecomment-2071248623__;Iw!!ACWV5N9M2RV99hQ!PLfmz4Pj-Wb17DCRZva4wDWu3nWdbLz1skiojB-1WAGnJ8PgdU8Eg_Xv53oGhCOT8qu_cVpRYLGi0TuC61ay3fAX3fk$>, or unsubscribe<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AAEMEPJZRKH3I43STJDLVV3Y6W3AHAVCNFSM6AAAAAA63MC2XCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZRGI2DQNRSGM__;!!ACWV5N9M2RV99hQ!PLfmz4Pj-Wb17DCRZva4wDWu3nWdbLz1skiojB-1WAGnJ8PgdU8Eg_Xv53oGhCOT8qu_cVpRYLGi0TuC61ayh5LNOdE$>. You are receiving this because you were mentioned.Message ID: ***@***.***> ------------- PR Comment: https://git.openjdk.org/jdk/pull/16483#issuecomment-2071258992
