On Fri, 17 Oct 2025 10:07:37 GMT, Alan Bateman <[email protected]> wrote:
>>> @AlanBateman are you ok with letting the original >>> [c6f1d5f](https://github.com/openjdk/jdk/commit/c6f1d5f374bfa9bde75765391d5dae0e8e28b4ab) >>> reviewers know of this fix and take a look? Or do you think further >>> discussion is needed somewhere else? >> >> Have you had time to try using the file key to detect the recursive include >> case? > >> Hi @AlanBateman, I'm resuming the work on this one. >> >> > Have you had time to try using the file key to detect the recursive >> > include case? >> >> By "file key", do you mean `Files.readAttributes(Path.of("..."), >> BasicFileAttributes.class).fileKey()`? This works on _Linux_ but returns >> `null` on _Windows_. > > Can you try Files.isSameFile? It will use the file key on all platforms. If > the file located by the included path, and the previous files encountered, > are accessible then it can be loadFromPath.. @AlanBateman: after building and retesting the last merge (b5063f0040187aa21cd2d320623e593db611a058), `ConfigFileTestDirPermissions.java` started failing, and I realized this is due to #27324, which restored sone pre JDK 24 behaviour. I didn't know `File::getCanonicalPath` was behaving different across JDK versions. With the following directories structure: 📁 %tmp%/ ├─📁 dir1/ │ └─🔗 link/ (directory junction ⟹ 📁 target/) └─📁 target/ └─📄 file1 * JDK 21's `File::getCanonicalPath` resolves `dir1\link\file1` as `%tmp%\dir1\link\file1` * JDK 25's `File::getCanonicalPath` resolves `dir1\link\file1` as `%tmp%\target\file1` As a consequence I'm going back to `Path::toRealPath`, which, with this PR changes, is better than the current status quo (as mentioned in my previous comment). Hopefully this addresses all your concerns, but let me know if there is more. However, independently from this PR work, you might want to check whether #27324 was expected to change that behaviour. Please note that `dir1\link\file1` is readable, and there isn't any network path involved, nor any parent directory permission issues here. Even more, `Path::toRealPath` resolves the directory junction in this case. Here it is a minimal _Windows_ CMD/Batch reproducer (current mainline behaves as JDK 21): PUSHD %tmp% MKDIR target ECHO content >target\file1 MKDIR dir1 MKLINK /J dir1\link target :: Output: :: Junction created for dir1\link <<===>> target :: Test code with JDKs 21 and 25 SET code=System.out.println("File::getCanonicalPath: " + new File("dir1\\link\\file1").getCanonicalPath()) SET code=%code%; System.out.println("Path::toRealPath: " + Path.of("dir1\\link\\file1").toRealPath()) ECHO %code% | C:\Users\test\programs\jdks\21\bin\jshell.exe - :: Output: :: File::getCanonicalPath: C:\Users\test\AppData\Local\Temp\dir1\link\file1 :: Path::toRealPath: C:\Users\test\AppData\Local\Temp\target\file1 ECHO %code% | C:\Users\test\programs\jdks\25\bin\jshell.exe - :: Output: :: File::getCanonicalPath: C:\Users\test\AppData\Local\Temp\target\file1 :: Path::toRealPath: C:\Users\test\AppData\Local\Temp\target\file1 :: Cleanup RD /S /Q target dir1 POPD ------------- PR Comment: https://git.openjdk.org/jdk/pull/24465#issuecomment-3582665183
