From 9ecfdc8ac2ecc9b1d116900046106457ea80d927 Mon Sep 17 00:00:00 2001 From: stone <18871162559@163.com> Date: Thu, 26 May 2022 14:39:00 +0800 Subject: [PATCH 01/10] fix-issue-8786 --- .idea/runConfigurations/JabRef_Main.xml | 14 ------ .../org/jabref/model/util/FileHelper.java | 43 ++++++++++++++++--- .../org/jabref/model/util/FileHelperTest.java | 18 ++++++++ 3 files changed, 55 insertions(+), 20 deletions(-) delete mode 100644 .idea/runConfigurations/JabRef_Main.xml diff --git a/.idea/runConfigurations/JabRef_Main.xml b/.idea/runConfigurations/JabRef_Main.xml deleted file mode 100644 index 731578b5c75..00000000000 --- a/.idea/runConfigurations/JabRef_Main.xml +++ /dev/null @@ -1,14 +0,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..18558e147ca 100644 --- a/src/main/java/org/jabref/model/util/FileHelper.java +++ b/src/main/java/org/jabref/model/util/FileHelper.java @@ -6,10 +6,8 @@ import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; -import java.util.List; -import java.util.Locale; -import java.util.Objects; -import java.util.Optional; +import java.util.*; + import org.jabref.model.database.BibDatabaseContext; import org.jabref.preferences.FilePreferences; @@ -22,9 +20,11 @@ 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 { - + 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). * @@ -104,7 +104,35 @@ public static Optional find(String fileName, List directories) { .flatMap(directory -> find(fileName, directory).stream()) .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; + } + private static boolean isCharLegal(char c) { + 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 + }; + 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 +140,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"); + 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..0a4c3d5bf27 100644 --- a/src/test/java/org/jabref/model/util/FileHelperTest.java +++ b/src/test/java/org/jabref/model/util/FileHelperTest.java @@ -25,4 +25,22 @@ public void testFileNameEmpty() { Path path = Path.of("/"); assertEquals(Optional.of(path), FileHelper.find("", path)); } + + @Test + public void testFileNameIllegal1() { + Path path = Path.of("/"); + assertEquals(Optional.empty(), FileHelper.find(">", path)); + } + + @Test + public void testFileNameIllegal2() { + Path path = Path.of("/"); + assertEquals(Optional.empty(), FileHelper.find("*", path)); + } + + @Test + public void testFileNameIllegal3() { + Path path = Path.of("/"); + assertEquals(Optional.empty(), FileHelper.find("?", path)); + } } From d6ae0715c714174e9fd53402f28b86bf62833146 Mon Sep 17 00:00:00 2001 From: stone <18871162559@163.com> Date: Thu, 26 May 2022 15:17:35 +0800 Subject: [PATCH 02/10] change.log and check style --- CHANGELOG.md | 1 + src/main/java/org/jabref/model/util/FileHelper.java | 13 ++++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8af025df4c1..f31a7d7ccac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We fixed an issue where deprecated fields tab is shown when the fields don't contain any values. [#8396](https://github.com/JabRef/jabref/issues/8396) - 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) ### Removed diff --git a/src/main/java/org/jabref/model/util/FileHelper.java b/src/main/java/org/jabref/model/util/FileHelper.java index 18558e147ca..1c16409005b 100644 --- a/src/main/java/org/jabref/model/util/FileHelper.java +++ b/src/main/java/org/jabref/model/util/FileHelper.java @@ -6,8 +6,11 @@ import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; -import java.util.*; - +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import java.util.Objects; +import java.util.Optional; import org.jabref.model.database.BibDatabaseContext; import org.jabref.preferences.FilePreferences; @@ -108,8 +111,10 @@ public static Optional find(String fileName, List directories) { * Detect illegal characters in given filename. * * @param fileName the fileName to detect - * @return Boolean whether there is a illegal name + * @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); @@ -136,7 +141,9 @@ private static boolean isCharLegal(char c) { /** * Converts a relative filename to an absolute one, if necessary. Returns * an empty optional if the file does not exist. + * */ + public static Optional find(String fileName, Path directory) { Objects.requireNonNull(fileName); Objects.requireNonNull(directory); From 4b0f89091e6ab0ad93cbac1be13a58b4201cce5c Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Fri, 27 May 2022 18:50:31 +0200 Subject: [PATCH 03/10] DRY move to common class --- .../jabref/logic/util/io/FileNameCleaner.java | 25 ++------------- .../org/jabref/model/util/FileHelper.java | 32 ++++++++++++------- 2 files changed, 23 insertions(+), 34 deletions(-) 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..63786a2e06e 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,21 +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 +23,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 +43,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 +51,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 1c16409005b..90c53f16ffb 100644 --- a/src/main/java/org/jabref/model/util/FileHelper.java +++ b/src/main/java/org/jabref/model/util/FileHelper.java @@ -27,6 +27,22 @@ 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). @@ -115,7 +131,7 @@ public static Optional find(String fileName, List directories) { * */ - public static Boolean detectBadFileName(String fileName) { + public static boolean detectBadFileName(String fileName) { for (int i = 0; i < fileName.length(); i++) { char c = fileName.charAt(i); if (!isCharLegal(c)) { @@ -125,19 +141,11 @@ public static Boolean detectBadFileName(String fileName) { return false; } - private static boolean isCharLegal(char c) { - 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 - }; + + 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. From 11f280eda8de59f4a7bf4b1c4a9984d38c985ee6 Mon Sep 17 00:00:00 2001 From: Stone Zhang <51976945+the-star-sea@users.noreply.github.com> Date: Sat, 28 May 2022 01:46:24 +0800 Subject: [PATCH 04/10] Update src/main/java/org/jabref/model/util/FileHelper.java Co-authored-by: Christoph --- src/main/java/org/jabref/model/util/FileHelper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/model/util/FileHelper.java b/src/main/java/org/jabref/model/util/FileHelper.java index 90c53f16ffb..7dadc9c7994 100644 --- a/src/main/java/org/jabref/model/util/FileHelper.java +++ b/src/main/java/org/jabref/model/util/FileHelper.java @@ -158,7 +158,7 @@ public static Optional find(String fileName, Path 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"); + LOGGER.error("Invalid characters in path for file {} ", fileName); return Optional.empty(); } if (fileName.isEmpty()) { From ae63e17f353b90a512dc6d4156e9120d0faa4b16 Mon Sep 17 00:00:00 2001 From: Stone Zhang <51976945+the-star-sea@users.noreply.github.com> Date: Sat, 4 Jun 2022 21:07:45 +0800 Subject: [PATCH 05/10] Update CHANGELOG.md Co-authored-by: Oliver Kopp --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c11489e2b00..08b3d79828d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,7 +33,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We fixed an issue where an exxception for DOI search occured 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) +- We fixed an issue where the exception that there are invalid characters in filename. [#8786](https://github.com/JabRef/jabref/issues/8786) - We fixed an issue where right clicking a group and choose "remove selected entries from this group" leads to error when Bibtex source tab is selected. [#8012](https://github.com/JabRef/jabref/issues/8012) ### Removed From 7f5020d23405cfa6efab476e8da60780b1c0b28f Mon Sep 17 00:00:00 2001 From: stone <18871162559@163.com> Date: Sat, 4 Jun 2022 21:30:44 +0800 Subject: [PATCH 06/10] Code style check and modify tests. --- .../jabref/logic/util/io/FileNameCleaner.java | 1 - .../org/jabref/model/util/FileHelper.java | 1 - .../org/jabref/model/util/FileHelperTest.java | 20 ++++++------------- 3 files changed, 6 insertions(+), 16 deletions(-) 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 63786a2e06e..83d764509e7 100644 --- a/src/main/java/org/jabref/logic/util/io/FileNameCleaner.java +++ b/src/main/java/org/jabref/logic/util/io/FileNameCleaner.java @@ -9,7 +9,6 @@ */ public class FileNameCleaner { - private FileNameCleaner() { } diff --git a/src/main/java/org/jabref/model/util/FileHelper.java b/src/main/java/org/jabref/model/util/FileHelper.java index 7dadc9c7994..577d1df4f74 100644 --- a/src/main/java/org/jabref/model/util/FileHelper.java +++ b/src/main/java/org/jabref/model/util/FileHelper.java @@ -141,7 +141,6 @@ public static boolean detectBadFileName(String fileName) { return false; } - public static boolean isCharLegal(char c) { return Arrays.binarySearch(ILLEGAL_CHARS, c) < 0; } diff --git a/src/test/java/org/jabref/model/util/FileHelperTest.java b/src/test/java/org/jabref/model/util/FileHelperTest.java index 0a4c3d5bf27..b99b89e01ee 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; @@ -26,21 +28,11 @@ public void testFileNameEmpty() { assertEquals(Optional.of(path), FileHelper.find("", path)); } - @Test - public void testFileNameIllegal1() { - Path path = Path.of("/"); - assertEquals(Optional.empty(), FileHelper.find(">", path)); - } - - @Test - public void testFileNameIllegal2() { + @ParameterizedTest + @ValueSource(strings = { "*","?",">","\"" }) + public void testFileNameIllegal(String fileName) { Path path = Path.of("/"); - assertEquals(Optional.empty(), FileHelper.find("*", path)); + assertEquals(Optional.empty(), FileHelper.find(fileName, path)); } - @Test - public void testFileNameIllegal3() { - Path path = Path.of("/"); - assertEquals(Optional.empty(), FileHelper.find("?", path)); - } } From 2423874a359bda84feaa19c59c42c8046c443150 Mon Sep 17 00:00:00 2001 From: stone <18871162559@163.com> Date: Sat, 4 Jun 2022 22:58:24 +0800 Subject: [PATCH 07/10] Modify tests. --- src/test/java/org/jabref/model/util/FileHelperTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/org/jabref/model/util/FileHelperTest.java b/src/test/java/org/jabref/model/util/FileHelperTest.java index b99b89e01ee..9584a7b53f8 100644 --- a/src/test/java/org/jabref/model/util/FileHelperTest.java +++ b/src/test/java/org/jabref/model/util/FileHelperTest.java @@ -29,10 +29,9 @@ public void testFileNameEmpty() { } @ParameterizedTest - @ValueSource(strings = { "*","?",">","\"" }) + @ValueSource(strings = { "*", "?", ">", "\"" }) public void testFileNameIllegal(String fileName) { Path path = Path.of("/"); assertEquals(Optional.empty(), FileHelper.find(fileName, path)); } - } From b9fc43e47d384c17912fbc3609d22eed40cff7df Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 6 Jun 2022 21:40:11 +0200 Subject: [PATCH 08/10] Fix empty lines --- src/main/java/org/jabref/model/util/FileHelper.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/jabref/model/util/FileHelper.java b/src/main/java/org/jabref/model/util/FileHelper.java index 577d1df4f74..5eddefb724e 100644 --- a/src/main/java/org/jabref/model/util/FileHelper.java +++ b/src/main/java/org/jabref/model/util/FileHelper.java @@ -44,6 +44,7 @@ public class FileHelper { // @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). * @@ -123,14 +124,13 @@ public static Optional find(String fileName, List directories) { .flatMap(directory -> find(fileName, directory).stream()) .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); @@ -148,9 +148,7 @@ public static boolean isCharLegal(char c) { /** * Converts a relative filename to an absolute one, if necessary. Returns * an empty optional if the file does not exist. - * */ - public static Optional find(String fileName, Path directory) { Objects.requireNonNull(fileName); Objects.requireNonNull(directory); From cea3c6c16a391909eb9e7897ee6827aecd53877c Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 6 Jun 2022 21:42:10 +0200 Subject: [PATCH 09/10] Restore JabRef_Main.xml --- .idea/runConfigurations/JabRef_Main.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .idea/runConfigurations/JabRef_Main.xml diff --git a/.idea/runConfigurations/JabRef_Main.xml b/.idea/runConfigurations/JabRef_Main.xml new file mode 100644 index 00000000000..731578b5c75 --- /dev/null +++ b/.idea/runConfigurations/JabRef_Main.xml @@ -0,0 +1,14 @@ + + + + From cbbfd5dbc2551910f5b2030a314a295438d5613f Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 6 Jun 2022 22:05:30 +0200 Subject: [PATCH 10/10] Fix CHANGELOG.md --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74da533ca1b..1d2f9fcc458 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,7 +42,6 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - 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) -- We fixed an issue where right clicking a group and choose "remove selected entries from this group" leads to error when Bibtex source tab is selected. [#8012](https://github.com/JabRef/jabref/issues/8012) - 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)