On Tue, 6 Jan 2026 17:23:16 GMT, Francisco Ferrari Bihurriet <[email protected]> wrote:
>> Hi, this is a proposal to fix >> [JDK-8352728](https://bugs.openjdk.org/browse/JDK-8352728 "InternalError >> loading java.security due to Windows parent folder permissions"). >> >> Path resolution with `Path::toRealPath` fails under the following conditions: >> >> * When there is a restricted >> [_ACL_](https://learn.microsoft.com/en-us/windows/win32/secauthz/access-control-lists) >> in a parent directory (_Windows_) >> * When dealing with an anonymous file only accesible through the _procfs_, >> such as `/proc/<pid>/fd/<fd>` (_Linux_) >> * Such a file can be created by a pipe, deleting an opened file, or with >> the [`memfd_create` _Linux_ >> API](https://man7.org/linux/man-pages/man2/memfd_create.2.html) >> >> Original code from [JDK-8319332: Security properties files >> inclusion](https://bugs.openjdk.org/browse/JDK-8319332) unconditionally >> resolves with `Path::toRealPath` all the processed properties files. This PR >> avoids resolving any paths. Cyclic includes detection is now performed with >> the unresolved paths and `Files::isSameFile`, so `activePaths` no longer >> needs to be a `Set`. >> >> <details> >> <summary>Previous approach and rationale for resolving <strong>file</strong> >> symlinks (kept here for historical reasons).</summary><br> >> >> In _Linux_, a relative `include` from an anonymous file referenced as >> `/proc/<pid>/fd/<fd>` makes little sense. >> >> However, in _Windows_, a relative `include` from a file where any of the >> parent directories has a restricted _ACL_ will be expected to work. It's >> important to note that such a restricted directory permissions scenario >> occurs to every >> [_UWP_](https://learn.microsoft.com/en-us/windows/uwp/get-started/universal-application-platform-guide) >> app (see [JDK-8369741](https://bugs.openjdk.org/browse/JDK-8369741 "cannot >> use java.security inside of UWP apps")). >> >> When computing a relative `include`, we were performing symlinks resolution >> of the parent file under the rationale that the person writing the original >> properties file is the one who decides where the relative includes should >> resolve. This reasoning has been nuanced and re-evaluated in a [subsequent >> discussion](#discussion_r2585623241). >> >> The original idea was to replace >> [`java.nio.file.Path::toRealPath`](https://docs.oracle.com/en/java/javase/24/docs/api/java.base/java/nio/file/Path.html#toRealPath(java.nio.file.LinkOption...)) >> by >> [`java.io.File::getCanonicalPath`](https://docs.oracle.com/en/java/javase/24/docs/api/java.base/java/io/File.html#getCanonicalPath()) >> for path ca... > > Francisco Ferrari Bihurriet has updated the pull request with a new target > base due to a merge or a rebase. The incremental webrev excludes the > unrelated changes brought in by the merge/rebase. The pull request contains > 31 additional commits since the last revision: > > - Merge openjdk:master into franferrax:JDK-8352728 > - Update copyright for the New Year > - Merge openjdk:master into franferrax:JDK-8352728 > - Remove superseded SecurityPropFile > - Move tests into SecurityPropFile and rename them > - Merge openjdk:master into franferrax:JDK-8352728 > - 3/3) Adjust ConfigFileTest for unresolved paths > - 2/3) Refactor ConfigFileTest ExtraMode enum > > Move ExtraMode as an ExtraPropsFile field, so we have this information > as soon as the ExtraPropsFile is created. This will be useful for the > next change. > - 1/3) Revert ConfigFileTest adjustment > > This reverts commit 7c80874c25bc99783ad24fb22d2c080d33c5503a only for > ConfigFileTest. > - Do not resolve symlinks for relative includes > > As @wangweij pointed out: > > The person writing the original properties file may have expected > > includes to resolve relative to its own location, but whoever created > > the symlink may have intended a different resolution path. If they > > wanted the original location, they could have just used the real file > > directly instead of introducing a symlink. > > Since path resolution is causing trouble under certain conditions, let's > avoid doing it. Other programs with include directives support in their > configuration files are doing the same. > - ... and 21 more: https://git.openjdk.org/jdk/compare/0c2e850c...d94feb26 Marked as reviewed by mullan (Reviewer). I have reviewed much of it, so I will also add my name as Reviewer. ------------- PR Review: https://git.openjdk.org/jdk/pull/24465#pullrequestreview-3639738923 PR Comment: https://git.openjdk.org/jdk/pull/24465#issuecomment-3724184162
