> 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/java.base/windows/native/libjava/c...
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 nine
additional commits since the last revision:
- Merge openjdk:master into franferrax:JDK-8352728
- Introduce a shell test for anonymous pipes
This use case has been discussed and analyzed in the pull request, but
we didn't have a test case for it. By introducing a test, we make sure
we don't have regressions in this area, regardless of the alternative
we choose to advance with for this fix.
- Merge openjdk:master into franferrax:JDK-8352728
- Fix typo
- Merge openjdk:master into franferrax:JDK-8352728
- Reintroduce links test using directory junctions
Junctions do not require elevation, so this is a way of testing
soft-links are resolved without requiring elevation. This is useful
because we need to avoid elevation in order to reproduce the parent
directories permission issue.
This is testing directories structure:
📁 JDK-8352728-tmp-*/
├─🔒 jdk-parent-dir/ (ACL with REMOVED-PERMISSIONS)
│ └─📁 jdk/
│ ├─📁 conf/
│ │ ├─📁 security/
│ │ │ ├─📄 java.security
│ │ │ │ 📝 include link-to-other-dir/other.properties
│ │ │ ├─🔗 link-to-other-dir/ ⟹ 📁 JDK-8352728-tmp-*/other-dir
│ │ │ └─... (JUNCTION)
│ │ └─...
│ └─...
├─📁 other-dir/
│ └─📄 other.properties
│ 📝 include ../relatively.included.properties
└─📄 relatively.included.properties
📝 test.property.name=test_property_value
- Do minor adjustments
Update copyright year, improve comments and use File::toPath to convert
back to Path.
- Merge openjdk/master into JDK-8352728
- 8352728: InternalError loading java.security due to Windows parent folder
permissions
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/24465/files
- new: https://git.openjdk.org/jdk/pull/24465/files/71718e52..4e77fb36
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=24465&range=06
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=24465&range=05-06
Stats: 355945 lines in 5286 files changed: 245773 ins; 77995 del; 32177 mod
Patch: https://git.openjdk.org/jdk/pull/24465.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/24465/head:pull/24465
PR: https://git.openjdk.org/jdk/pull/24465
