Fix exception if linked file has masked umlauts / invalid characters …

…in path (#8851)

Co-authored-by: Christoph <siedlerkiller@gmail.com>

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)
 

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,16 +42,12 @@ 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('_');
             }
         }
         return cleanName.toString().trim();
     }
-
-    private static boolean isCharLegal(char c) {
-        return Arrays.binarySearch(FileNameCleaner.ILLEGAL_CHARS, c) < 0;
-    }
 }

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,16 +125,39 @@ public static Optional<Path> find(String fileName, List<Path> 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.
      */
     public static Optional<Path> 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);
         }

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));
+    }
 }