On Thu, 2 May 2024 16:45:09 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 incrementally with > one additional commit since the last revision: > > Profiles documentation adjustments. > > Co-authored-by: Francisco Ferrari <fferr...@redhat.com> > Co-authored-by: Martin Balao <mba...@redhat.com> I still suggest you include all the new lines in `java.security` in the specification section of the the CSR in **raw code mode**. I know most of the content is already there but The CSR reviewers might care about the exact wording. src/java.base/share/conf/security/java.security line 35: > 33: # this file with a filesystem path value. The effect of each definition > 34: # is to include a referred security properties file inline, adding all > 35: # its properties and values. Included files, as well as files pointed by Not sure if "properties and values" are precise. A "property" already contains its "key" and "value", right? src/java.base/share/conf/security/java.security line 69: > 67: # statement will override any matching property defined in a profile. In > order > 68: # to avoid this behavior, the include statement may be placed at the end > of > 69: # this file. It's probably better to place the newly added "Security properties defined after an include statement..." in an individual paragraph since it's not limited to "the latter case" (or you mean "last"?). Then you might not want to use the "profile" word since it's not defined elsewhere. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16483#issuecomment-2091305451 PR Review Comment: https://git.openjdk.org/jdk/pull/16483#discussion_r1588209026 PR Review Comment: https://git.openjdk.org/jdk/pull/16483#discussion_r1588197310
