diff --git a/CHANGELOG.md b/CHANGELOG.md index 3828ab2b996..ca43060f94d 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java index 9d99d4f9b8e..dde6df5d02e 100644 --- a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java +++ b/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 getUniquePathDirectory(List paths, Path comparePath) { @@ -153,7 +153,7 @@ public static Optional getUniquePathDirectory(List paths, Path c List 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))); } @@ -161,7 +161,7 @@ public static Optional getUniquePathDirectory(List paths, Path c /** * 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 getUniquePathFragment(List paths, Path comparePath) { @@ -219,7 +219,7 @@ public static List uniquePathSubstrings(List 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,40 @@ public static Path relativize(Path file, List directories) { if (!file.isAbsolute()) { return file; } + Optional realFileOpt = toRealPath(file); for (Path directory : directories) { if (file.startsWith(directory)) { return directory.relativize(file); } + + if (realFileOpt.isPresent()) { + Optional realDirOpt = toRealPath(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 toRealPath(Path path) { + if (Files.exists(path)) { + try { + return Optional.of(path.toRealPath()); + } catch (IOException e) { + LOGGER.warn("Could not resolve real path for {}", path, 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 +367,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 +395,9 @@ public static String createDirNameFromPattern(BibDatabase database, BibEntry ent public static Optional findSingleFileRecursively(String filename, Path rootDirectory) { try (Stream 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 +435,8 @@ public static Optional find(String fileName, List 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 find(String fileName, Path directory) { @@ -504,10 +528,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 +602,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) { diff --git a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java index 6d63a6bd98f..869a3aa974a 100644 --- a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java +++ b/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 paths = List.of("C:/uniquefile.bib", - "C:/downloads/filename.bib", - "C:/mypaper/bib/filename.bib", - "C:/external/mypaper/bib/filename.bib", - ""); + List paths = List.of("C:/uniquefile.bib", + "C:/downloads/filename.bib", + "C:/mypaper/bib/filename.bib", + "C:/external/mypaper/bib/filename.bib", + ""); List 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 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 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 + static Stream relativizeSymlinks() throws IOException { + List 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 ) + 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} */