RFR: 8352728: InternalError loading java.security due to Windows parent folder permissions [v8]

[Sean Mullan]

On Thu, 30 Oct 2025 23:07:23 GMT, Francisco Ferrari Bihurriet <fferrari at openjdk.org> wrote:

>> Hi, this is a proposal to fix 8352728.
>> 
>> The main idea is 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 canonicalization purposes. The rationale behind this decision is the following:
>> 
>> 1. In _Windows_, `File::getCanonicalPath` handles restricted permissions in parent directories. Contrarily, `Path::toRealPath` fails with `AccessDeniedException`.
>> 2. In _Linux_, `File::getCanonicalPath` handles non-regular files (e.g. `/dev/stdin`). Contrarily, `Path::toRealPath` fails with `NoSuchFileException`.
>> 
>> #### Windows Case
>> 
>> @martinuy and I tracked down the `File::getCanonicalPath` vs `Path::toRealPath` behaviour differences in _Windows_. Both methods end up calling the [`FindFirstFileW`](https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-findfirstfilew) API inside a loop for each parent directory in the path, until they include the leaf:
>> 
>> * [`File::getCanonicalPath`](https://github.com/openjdk/jdk/blob/jdk-24-ga/src/java.base/share/classes/java/io/File.java#L618 "src/java.base/share/classes/java/io/File.java:618") goes through the following stack into `FindFirstFileW`:
>>     * [`WinNTFileSystem::canonicalize`](https://github.com/openjdk/jdk/blob/jdk-24-ga/src/java.base/windows/classes/java/io/WinNTFileSystem.java#L473 "src/java.base/windows/classes/java/io/WinNTFileSystem.java:473")
>>     * [`WinNTFileSystem::canonicalize0`](https://github.com/openjdk/jdk/blob/jdk-24-ga/src/java.base/windows/native/libjava/WinNTFileSystem_md.c#L288 "src/java.base/windows/native/libjava/WinNTFileSystem_md.c:288")
>>     * [`wcanonicalize`](https://github.com/openjdk/jdk/blob/jdk-24-ga/src/java.base/windows/native/libjava/canonicalize_md.c#L233 "src/java.base/windows/native/libjava/canonicalize_md.c:233") (here is the loop)
>>     * If `FindFirstFileW` fails with `ERROR_ACCESS_DENIED`, `lastErrorReportable` is consulted, the error is [considered non-reportable](https://github.com/openjdk/jdk/blob/jdk-24-ga/src/java.base/windows/native/libjava/canonicalize_md.c#L139 "src/java.base/windows/native/libjava/canonicalize_md.c:139") and the iteration is stopped [here](https://github.com/openjdk/jdk/blob/jdk-24-ga/src/java.base/windows/native/libjava/canonicalize_md.c#L246-L250 "src/ja...
>
> Francisco Ferrari Bihurriet has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Detect cyclic includes with Files::isSameFile
>   
>   checkCyclicInclude() is invoked after we successfully get an InputStream
>   for the path to avoid skipping the same IOException several times inside
>   checkCyclicInclude() if the path doesn't exist.
>   
>   Also, perform symlinks resolution only in the following cases:
>    • When we need to resolve a relative include
>    • For clarity to the user in logging messages
>    • For clarity to the user in exception messages
>   
>   In the first case, the resolution is a requirement, in the last two
>   cases it is a nice-to-have. But given the last two are exceptional
>   cases anyway, we let any resolution error bubble up.

src/java.base/share/classes/java/security/Security.java line 286:

> 284:                     if (Files.isSameFile(path, activePath)) {
> 285:                         throw new InternalError(
> 286:                                 "Cyclic include of '" + resolve(path) + "'");

Why try to resolve the path for an exception message? If that causes an exception an `InternalError` will be thrown and this error message will be lost, making it harder to debug.

src/java.base/share/classes/java/security/Security.java line 288:

> 286:                                 "Cyclic include of '" + resolve(path) + "'");
> 287:                     }
> 288:                 } catch (IOException ignore) {}

Not sure you want to ignore this - seems better to let this propagate and be thrown as an `InternalError`.

src/java.base/share/classes/java/security/Security.java line 295:

> 293:                 throws IOException {
> 294:             boolean isRegularFile = Files.isRegularFile(path);
> 295:             if (!isRegularFile && Files.isDirectory(path)) {

Can a directory ever be a regular file? If not, you don't need the `!isRegularFile` check.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24465#discussion_r2504468308
PR Review Comment: https://git.openjdk.org/jdk/pull/24465#discussion_r2504448064
PR Review Comment: https://git.openjdk.org/jdk/pull/24465#discussion_r2504402772