> 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 14 additional
commits since the last revision:
- Merge openjdk:master into franferrax:JDK-8352728
- Do symlinks resolution in a single place
Remove path normalization from debugging messages, only keep it where
strictly necessary.
Adjust test case to the new debugging messages which contain absolute
but not normalized paths (can contain some ../../ elements).
- Use the right OutputAnalyzer method
OutputAnalyzer::stderrMatches returns a boolean while
OutputAnalyzer::stderrShouldMatch performs the check.
- Remove path resolution from exception message
- 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.
- 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
- ... and 4 more: https://git.openjdk.org/jdk/compare/996ef562...b5063f00
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/24465/files
- new: https://git.openjdk.org/jdk/pull/24465/files/4483469e..b5063f00
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=24465&range=10
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=24465&range=09-10
Stats: 268925 lines in 2438 files changed: 180885 ins; 48795 del; 39245 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
