Skip to content

Auto file name truncation if path length exceeding limit while renaming on Windows #15488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xIshanSandhux wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Auto file name truncation if path length exceeding limit while renaming on Windows #15488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3836e4f
fixed the issue for windows
xIshanSandhux Apr 3, 2026
ef31e65
found which line is causing the error
xIshanSandhux Apr 3, 2026
232f677
fixed
xIshanSandhux Apr 3, 2026
7b8976d
added a const
xIshanSandhux Apr 3, 2026
8757e8f
added a const
xIshanSandhux Apr 4, 2026
1843b7e
small fix
xIshanSandhux Apr 4, 2026
7f47bfe
Revert unintended IntelliJ config changes
xIshanSandhux Apr 6, 2026
31b9e08
checkstyle fix
xIshanSandhux Apr 10, 2026
cfc7f1d
checkstyle fix
xIshanSandhux Apr 10, 2026
7b8ccc6
fixed test by adding localization key in properties
xIshanSandhux Apr 22, 2026
f098f05
fixed test by adding localization key in properties
xIshanSandhux Apr 22, 2026
b6aed17
Merge branch 'main' into fix-14771-issue
xIshanSandhux Apr 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.jabref.gui.fieldeditors;

import java.io.IOException;
import java.nio.file.FileSystemException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
Expand Down Expand Up @@ -270,12 +271,14 @@ public void renameFileToName(String targetFileName) {
private void performRenameWithConflictCheck(String targetFileName) {
// Check if a file with the same name already exists
Optional<Path> existingFile = linkedFileHandler.findExistingFile(linkedFile, entry, targetFileName);
LOGGER.debug("file already exists: {}", existingFile.isPresent());
boolean overwriteFile = false;

if (existingFile.isPresent()) {
// Get existing file path and its directory
Path existingFilePath = existingFile.get();
Path targetDirectory = existingFilePath.getParent();
LOGGER.debug("existing file path: {} || target Directory: {}", existingFilePath, targetDirectory);
Comment on lines +274 to +281
Copy link
Copy Markdown
Contributor

@LoayTarek5 LoayTarek5 Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove these development leftovers as you said, i think not useful for production logging.


// Suggest a non-conflicting file name
String suggestedFileName = FileNameUniqueness.getNonOverWritingFileName(targetDirectory, targetFileName);
Expand Down Expand Up @@ -316,10 +319,18 @@ private void performRenameWithConflictCheck(String targetFileName) {
// Attempt the rename operation
linkedFileHandler.renameToName(targetFileName, overwriteFile);
} catch (IOException e) {
// Display an error dialog if file is locked or inaccessible
dialogService.showErrorDialogAndWait(
Localization.lang("Rename failed"),
Localization.lang("JabRef cannot access the file because it is being used by another process."));
LOGGER.error("ERROR MESSAGE FOR RENAMING THE FILE: {}", e.getMessage());
if (e instanceof FileSystemException fe) {
LOGGER.error(fe.getReason());
dialogService.showErrorDialogAndWait(
Localization.lang("Rename failed"),
Localization.lang("JabRef could not rename the file. Please use a shorter filename or a shorter pattern or try changing the directory."));
} else {
// Display an error dialog if file is locked or inaccessible
dialogService.showErrorDialogAndWait(
Localization.lang("Rename failed"),
Localization.lang("JabRef cannot access the file because it is being used by another process."));
}
Comment on lines +322 to +333
Copy link
Copy Markdown
Contributor

@LoayTarek5 LoayTarek5 Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1- Remove the LOGGER.error with ALL CAPS message development leftover
2- fe.getReason() can return null on some implementations, so LOGGER.error(fe.getReason()) would log the literal string "null".

3-FileSystemException is too broad, it's the parent class of AccessDeniedException, FileAlreadyExistsException, NoSuchFileException, DirectoryNotEmptyException, etc.
with this code, a user whose file is genuinely locked by another process will now see "Please use a shorter filename", which i think reverses the original bug(if i wrong please corrrect me)

Comment on lines +323 to +333
Copy link
Copy Markdown
Contributor

@LoayTarek5 LoayTarek5 Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it is better to use two separate catch clauses instead of instanceof this is cleaner and matches JabRef style
maybe even better, check for the specific path length scenario rather than catching all FileSystemExceptions with a path length message, you could check fe.getMessage() or fe.getReason() for path-length indicators, or even better, since the truncation logic should prevent this case, consider what FileSystemException subtypes can actually still reach this point.

that is my thought, what do you think?

}
}

Expand Down
62 changes: 60 additions & 2 deletions jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.stream.Stream;

import org.jabref.logic.FilePreferences;
import org.jabref.logic.os.OS;
import org.jabref.logic.util.io.FileNameUniqueness;
import org.jabref.logic.util.io.FileUtil;
import org.jabref.logic.util.strings.StringUtil;
Expand All @@ -16,13 +17,17 @@
import org.jabref.model.entry.LinkedFile;

import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class LinkedFileHandler {

private static final Logger LOGGER = LoggerFactory.getLogger(LinkedFileHandler.class);

private static final int MAX_PATH_LENGTH_WINDOWS = 259;
private static final int SEPERATOR_WINDOWS = 1;
Comment on lines +28 to +29
Copy link
Copy Markdown
Contributor

@LoayTarek5 LoayTarek5 Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: "SEPERATOR_WINDOWS " -> "SEPARATOR_WINDOWS"
the diff is "A" instead of "E" in "SEPERATOR"

i think the it's also wrong location, MAX_PATH_LENGTH_WINDOWS is a platform fact, not a LinkedFileHandler concept, it belongs in org.jabref.logic.os.OS, next to OS.WINDOWS, that way copyOrMoveToDefaultDirectory (same file, same bug class) and anyone else can reuse it instead of redefining 259 late

but i will say that the second point is optional, if you want


private final BibDatabaseContext databaseContext;
private final FilePreferences filePreferences;
private final BibEntry entry;
Expand Down Expand Up @@ -111,7 +116,10 @@ public boolean copyOrMoveToDefaultDirectory(boolean shouldMove, boolean shouldRe
/// If exists: the path already exists and has the same content as the given sourcePath
///
/// @param renamed The original/suggested filename was adapted to fit it
private record GetTargetPathResult(boolean exists, boolean renamed, Path path) {
private record GetTargetPathResult(
boolean exists,
boolean renamed,
Path path) {
}

private GetTargetPathResult getTargetPath(Path sourcePath, Path targetDirectory, boolean useSuggestedName) throws IOException {
Expand Down Expand Up @@ -193,14 +201,29 @@ public boolean renameToName(String targetFileName, boolean overwriteExistingFile
}

final Path oldPath = oldFile.get();
final int parentPathLength = oldPath.getParent() == null ? 0 : oldPath.getParent().toString().length();

LOGGER.debug("PARENT: {}", oldPath.getParent());
Optional<String> oldExtension = FileUtil.getFileExtension(oldPath);
Optional<String> newExtension = FileUtil.getFileExtension(targetFileName);

Path newPath;
if (newExtension.isPresent() || (oldExtension.isEmpty() && newExtension.isEmpty())) {
if (OS.WINDOWS && (parentPathLength + targetFileName.length() + SEPERATOR_WINDOWS) > MAX_PATH_LENGTH_WINDOWS) {
if (newExtension.isPresent()) {
targetFileName = truncateFileNameOnWindows(targetFileName, parentPathLength, newExtension.get(), null);
} else {
targetFileName = truncateFileNameOnWindows(targetFileName, parentPathLength, null, null);
}
}
newPath = oldPath.resolveSibling(targetFileName);

LOGGER.debug("NEW PATH WITH THE NEW FILENAME: {}", newPath);
} else {
assert oldExtension.isPresent() && newExtension.isEmpty();
if (OS.WINDOWS && (parentPathLength + targetFileName.length() + SEPERATOR_WINDOWS) > MAX_PATH_LENGTH_WINDOWS) {
targetFileName = truncateFileNameOnWindows(targetFileName, parentPathLength, null, oldExtension.get());
}
newPath = oldPath.resolveSibling(targetFileName + "." + oldExtension.get());
}
Comment on lines +224 to 228
Copy link
Copy Markdown
Contributor

@LoayTarek5 LoayTarek5 Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guard and the actual path length don't match the final path is targetFileName + "." + oldExtension, but the check only counts targetFileName.length() + 1. so truncation is skipped for names that will still overflow Files.move.

Example:

parentPathLength = 240
targetFileName = "abcdefghijklmnopqr" (18 chars)
SEPARATOR = 1
Checked: 240 + 18 + 1 = 259 -> guard does NOT fire (259 > 259 is false)
Actual: 240 + 1 + 18 + 1 + 3 = 263 (parent + "" + name + "." + "pdf") -> EXCEEDS LIMIT


Expand All @@ -222,7 +245,9 @@ public boolean renameToName(String targetFileName, boolean overwriteExistingFile
Files.move(oldPath, newPath, StandardCopyOption.REPLACE_EXISTING);
} else {
Files.createDirectories(newPath.getParent());
LOGGER.debug("OVERWRITING FILENAME TO {}", newPath);
Files.move(oldPath, newPath);
LOGGER.debug("OVERWRITING SUCCESSFUL");
}

// Update path
Expand All @@ -235,15 +260,48 @@ public boolean renameToName(String targetFileName, boolean overwriteExistingFile
return true;
}

/// Helper function which truncates a file name for Windows if length (path + filename) exceeds the max windows
/// path limit.
///
/// @param targetFileName proposed file name (may include an extension; only the base name is truncated)
/// @param parentLength Length of the parent directory for the file being renamed
/// @param newExtension extension from the target name (no leading "."), or null if the target has none
/// @param oldExtension extension from the existing file when the target has no extension and the old extension is kept
/// @return the shortened file name; includes {@code .extension} when {@code newExtension} is not {@code null}
private String truncateFileNameOnWindows(String targetFileName, int parentLength, @Nullable String newExtension, @Nullable String oldExtension) {
Copy link
Copy Markdown
Contributor

@LoayTarek5 LoayTarek5 Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StringIndexOutOfBoundsException crash, if parentLength + extensionLength + 2 >= 259 (deep dir tree, long extension think OneDrive synced folders),
the substring index goes negative and this throws at runtime, need a guard and an upper bound

String baseName = FileUtil.getBaseName(targetFileName);
LOGGER.debug("BASENAME: {}", baseName);
int extensionLength = 0;
int dot = 0;
String fileName;

if (newExtension != null) {
extensionLength = newExtension.length();
dot = 1;
fileName = baseName.substring(0, (MAX_PATH_LENGTH_WINDOWS - parentLength - extensionLength - dot - SEPERATOR_WINDOWS)) + "." + newExtension;
} else if (oldExtension != null) {
extensionLength = oldExtension.length();
dot = 1;
fileName = baseName.substring(0, (MAX_PATH_LENGTH_WINDOWS - parentLength - extensionLength - dot - SEPERATOR_WINDOWS));
} else {
fileName = baseName.substring(0, (MAX_PATH_LENGTH_WINDOWS - parentLength - extensionLength - dot - SEPERATOR_WINDOWS));
}
LOGGER.debug("NEW FILE NAME: {}", fileName);

return fileName;
}

/// Determines the suggested file name based on the pattern specified in the preferences and valid for the file system.
/// Uses file extension from original file.
///
/// @return the suggested filename, including extension
public String getSuggestedFileName() {
LOGGER.debug("NORMAL getSuggestedFileName METHOD CALLED");
String filename = linkedFile.getFileName().orElse("file");
LOGGER.debug("FILENAME: {}", filename);
final String targetFileName = FileUtil.createFileNameFromPattern(databaseContext.getDatabase(), entry, filePreferences.getFileNamePattern())
.orElse(FileUtil.getBaseName(filename));

LOGGER.debug("TARGET FILE NAME: {}", targetFileName);
return FileUtil.getValidFileName(FileUtil.getFileExtension(filename).map(ext -> targetFileName + "." + ext).orElse(targetFileName));
}

Expand Down
2 changes: 2 additions & 0 deletions jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ public static List<Path> getListOfLinkedFiles(@NonNull List<BibEntry> entries, @
public static Optional<String> createFileNameFromPattern(BibDatabase database, BibEntry entry, String fileNamePattern) {
String targetName = BracketedPattern.expandBrackets(fileNamePattern, ';', entry, database).trim();

LOGGER.debug("TARGET NAME BEFORE CLEANUP: {}", targetName);
if (targetName.isEmpty() || "-".equals(targetName)) {
return entry.getCitationKey().map(FileNameCleaner::cleanFileName);
}
Expand All @@ -392,6 +393,7 @@ public static Optional<String> createFileNameFromPattern(BibDatabase database, B
targetName = REMOVE_LATEX_COMMANDS_FORMATTER.format(targetName);
// Removes illegal characters from filename
targetName = FileNameCleaner.cleanFileName(targetName);
LOGGER.debug("TARGET NAME AFTER CLEANUP: {}", targetName);

return Optional.of(targetName);
}
Expand Down
1 change: 1 addition & 0 deletions jablib/src/main/resources/l10n/JabRef_en.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1984,6 +1984,7 @@ Could\ not\ copy\ file=Could not copy file
Copied\ %0\ files\ of\ %1\ successfully\ to\ %2=Copied %0 files of %1 successfully to %2
Rename\ failed=Rename failed
JabRef\ cannot\ access\ the\ file\ because\ it\ is\ being\ used\ by\ another\ process.=JabRef cannot access the file because it is being used by another process.
JabRef\ could\ not\ rename\ the\ file.\ Please\ use\ a\ shorter\ filename\ or\ a\ shorter\ pattern\ or\ try\ changing\ the\ directory.=JabRef could not rename the file. Please use a shorter filename or a shorter pattern or try changing the directory.

Remove\ line\ breaks=Remove line breaks
Removes\ all\ line\ breaks\ in\ the\ field\ content.=Removes all line breaks in the field content.
Expand Down
Loading