From 05cdd6b4e2db56c25fbc65d50c512235d0cbfb49 Mon Sep 17 00:00:00 2001 From: Ruslan Cheremin Date: Wed, 24 Sep 2025 16:30:08 +0200 Subject: [PATCH] [vfs][tests] improve symlink processing GitOrigin-RevId: 225401b1d32eee1cb6ea23a9073e78e101d08d86 --- .../newvfs/impl/VirtualFileSystemEntry.java | 11 +++- .../impl/PersistentFS_FindFilesTest.java | 60 ++++++++++++++++--- 2 files changed, 61 insertions(+), 10 deletions(-) diff --git a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/impl/VirtualFileSystemEntry.java b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/impl/VirtualFileSystemEntry.java index 4a3a6e8a4443..dc8af77b24b2 100644 --- a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/impl/VirtualFileSystemEntry.java +++ b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/impl/VirtualFileSystemEntry.java @@ -760,7 +760,16 @@ public abstract class VirtualFileSystemEntry extends NewVirtualFile { } /** - * @return true, if this file is a symlink or there is a symlink parent + * BEWARE: This holds only while inside the same file-system, but breaks on file-system borders. + * I.e. lets' take a [jar:///local/path/file.jar!/path/inside/My.class] file: if [/local/path] is a + * symlink, then + * VirtualFile[file:///local/path/file.jar].thisOrParentHaveSymlink() == true + * but + * VirtualFile[ jar:///local/path/file.jar!/path/inside/My.class].thisOrParentHaveSymlink() == false + * because for VFS VirtualFile[jar:///local/path/file.jar!/path/inside/My.class] belongs to + * [jar:///local/path/file.jar!/] root, and up to this root there are no symlinks. + * I don't know is it 'intended' behavior, or just an omission though + * @return true, if this file is a symlink or there is a symlink parent, up to VFS root (not file-system root!) */ @ApiStatus.Internal public boolean thisOrParentHaveSymlink() { diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/impl/PersistentFS_FindFilesTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/impl/PersistentFS_FindFilesTest.java index 5f9109c426df..b454327792da 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/impl/PersistentFS_FindFilesTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/newvfs/impl/PersistentFS_FindFilesTest.java @@ -206,22 +206,46 @@ public class PersistentFS_FindFilesTest { return; } + //Quite a lot of trickiness in this test relates to symlink processing. + //POSIX path resolution rules (which we sort-of follow in .findFileByPath) demands symlinks resolution as soon, as + // they are met in the path -- which means '/a/../a/' is NOT always equivalent to just '/a/'. + // Namely, if '/a' is a symlink to -> '/b/c/d' then POSIX (and .findFileByPath) resolves '/a/../a/' to the '/b/c/a', + // instead of just '/a/' (= '/b/c/d'). + // So, we better skip paths with symlink from this test -- _before_ checking [/a/] -> [/a/../a/] equivalent transformation, + // we ensure no symlinks in the path -- when [/a/] <=> [/a/../a/] equivalency is correct. + // + // Surprisingly, determine is there a symlink in the path happens to be untrivial too. + // It seems like we have VirtualFileSystemEntry.thisOrParentHaveSymlink() method exactly for that -- but it is only + // partially correct, since in VFS .thisOrParentHaveSymlink() works only up to _VFS root_, which is different from + // file-path root. + // The most obvious example is jar-paths. Lets' take [jar:///local/path/file.jar!/path/inside/jar.class] file: if + // [/local/path] is a symlink, then + // VirtualFile[file:///local/path/file.jar].thisOrParentHaveSymlink() == true + // but + // VirtualFile[ jar:///local/path/file.jar!/path/inside/jar.class].thisOrParentHaveSymlink() == false, + // because for VFS VirtualFile[jar:///local/path/file.jar/!/path/inside/jar.class] belongs to the + // [jar:///local/path/file.jar!/] root, and up to this root there are no symlinks. But then VFS resolves the + // [jar:///local/path/file.jar!/path/inside/jar.class] (and it's non-canonical form, produced in this test) it + // navigates through 'file:///local/path/file.jar' parts first, and in these parts there _is_ a symlink, which + // ruins the invariant checked by this test. + //So, the 'no-symlink' check is split into .thisOrParentHaveSymlink() fast-path, and .hasSymlinkInThePath() slow-paths + if (((VirtualFileSystemEntry)file).thisOrParentHaveSymlink()) { - //POSIX path resolution rules (which we sort-of follow in .findFileByPath) demands symlinks resolution as soon, as - // they are met in the path -- which means '/a/../a/' is NOT always equivalent to just '/a/'. - // Namely, if '/a' is a symlink to -> '/b/c/d' then POSIX (and .findFileByPath) resolves '/a/../a/' to the '/b/c/a', - // instead of just '/a/' (= '/b/c/d'). - // So _before_ checking [/a/] -> [/a/../a/] equivalent transformation, we ensure no symlinks in the path -- when - // [/a/] <=> [/a/../a/] equivalency is correct return; } String path = file.getPath(); - String nonCanonicalPath = path.replaceAll("([\\w+\\-.@]+)/", "$1/../$1/");// [/a/] -> [/a/../a/] + String nonCanonicalPath = path.replaceAll("([\\w+\\-.@\\s]+)/", "$1/../$1/");// [/a/] -> [/a/../a/] + NewVirtualFile fileFoundByNonCanonicalPath = VfsImplUtil.findFileByPath(fileSystem, nonCanonicalPath); - assertEquals(file, fileFoundByNonCanonicalPath, - () -> "[" + nonCanonicalPath + "] must be resolved to it's original [" + path + "]"); + if (!file.equals(fileFoundByNonCanonicalPath)) { + if (!hasSymlinkInThePath(path)) { + fail("[" + nonCanonicalPath + "] must be resolved to it's original [" + path + "]:\n" + + file + " original file\n" + + fileFoundByNonCanonicalPath + " found by non-original file"); + } + } }); } @@ -332,6 +356,24 @@ public class PersistentFS_FindFilesTest { } } + private static boolean hasSymlinkInThePath(@NotNull String path) { + try { + Path nioPath = Path.of(path); + do { + boolean isSymlink = Files.isSymbolicLink(nioPath); + if (isSymlink) { + return true; + } + nioPath = nioPath.getParent(); + } + while (nioPath != null); + return false; + } + catch (Exception e) { + return false; + } + } + /** * @return true if the file root is 'loadable' -- i.e. it has it's FileSystem known to VFS, and other required staff is OK. * If the root can't be loaded -- all files under it also invalid for VFS