From 45b40a7e352db58ea53bb73d360b56036e5ec173 Mon Sep 17 00:00:00 2001 From: Stone Zhang <51976945+the-star-sea@users.noreply.github.com> Date: Tue, 7 Jun 2022 04:08:32 +0800 Subject: [PATCH] Fix exception if linked file has masked umlauts / invalid characters in path (#8851) Co-authored-by: Christoph --- CHANGELOG.md | 1 + .../jabref/logic/util/io/FileNameCleaner.java | 26 ++--------- .../org/jabref/model/util/FileHelper.java | 45 ++++++++++++++++++- .../org/jabref/model/util/FileHelperTest.java | 9 ++++ 4 files changed, 57 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e505e0fc3a5..1d2f9fcc458 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We fixed an issue where an exception for DOI search occurred when the DOI contained urlencoded characters. [#8787](https://github.com/JabRef/jabref/issues/8787) - We fixed an issue which allow us to select and open identifiers from a popup list in the maintable [#8758](https://github.com/JabRef/jabref/issues/8758), [8802](https://github.com/JabRef/jabref/issues/8802) - We fixed an issue where the escape button had no functionality within the "Filter groups" textfield. [koppor#562](https://github.com/koppor/jabref/issues/562) +- We fixed an issue where the exception that there are invalid characters in filename. [#8786](https://github.com/JabRef/jabref/issues/8786) - When the proxy configuration removed the proxy user/password, this change is applied immediately. - We fixed an issue where removing several groups deletes only one of them. [#8390](https://github.com/JabRef/jabref/issues/8390) diff --git a/src/main/java/org/jabref/logic/util/io/FileNameCleaner.java b/src/main/java/org/jabref/logic/util/io/FileNameCleaner.java index 33733a3a900..83d764509e7 100644 --- a/src/main/java/org/jabref/logic/util/io/FileNameCleaner.java +++ b/src/main/java/org/jabref/logic/util/io/FileNameCleaner.java @@ -1,6 +1,6 @@ package org.jabref.logic.util.io; -import java.util.Arrays; +import org.jabref.model.util.FileHelper; /** * This class is based on http://stackoverflow.com/a/5626340/873282 @@ -9,22 +9,6 @@ */ public class FileNameCleaner { - /** - * MUST ALWAYS BE A SORTED ARRAY because it is used in a binary search - */ - // @formatter:off - private static final int[] ILLEGAL_CHARS = { - 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, - 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, - 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, - 30, 31, 34, - 42, - 58, - 60, 62, 63, - 123, 124, 125 - }; - // @formatter:on - private FileNameCleaner() { } @@ -38,7 +22,7 @@ public static String cleanFileName(String badFileName) { StringBuilder cleanName = new StringBuilder(badFileName.length()); for (int i = 0; i < badFileName.length(); i++) { char c = badFileName.charAt(i); - if (FileNameCleaner.isCharLegal(c) && (c != '/') && (c != '\\')) { + if (FileHelper.isCharLegal(c) && (c != '/') && (c != '\\')) { cleanName.append(c); } else { cleanName.append('_'); @@ -58,7 +42,7 @@ public static String cleanDirectoryName(String badFileName) { StringBuilder cleanName = new StringBuilder(badFileName.length()); for (int i = 0; i < badFileName.length(); i++) { char c = badFileName.charAt(i); - if (FileNameCleaner.isCharLegal(c)) { + if (FileHelper.isCharLegal(c)) { cleanName.append(c); } else { cleanName.append('_'); @@ -66,8 +50,4 @@ public static String cleanDirectoryName(String badFileName) { } return cleanName.toString().trim(); } - - private static boolean isCharLegal(char c) { - return Arrays.binarySearch(FileNameCleaner.ILLEGAL_CHARS, c) < 0; - } } diff --git a/src/main/java/org/jabref/model/util/FileHelper.java b/src/main/java/org/jabref/model/util/FileHelper.java index 25509e50c69..5eddefb724e 100644 --- a/src/main/java/org/jabref/model/util/FileHelper.java +++ b/src/main/java/org/jabref/model/util/FileHelper.java @@ -6,6 +6,7 @@ import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.Objects; @@ -22,8 +23,27 @@ import org.apache.tika.mime.MimeType; import org.apache.tika.mime.MimeTypeException; import org.apache.tika.parser.AutoDetectParser; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class FileHelper { + /** + * MUST ALWAYS BE A SORTED ARRAY because it is used in a binary search + */ + // @formatter:off + private static final int[] ILLEGAL_CHARS = { + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, + 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, + 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, + 30, 31, 34, + 42, + 58, + 60, 62, 63, + 123, 124, 125 + }; + // @formatter:on + + private static final Logger LOGGER = LoggerFactory.getLogger(FileHelper.class); /** * Returns the extension of a file or Optional.empty() if the file does not have one (no . in name). @@ -105,6 +125,26 @@ public static Optional find(String fileName, List directories) { .findFirst(); } + /** + * Detect illegal characters in given filename. + * + * @param fileName the fileName to detect + * @return Boolean whether there is a illegal name. + */ + public static boolean detectBadFileName(String fileName) { + for (int i = 0; i < fileName.length(); i++) { + char c = fileName.charAt(i); + if (!isCharLegal(c)) { + return true; + } + } + return false; + } + + public static boolean isCharLegal(char c) { + return Arrays.binarySearch(ILLEGAL_CHARS, c) < 0; + } + /** * Converts a relative filename to an absolute one, if necessary. Returns * an empty optional if the file does not exist. @@ -112,9 +152,12 @@ public static Optional find(String fileName, List directories) { public static Optional find(String fileName, Path directory) { Objects.requireNonNull(fileName); Objects.requireNonNull(directory); - // Explicitly check for an empty String, as File.exists returns true on that empty path, because it maps to the default jar location // if we then call toAbsoluteDir, it would always return the jar-location folder. This is not what we want here + if (detectBadFileName(fileName)) { + LOGGER.error("Invalid characters in path for file {} ", fileName); + return Optional.empty(); + } if (fileName.isEmpty()) { return Optional.of(directory); } diff --git a/src/test/java/org/jabref/model/util/FileHelperTest.java b/src/test/java/org/jabref/model/util/FileHelperTest.java index 6d047662b12..9584a7b53f8 100644 --- a/src/test/java/org/jabref/model/util/FileHelperTest.java +++ b/src/test/java/org/jabref/model/util/FileHelperTest.java @@ -4,6 +4,8 @@ import java.util.Optional; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -25,4 +27,11 @@ public void testFileNameEmpty() { Path path = Path.of("/"); assertEquals(Optional.of(path), FileHelper.find("", path)); } + + @ParameterizedTest + @ValueSource(strings = { "*", "?", ">", "\"" }) + public void testFileNameIllegal(String fileName) { + Path path = Path.of("/"); + assertEquals(Optional.empty(), FileHelper.find(fileName, path)); + } }