Fix: Make FileUtil.relativize symlink-aware

CHANGELOG.md

@@ -27,6 +27,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 - We added an initial [cite as you write](https://retorque.re/zotero-better-bibtex/citing/cayw/) endpoint. [#13187](https://github.com/JabRef/jabref/issues/13187)
 - We added "copy preview as markdown" feature. [#12552](https://github.com/JabRef/jabref/issues/12552)
 - In case no citation relation information can be fetched, we show the data providers reason. [#13549](https://github.com/JabRef/jabref/pull/13549)
+- When relativizing file names, symlinks are now taken into account. [#12995](https://github.com/JabRef/jabref/issues/12995)
 - We added a new button for shortening the DOI near the DOI field in the general tab when viewing an entry. [#13639](https://github.com/JabRef/jabref/issues/13639)
 
 ### Changed

jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java

@@ -144,7 +144,7 @@ public static Path addExtension(Path path, String extension) {
     /**
      * Looks for the unique directory, if any, different to the provided paths
      *
-     * @param paths List of paths as Strings
+     * @param paths       List of paths as Strings
      * @param comparePath The to be tested path
      */
     public static Optional<String> getUniquePathDirectory(List<String> paths, Path comparePath) {
@@ -153,15 +153,15 @@ public static Optional<String> getUniquePathDirectory(List<String> paths, Path c
         List<String> uniquePathParts = uniquePathSubstrings(paths);
         return uniquePathParts.stream()
                               .filter(part -> comparePath.toString().contains(part)
-                                              && !part.equals(fileName) && part.contains(File.separator))
+                                      && !part.equals(fileName) && part.contains(File.separator))
                               .findFirst()
                               .map(part -> part.substring(0, part.lastIndexOf(File.separator)));
     }
 
     /**
      * Looks for the shortest unique path of the in a list of paths
      *
-     * @param paths List of paths as Strings
+     * @param paths       List of paths as Strings
      * @param comparePath The to be shortened path
      */
     public static Optional<String> getUniquePathFragment(List<String> paths, Path comparePath) {
@@ -219,7 +219,7 @@ public static List<String> uniquePathSubstrings(List<String> paths) {
      * @param pathToSourceFile      Path Source file
      * @param pathToDestinationFile Path Destination file
      * @param replaceExisting       boolean Determines whether the copy goes on even if the file exists.
-     * @return boolean Whether the copy succeeded, or was stopped due to the file already existing.
+     * @return boolean Whether the copy succeeded or was stopped due to the file already existing.
      */
     public static boolean copyFile(Path pathToSourceFile, Path pathToDestinationFile, boolean replaceExisting) {
         // Check if the file already exists.
@@ -255,15 +255,39 @@ public static Path relativize(Path file, List<Path> directories) {
         if (!file.isAbsolute()) {
             return file;
         }
+        Optional<Path> realFileOpt = tryRealPath(file);
 
         for (Path directory : directories) {
             if (file.startsWith(directory)) {
                 return directory.relativize(file);
             }
+
+            if (realFileOpt.isPresent()) {
+                Optional<Path> realDirOpt = tryRealPath(directory);
+                if (realDirOpt.isPresent()) {
+                    Path realFile = realFileOpt.get();
+                    Path realDir = realDirOpt.get();
+                    if (realFile.startsWith(realDir)) {
+                        return realDir.relativize(realFile);
+                    }
+                }
+            }
         }
         return file;
     }
 
+    private static Optional<Path> tryRealPath(Path path) {
+        if (Files.exists(path)) {
+            try {
+                return Optional.of(path.toRealPath());
+            } catch (IOException e) {
+                return Optional.empty();
+            }
+        } else {
+            return Optional.of(path.toAbsolutePath());
+        }
+    }
+
     /**
      * Converts an absolute file to a relative one, if possible. Returns the parameter file itself if no shortening is
      * possible.
@@ -342,8 +366,8 @@ public static String createFileNameFromPattern(BibDatabase database, BibEntry en
     /**
      * Determines directory name provided by an entry in a database
      *
-     * @param database        the database, where the entry is located
-     * @param entry           the entry to which the directory should be linked to
+     * @param database             the database, where the entry is located
+     * @param entry                the entry to which the directory should be linked to
      * @param directoryNamePattern the dirname pattern
      * @return a suggested dirName
      */
@@ -370,9 +394,9 @@ public static String createDirNameFromPattern(BibDatabase database, BibEntry ent
     public static Optional<Path> findSingleFileRecursively(String filename, Path rootDirectory) {
         try (Stream<Path> pathStream = Files.walk(rootDirectory)) {
             return pathStream
-                             .filter(Files::isRegularFile)
-                             .filter(f -> f.getFileName().toString().equals(filename))
-                             .findFirst();
+                    .filter(Files::isRegularFile)
+                    .filter(f -> f.getFileName().toString().equals(filename))
+                    .findFirst();
         } catch (UncheckedIOException | IOException ex) {
             LOGGER.error("Error trying to locate the file {} inside the directory {}", filename, rootDirectory, ex);
         }
@@ -410,9 +434,8 @@ public static Optional<Path> find(String fileName, List<Path> directories) {
     /**
      * Converts a relative filename to an absolute one, if necessary.
      *
-     * @param fileName the filename (e.g., a .pdf file), may contain path separators
+     * @param fileName  the filename (e.g., a .pdf file), may contain path separators
      * @param directory the directory which should be search starting point
-     *
      * @return an empty optional if the file does not exist, otherwise, the absolute path
      */
     public static Optional<Path> find(String fileName, Path directory) {
@@ -504,10 +527,9 @@ public static Path getInitialDirectory(BibDatabaseContext databaseContext, Path
     /**
      * Detect illegal characters in given filename.
      *
-     * @see org.jabref.logic.util.io.FileNameCleaner#cleanFileName
-     *
      * @param fileName the fileName to detect
      * @return Boolean whether there is an illegal name.
+     * @see org.jabref.logic.util.io.FileNameCleaner#cleanFileName
      */
     public static boolean detectBadFileName(String fileName) {
         // fileName could be a path, we want to check the fileName only (and don't care about the path)
@@ -579,9 +601,9 @@ public static String shortenFileName(String fileName, Integer maxLength) {
         numCharsAfterEllipsis = Math.min(numCharsAfterEllipsis, name.length() - numCharsBeforeEllipsis);
 
         return name.substring(0, numCharsBeforeEllipsis) +
-               ELLIPSIS +
-               name.substring(name.length() - numCharsAfterEllipsis) +
-               extension;
+                ELLIPSIS +
+                name.substring(name.length() - numCharsAfterEllipsis) +
+                extension;
     }
 
     public static boolean isCharLegal(char c) {

jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java

@@ -4,7 +4,7 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.StandardOpenOption;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
@@ -39,7 +39,7 @@
 class FileUtilTest {
 
     @TempDir
-    static Path bibTempDir;
+    private static Path bibTempDir;
 
     private final Path nonExistingTestPath = Path.of("nonExistingTestPath");
     private Path existingTestFile;
@@ -54,11 +54,11 @@ void setUpViewModel(@TempDir Path temporaryFolder) throws IOException {
 
         existingTestFile = subDir.resolve("existingTestFile.txt");
         Files.createFile(existingTestFile);
-        Files.write(existingTestFile, "existingTestFile.txt".getBytes(StandardCharsets.UTF_8), StandardOpenOption.APPEND);
+        Files.writeString(existingTestFile, "existingTestFile.txt");
 
         otherExistingTestFile = subDir.resolve("otherExistingTestFile.txt");
         Files.createFile(otherExistingTestFile);
-        Files.write(otherExistingTestFile, "otherExistingTestFile.txt".getBytes(StandardCharsets.UTF_8), StandardOpenOption.APPEND);
+        Files.writeString(otherExistingTestFile, "otherExistingTestFile.txt");
     }
 
     @Test
@@ -222,16 +222,16 @@ void getFileNameWithMultipleDotsString() {
     @Test
     @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Assumed path separator is /")
     void uniquePathSubstrings() {
-       List<String> paths = List.of("C:/uniquefile.bib",
-               "C:/downloads/filename.bib",
-               "C:/mypaper/bib/filename.bib",
-               "C:/external/mypaper/bib/filename.bib",
-               "");
+        List<String> paths = List.of("C:/uniquefile.bib",
+                "C:/downloads/filename.bib",
+                "C:/mypaper/bib/filename.bib",
+                "C:/external/mypaper/bib/filename.bib",
+                "");
         List<String> uniqPath = List.of("uniquefile.bib",
-              "downloads/filename.bib",
-              "C:/mypaper/bib/filename.bib",
-              "external/mypaper/bib/filename.bib",
-              "");
+                "downloads/filename.bib",
+                "C:/mypaper/bib/filename.bib",
+                "external/mypaper/bib/filename.bib",
+                "");
 
         List<String> result = FileUtil.uniquePathSubstrings(paths);
         assertEquals(uniqPath, result);
@@ -448,6 +448,72 @@ public void cTemp() {
         }
     }
 
+    @ParameterizedTest
+    @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows")
+    @MethodSource
+    void relativizeSymlinks(Path file, List<Path> directories, Path expected, String message) {
+        if (message.startsWith("IGNORED")) {
+            org.junit.jupiter.api.Assumptions.assumeTrue(false, message);
+        }
+        Path result = FileUtil.relativize(file, directories);
+        assertEquals(expected, result, message);
+    }
+
+    /// Tests for issue <https://github.com/JabRef/jabref/issues/12995>
+    static Stream<Arguments> relativizeSymlinks() throws IOException {
+        List<Arguments> result = new ArrayList<>();
+
+        Path realDir = bibTempDir.resolve("realDir");
+        Files.createDirectories(realDir);
+
+        // symlinkDir -> realDir
+        // realDir/simple.pdf
+        Path simpleFile = Files.createFile(realDir.resolve("simple.pdf"));
+        Path symlinkDir = bibTempDir.resolve("symlinkDir");
+        Files.createSymbolicLink(symlinkDir, realDir);
+        result.add(Arguments.of(simpleFile, List.of(symlinkDir), Path.of("simple.pdf"), "Simple symlink resolves to relative"));
+
+        // chainLink1 -> chainLink2 -> chainReal
+        // chainReal/chained.pdf
+        Path chainReal = bibTempDir.resolve("chainReal");
+        Files.createDirectories(chainReal);
+        Path chainedFile = Files.createFile(chainReal.resolve("chained.pdf"));
+        Path chainLink2 = bibTempDir.resolve("chainLink2");
+        Files.createSymbolicLink(chainLink2, chainReal);
+        Path chainLink1 = bibTempDir.resolve("chainLink1");
+        Files.createSymbolicLink(chainLink1, chainLink2);
+        result.add(Arguments.of(chainedFile, List.of(chainLink1), Path.of("chained.pdf"), "Chained symlink resolves to relative"));
+
+        // realDir/nestedLink -> realDir/nested
+        // realDir/nested/nested.pdf
+        Path nestedDir = realDir.resolve("nested");
+        Files.createDirectories(nestedDir);
+        Path nestedFile = Files.createFile(nestedDir.resolve("nested.pdf"));
+        Path nestedSymlink = realDir.resolve("nestedLink");
+        Files.createSymbolicLink(nestedSymlink, nestedDir);
+        result.add(Arguments.of(nestedFile, List.of(nestedSymlink), Path.of("nested.pdf"), "Nested symlink resolves to relative"));
+
+        // symlinkDir -> realDir
+        // outside.pdf
+        Path outsideFile = Files.createFile(bibTempDir.resolve("outside.pdf"));
+        result.add(Arguments.of(outsideFile, List.of(symlinkDir), outsideFile, "Unrelated file remains absolute"));
+
+        // symlink chain escaping base dir (ignored test case, see #12995 issue comment)
+        Path veryPrivate = bibTempDir.resolve("veryprivate");
+        Files.createDirectories(veryPrivate);
+        Path secretFile = Files.createFile(veryPrivate.resolve("a.pdf"));
+        Path expensive = bibTempDir.resolve("expensive");
+        Files.createSymbolicLink(expensive, veryPrivate);
+        Path things = bibTempDir.resolve("things");
+        Files.createSymbolicLink(things, expensive);
+        Path libDir = bibTempDir.resolve("lib");
+        Files.createDirectories(libDir);
+        Path bibFile = Files.createFile(libDir.resolve("bib.bib"));
+        result.add(Arguments.of(secretFile, List.of(things), secretFile, "IGNORED: Symlink chain escaping base dir (#12995 comment)"));
+
+        return result.stream();
+    }
+
     /**
      * @implNote Tests inspired by {@link org.jabref.model.database.BibDatabaseContextTest#getFileDirectoriesWithRelativeMetadata}
      */