Merge pull request JabRef#10205 from JabRef/fixAddingFilesWithIllegal

Fix adding files with illegal characters

CHANGELOG.md

@@ -39,6 +39,8 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We added an option to encrypt and remember the proxy password. [#8055](https://github.com/JabRef/jabref/issues/8055)[#10044](https://github.com/JabRef/jabref/issues/10044)
 - We added support for showing journal information, via info buttons next to the `Journal` and `ISSN` fields in the entry editor. [#6189](https://github.com/JabRef/jabref/issues/6189)
 - We added support for pushing citations to Sublime Text 3 [#10098](https://github.com/JabRef/jabref/issues/10098)
+- We added support for the Finnish language. [#10183](https://github.com/JabRef/jabref/pull/10183)
+- We added the option to automatically repleaces  illegal characters in the filename when adding a file to JabRef. [#10182](https://github.com/JabRef/jabref/issues/10182)
 
 ### Changed
 
@@ -129,7 +131,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We fixed an issue where JabRef would not remember if the window was in fullscreen or not [#4939](https://github.com/JabRef/jabref/issues/4939)
 - We fixed an issue where the ACM Portal search sometimes would not return entries for some search queries when the article author had no given name [#10107](https://github.com/JabRef/jabref/issues/10107)
 - We fixed an issue that caused high CPU usage and a zombie process after quitting JabRef because of author names autocompletion [#10159](https://github.com/JabRef/jabref/pull/10159)
-- We added support for the Finnish language. [#10183](https://github.com/JabRef/jabref/pull/10183)
+- We fixed an issue where files with illegal characters in the filename could be added to JabRef. [#10182](https://github.com/JabRef/jabref/issues/10182)
 
 ### Removed
 

src/main/java/org/jabref/gui/entryeditor/EntryEditor.java

@@ -122,7 +122,7 @@ public EntryEditor(LibraryTab libraryTab) {
                   .load();
 
         this.entryEditorPreferences = preferencesService.getEntryEditorPreferences();
-        this.fileLinker = new ExternalFilesEntryLinker(preferencesService.getFilePreferences(), databaseContext);
+        this.fileLinker = new ExternalFilesEntryLinker(preferencesService.getFilePreferences(), databaseContext, dialogService);
 
         EasyBind.subscribe(tabbed.getSelectionModel().selectedItemProperty(), tab -> {
             EntryEditorTab activeTab = (EntryEditorTab) tab;
@@ -147,19 +147,19 @@ public EntryEditor(LibraryTab libraryTab) {
             boolean success = false;
 
             if (event.getDragboard().hasContent(DataFormat.FILES)) {
-                List<Path> files = event.getDragboard().getFiles().stream().map(File::toPath).collect(Collectors.toList());
+                List<Path> draggedFiles = event.getDragboard().getFiles().stream().map(File::toPath).collect(Collectors.toList());
                 switch (event.getTransferMode()) {
                     case COPY -> {
                         LOGGER.debug("Mode COPY");
-                        fileLinker.copyFilesToFileDirAndAddToEntry(entry, files, libraryTab.getIndexingTaskManager());
+                        fileLinker.copyFilesToFileDirAndAddToEntry(entry, draggedFiles, libraryTab.getIndexingTaskManager());
                     }
                     case MOVE -> {
                         LOGGER.debug("Mode MOVE");
-                        fileLinker.moveFilesToFileDirAndAddToEntry(entry, files, libraryTab.getIndexingTaskManager());
+                        fileLinker.moveFilesToFileDirRenameAndAddToEntry(entry, draggedFiles, libraryTab.getIndexingTaskManager());
                     }
                     case LINK -> {
                         LOGGER.debug("Mode LINK");
-                        fileLinker.addFilesToEntry(entry, files);
+                        fileLinker.addFilesToEntry(entry, draggedFiles);
                     }
                 }
                 success = true;

src/main/java/org/jabref/gui/externalfiles/ExternalFilesEntryLinker.java

@@ -1,18 +1,23 @@
 package org.jabref.gui.externalfiles;
 
 import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
+import org.jabref.gui.DialogService;
 import org.jabref.gui.externalfiletype.ExternalFileType;
 import org.jabref.gui.externalfiletype.ExternalFileTypes;
 import org.jabref.gui.externalfiletype.UnknownExternalFileType;
 import org.jabref.logic.cleanup.MoveFilesCleanup;
 import org.jabref.logic.cleanup.RenamePdfCleanup;
+import org.jabref.logic.l10n.Localization;
 import org.jabref.logic.pdf.search.indexing.IndexingTaskManager;
 import org.jabref.logic.pdf.search.indexing.PdfIndexer;
+import org.jabref.logic.util.io.FileNameCleaner;
 import org.jabref.logic.util.io.FileUtil;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
@@ -30,12 +35,14 @@ public class ExternalFilesEntryLinker {
     private final BibDatabaseContext bibDatabaseContext;
     private final MoveFilesCleanup moveFilesCleanup;
     private final RenamePdfCleanup renameFilesCleanup;
+    private final DialogService dialogService;
 
-    public ExternalFilesEntryLinker(FilePreferences filePreferences, BibDatabaseContext bibDatabaseContext) {
+    public ExternalFilesEntryLinker(FilePreferences filePreferences, BibDatabaseContext bibDatabaseContext, DialogService dialogService) {
         this.filePreferences = filePreferences;
         this.bibDatabaseContext = bibDatabaseContext;
         this.moveFilesCleanup = new MoveFilesCleanup(bibDatabaseContext, filePreferences);
         this.renameFilesCleanup = new RenamePdfCleanup(false, bibDatabaseContext, filePreferences);
+        this.dialogService = dialogService;
     }
 
     public Optional<Path> copyFileToFileDir(Path file) {
@@ -58,7 +65,8 @@ public void moveLinkedFilesToFileDir(BibEntry entry) {
     }
 
     public void addFilesToEntry(BibEntry entry, List<Path> files) {
-        for (Path file : files) {
+        List<Path> validFiles = getValidFileNames(files);
+        for (Path file : validFiles) {
             FileUtil.getFileExtension(file).ifPresent(ext -> {
                 ExternalFileType type = ExternalFileTypes.getExternalFileTypeByExt(ext, filePreferences)
                                                          .orElse(new UnknownExternalFileType(ext));
@@ -69,7 +77,7 @@ public void addFilesToEntry(BibEntry entry, List<Path> files) {
         }
     }
 
-    public void moveFilesToFileDirAndAddToEntry(BibEntry entry, List<Path> files, IndexingTaskManager indexingTaskManager) {
+    public void moveFilesToFileDirRenameAndAddToEntry(BibEntry entry, List<Path> files, IndexingTaskManager indexingTaskManager) {
         try (AutoCloseable blocker = indexingTaskManager.blockNewTasks()) {
             addFilesToEntry(entry, files);
             moveLinkedFilesToFileDir(entry);
@@ -102,4 +110,32 @@ public void copyFilesToFileDirAndAddToEntry(BibEntry entry, List<Path> files, In
             LOGGER.error("Could not access Fulltext-Index", e);
         }
     }
+
+    private List<Path> getValidFileNames(List<Path> filesToAdd) {
+        List<Path> validFileNames = new ArrayList<>();
+
+        for (Path fileToAdd : filesToAdd) {
+            if (FileUtil.detectBadFileName(fileToAdd.toString())) {
+                String newFilename = FileNameCleaner.cleanFileName(fileToAdd.getFileName().toString());
+
+                boolean correctButtonPressed = dialogService.showConfirmationDialogAndWait(Localization.lang("File \"%0\" cannot be added!", fileToAdd.getFileName()),
+                        Localization.lang("Illegal characters in the file name detected.\nFile will be renamed to \"%0\" and added.", newFilename),
+                        Localization.lang("Rename and add"));
+
+                if (correctButtonPressed) {
+                    Path correctPath = fileToAdd.resolveSibling(newFilename);
+                    try {
+                        Files.move(fileToAdd, correctPath);
+                        validFileNames.add(correctPath);
+                    } catch (IOException ex) {
+                        LOGGER.error("Error moving file", ex);
+                        dialogService.showErrorDialogAndWait(ex);
+                    }
+                }
+            } else {
+                validFileNames.add(fileToAdd);
+            }
+        }
+        return validFileNames;
+    }
 }

src/main/java/org/jabref/gui/externalfiles/ImportHandler.java

@@ -83,7 +83,7 @@ public ImportHandler(BibDatabaseContext database,
         this.dialogService = dialogService;
         this.taskExecutor = taskExecutor;
 
-        this.linker = new ExternalFilesEntryLinker(preferencesService.getFilePreferences(), database);
+        this.linker = new ExternalFilesEntryLinker(preferencesService.getFilePreferences(), database, dialogService);
         this.contentImporter = new ExternalFilesContentImporter(preferencesService.getImportFormatPreferences());
         this.undoManager = undoManager;
     }

src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java

@@ -3,6 +3,7 @@
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;
@@ -34,6 +35,7 @@
 import org.jabref.logic.importer.util.FileFieldParser;
 import org.jabref.logic.integrity.FieldCheckers;
 import org.jabref.logic.l10n.Localization;
+import org.jabref.logic.util.io.FileNameCleaner;
 import org.jabref.logic.util.io.FileUtil;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
@@ -43,7 +45,11 @@
 import org.jabref.preferences.FilePreferences;
 import org.jabref.preferences.PreferencesService;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 public class LinkedFilesEditorViewModel extends AbstractEditorViewModel {
+    private static final Logger LOGGER = LoggerFactory.getLogger(LinkedFilesEditorViewModel.class);
 
     private final ListProperty<LinkedFileViewModel> files = new SimpleListProperty<>(FXCollections.observableArrayList(LinkedFileViewModel::getObservables));
     private final BooleanProperty fulltextLookupInProgress = new SimpleBooleanProperty(false);
@@ -139,16 +145,41 @@ public void addNewFile() {
                 .build();
 
         List<Path> fileDirectories = databaseContext.getFileDirectories(preferences.getFilePreferences());
-        dialogService.showFileOpenDialogAndGetMultipleFiles(fileDialogConfiguration).forEach(newFile -> {
-            LinkedFile newLinkedFile = fromFile(newFile, fileDirectories, preferences.getFilePreferences());
-            files.add(new LinkedFileViewModel(
-                    newLinkedFile,
-                    entry,
-                    databaseContext,
-                    taskExecutor,
-                    dialogService,
-                    preferences));
-        });
+        List<Path> selectedFiles = dialogService.showFileOpenDialogAndGetMultipleFiles(fileDialogConfiguration);
+
+        for (Path fileToAdd : selectedFiles) {
+            if (FileUtil.detectBadFileName(fileToAdd.toString())) {
+                String newFilename = FileNameCleaner.cleanFileName(fileToAdd.getFileName().toString());
+
+                boolean correctButtonPressed = dialogService.showConfirmationDialogAndWait(Localization.lang("File \"%0\" cannot be added!", fileToAdd.getFileName()),
+                        Localization.lang("Illegal characters in the file name detected.\nFile will be renamed to \"%0\" and added.", newFilename),
+                        Localization.lang("Rename and add"));
+
+                if (correctButtonPressed) {
+                    Path correctPath = fileToAdd.resolveSibling(newFilename);
+                    try {
+                        Files.move(fileToAdd, correctPath);
+                        addNewLinkedFile(correctPath, fileDirectories);
+                    } catch (IOException ex) {
+                        LOGGER.error("Error moving file", ex);
+                        dialogService.showErrorDialogAndWait(ex);
+                    }
+                }
+            } else {
+                addNewLinkedFile(fileToAdd, fileDirectories);
+            }
+        }
+    }
+
+    private void addNewLinkedFile(Path correctPath, List<Path> fileDirectories) {
+        LinkedFile newLinkedFile = fromFile(correctPath, fileDirectories, preferences.getFilePreferences());
+        files.add(new LinkedFileViewModel(
+                newLinkedFile,
+                entry,
+                databaseContext,
+                taskExecutor,
+                dialogService,
+                preferences));
     }
 
     @Override

src/main/java/org/jabref/gui/maintable/MainTable.java

@@ -433,7 +433,7 @@ private void handleOnDragDropped(TableRow<BibEntryTableViewModel> row, BibEntryT
                         }
                         case MOVE -> {
                             LOGGER.debug("Mode MOVE"); // alt on win
-                            importHandler.getLinker().moveFilesToFileDirAndAddToEntry(entry, files, libraryTab.getIndexingTaskManager());
+                            importHandler.getLinker().moveFilesToFileDirRenameAndAddToEntry(entry, files, libraryTab.getIndexingTaskManager());
                         }
                         case COPY -> {
                             LOGGER.debug("Mode Copy"); // ctrl on win

src/main/java/org/jabref/gui/preview/PreviewPanel.java

@@ -59,7 +59,7 @@ public PreviewPanel(BibDatabaseContext database,
         this.stateManager = stateManager;
         this.previewPreferences = preferences.getPreviewPreferences();
         this.indexingTaskManager = indexingTaskManager;
-        this.fileLinker = new ExternalFilesEntryLinker(preferences.getFilePreferences(), database);
+        this.fileLinker = new ExternalFilesEntryLinker(preferences.getFilePreferences(), database, dialogService);
 
         PreviewPreferences previewPreferences = preferences.getPreviewPreferences();
         previewView = new PreviewViewer(database, dialogService, stateManager, themeManager);
@@ -90,7 +90,7 @@ public PreviewPanel(BibDatabaseContext database,
 
                 if (event.getTransferMode() == TransferMode.MOVE) {
                     LOGGER.debug("Mode MOVE"); // shift on win or no modifier
-                    fileLinker.moveFilesToFileDirAndAddToEntry(entry, files, indexingTaskManager);
+                    fileLinker.moveFilesToFileDirRenameAndAddToEntry(entry, files, indexingTaskManager);
                 }
                 if (event.getTransferMode() == TransferMode.LINK) {
                     LOGGER.debug("Node LINK"); // alt on win

src/main/resources/l10n/JabRef_en.properties

@@ -2560,3 +2560,7 @@ This\ operation\ requires\ selected\ linked\ files.=This operation requires sele
 
 Create\ backup=Create backup
 Automatically\ search\ and\ show\ unlinked\ files\ in\ the\ entry\ editor=Automatically search and show unlinked files in the entry editor
+
+File\ "%0"\ cannot\ be\ added\!=File "%0" cannot be added!
+Illegal\ characters\ in\ the\ file\ name\ detected.\nFile\ will\ be\ renamed\ to\ "%0"\ and\ added.=Illegal characters in the file name detected.\nFile will be renamed to "%0" and added.
+Rename\ and\ add=Rename and add

src/test/java/org/jabref/logic/util/FileNameCleanerTest.java

@@ -12,6 +12,7 @@ public class FileNameCleanerTest {
     public void testCleanFileName() {
         assertEquals("legalFilename.txt", FileNameCleaner.cleanFileName("legalFilename.txt"));
         assertEquals("illegalFilename______.txt", FileNameCleaner.cleanFileName("illegalFilename/?*<>|.txt"));
+        assertEquals("illegalFileName_.txt", FileNameCleaner.cleanFileName("illegalFileName{.txt"));
     }
 
     @Test