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

Thu Apr 10 13:09:32 UTC 2025

On Thu, 10 Apr 2025 05:52:17 GMT, Alan Bateman <alanb 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...
>
> I don't think this change should be integrated before more investigation. I think start by finding out why this code is using toRealPath. For the two cases listed, it looks like toRealPath is correctly failing for the first, but for /dev/stdin then please bring it to nio-dev to discuss how special devices should be handled by that method.

Hi @AlanBateman.

> I don't think this change should be integrated before more investigation.

Ok, makes sense.

> I think start by finding out why this code is using toRealPath.

The usage of `Path::toRealPath` was introduced by the [8319332: Security properties files inclusion](https://bugs.openjdk.org/browse/JDK-8319332) proposal for the following reasons:

1. Weak reason: detect cyclic re-inclusion through an alias of the same file (e.g. symlink, or alternative case in case-insensitive filesystems). This would only make cycle detection trigger earlier, but is not strictly necessary (infinite recursion will lead to path repetition even if not normalized).
2. Weak reason: resolve a relative path passed through `-Djava.security.properties=relative.props` against the current working directory (stack: `loadAll`, `loadExtra`, `loadExtraHelper`, `loadExtraFromPath`, `loadFromPath`). This resolution could be done with `Path::toAbsolutePath`, but this case is also subject to the 3ʳᵈ reason.
3. Strong reason: resolve symlinks, so that properties files use their original path to resolve relative includes. The rationale behind this is that the writer of the original properties file is the one who reasoned where relative includes should resolve to. On the other hand, the writer of the symlink just wants to use the original file with all its includes, without having to replicate anything else. This case is exercised by the _PowerShell_ test on this PR's description.

> For the two cases listed, it looks like toRealPath is correctly failing for the first […]

Yes, that was our impression, and that's why we are not proposing any fix to `Path::toRealPath`: there's nothing wrong with failing if normalization is not complete. But `File::getCanonicalPath` takes a best-effort approach that is more suitable to our needs.

> […] but for /dev/stdin then please bring it to nio-dev to discuss how special devices should be handled by that method.

I will investigate the _Linux_ case, I had skipped it because `File::getCanonicalPath` looked like the only alternative on _Windows_, while it is also working on _Linux_.

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

PR Comment: https://git.openjdk.org/jdk/pull/24465#issuecomment-2792757981