Allow drag and drop of Windows shortcut (.lnk) files : Fix for issue #15036#15053
Allow drag and drop of Windows shortcut (.lnk) files : Fix for issue #15036#15053faneeshh wants to merge 4 commits intoJabRef:mainfrom
Conversation
Review Summary by QodoSupport drag and drop of Windows shortcut (.lnk) files
WalkthroughsDescription• Add support for dragging and dropping Windows shortcut (.lnk) files • Resolve .lnk shortcuts to target .bib files using PowerShell • Update FrameDndHandler and MainTable to handle shortcut resolution • Add utility methods in FileUtil for shortcut detection and resolution Diagramflowchart LR
A["User drags .lnk file"] --> B["FrameDndHandler/MainTable receives file"]
B --> C["FileUtil.isShortcutFile checks extension"]
C --> D["FileUtil.resolveWindowsShortcut uses PowerShell"]
D --> E["Target .bib file path returned"]
E --> F["Library opens with entries"]
File Changes1. jabgui/src/main/java/org/jabref/gui/frame/FrameDndHandler.java
|
github-actions
bot
added
the
good first issue
|
The title of the pull request must not start with "Fix for issue xyz". Please use a concise one-line summary that explains what the fix or change actually does. Example of a good title: "Prevent crash when importing malformed BibTeX entries". |
|
You have removed the "Mandatory Checks" section from your pull request description. Please adhere to our pull request template. |
|
Note that your PR will not be reviewed/accepted until you have gone through the mandatory checks in the description and marked each of them them exactly in the format of |
Code Review by Qodo
1. resolveWindowsShortcut catches Exception
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| try { | ||
| // Use PowerShell to resolve the shortcut target | ||
| ProcessBuilder pb = new ProcessBuilder( | ||
| "powershell", "-NoProfile", "-Command", | ||
| "$ws = New-Object -ComObject WScript.Shell; " + | ||
| "$shortcut = $ws.CreateShortcut('" + shortcutPath.toAbsolutePath().toString().replace("'", "''") + "'); " + | ||
| "Write-Output $shortcut.TargetPath" | ||
| ); | ||
|
|
||
| pb.redirectErrorStream(true); | ||
| Process process = pb.start(); | ||
|
|
||
| String targetPath; | ||
| try (java.io.BufferedReader reader = new java.io.BufferedReader( | ||
| new java.io.InputStreamReader(process.getInputStream()))) { | ||
| targetPath = reader.readLine(); | ||
| } | ||
|
|
||
| int exitCode = process.waitFor(); | ||
|
|
||
| if (exitCode == 0 && targetPath != null && !targetPath.trim().isEmpty()) { | ||
| Path resolvedPath = Path.of(targetPath.trim()); | ||
| if (Files.exists(resolvedPath)) { | ||
| LOGGER.debug("Resolved shortcut {} to {}", shortcutPath, resolvedPath); | ||
| return Optional.of(resolvedPath); | ||
| } else { | ||
| LOGGER.warn("Shortcut target does not exist: {}", resolvedPath); | ||
| return Optional.empty(); | ||
| } | ||
| } else { | ||
| LOGGER.warn("Failed to resolve shortcut: {} (exit code: {})", shortcutPath, exitCode); | ||
| return Optional.empty(); | ||
| } | ||
| } catch (Exception e) { | ||
| LOGGER.warn("Exception while resolving shortcut: {}", shortcutPath, e); | ||
| return Optional.empty(); | ||
| } |
There was a problem hiding this comment.
1. resolvewindowsshortcut catches exception 📘 Rule violation ⛯ Reliability
• resolveWindowsShortcut wraps the whole PowerShell invocation in a broad catch (Exception) and degrades by returning Optional.empty(), which can silently drop .lnk files during drag-and-drop without a clear recovery path. • The method blocks on process.waitFor() with no timeout and does not restore the interrupt flag on interruption, which risks hangs and makes failures harder to diagnose reliably. • This violates the requirement for specific, non-disruptive exception handling and robust edge-case management around external process execution.
Agent Prompt
## Issue description
`FileUtil.resolveWindowsShortcut` catches a broad `Exception` and blocks on `process.waitFor()` without a timeout. This can lead to silent failures (returning `Optional.empty()`), and it risks hanging the application thread.
## Issue Context
The shortcut resolution executes an external process (PowerShell) which is a high-failure-risk dependency (missing executable, permission issues, COM errors, long-running/hung process). Exception handling should be specific and interruptions should preserve the thread interrupt status.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[548-584]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| /// Test if the file is a shortcut file by simply checking the extension to be ".lnk" | ||
| /// | ||
| /// @param file The file to check | ||
| /// @return True if file extension is ".lnk", false otherwise | ||
| public static boolean isShortcutFile(Path file) { | ||
| return getFileExtension(file).filter("lnk"::equals).isPresent(); | ||
| } | ||
|
|
||
| /// Resolves a Windows shortcut (.lnk) file to its target path. | ||
| /// Only works on Windows systems. On other systems or if resolution fails, returns empty Optional. | ||
| /// | ||
| /// @param shortcutPath The path to the .lnk file | ||
| /// @return Optional containing the target path, or empty if resolution fails | ||
| public static Optional<Path> resolveWindowsShortcut(Path shortcutPath) { | ||
| if (!isShortcutFile(shortcutPath)) { | ||
| return Optional.empty(); | ||
| } | ||
|
|
||
| // Only attempt on Windows | ||
| if (!OS.WINDOWS) { | ||
| LOGGER.debug("Shortcut resolution only supported on Windows"); | ||
| return Optional.empty(); | ||
| } | ||
|
|
||
| try { | ||
| // Use PowerShell to resolve the shortcut target | ||
| ProcessBuilder pb = new ProcessBuilder( | ||
| "powershell", "-NoProfile", "-Command", | ||
| "$ws = New-Object -ComObject WScript.Shell; " + | ||
| "$shortcut = $ws.CreateShortcut('" + shortcutPath.toAbsolutePath().toString().replace("'", "''") + "'); " + | ||
| "Write-Output $shortcut.TargetPath" | ||
| ); | ||
|
|
||
| pb.redirectErrorStream(true); | ||
| Process process = pb.start(); | ||
|
|
||
| String targetPath; | ||
| try (java.io.BufferedReader reader = new java.io.BufferedReader( | ||
| new java.io.InputStreamReader(process.getInputStream()))) { | ||
| targetPath = reader.readLine(); | ||
| } | ||
|
|
||
| int exitCode = process.waitFor(); | ||
|
|
||
| if (exitCode == 0 && targetPath != null && !targetPath.trim().isEmpty()) { | ||
| Path resolvedPath = Path.of(targetPath.trim()); | ||
| if (Files.exists(resolvedPath)) { | ||
| LOGGER.debug("Resolved shortcut {} to {}", shortcutPath, resolvedPath); | ||
| return Optional.of(resolvedPath); | ||
| } else { | ||
| LOGGER.warn("Shortcut target does not exist: {}", resolvedPath); | ||
| return Optional.empty(); | ||
| } | ||
| } else { | ||
| LOGGER.warn("Failed to resolve shortcut: {} (exit code: {})", shortcutPath, exitCode); | ||
| return Optional.empty(); | ||
| } | ||
| } catch (Exception e) { | ||
| LOGGER.warn("Exception while resolving shortcut: {}", shortcutPath, e); | ||
| return Optional.empty(); | ||
| } | ||
| } | ||
|
|
||
| public static List<Path> resolveWindowsShortcuts(List<Path> paths) { | ||
| return paths.stream() | ||
| .flatMap(path -> { | ||
| if (isShortcutFile(path)) { | ||
| return resolveWindowsShortcut(path).stream(); | ||
| } | ||
| return Stream.of(path); | ||
| }) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
2. Missing tests for .lnk 📘 Rule violation ✓ Correctness
• New behavior was introduced for resolving Windows shortcuts via FileUtil.resolveWindowsShortcut/resolveWindowsShortcuts and used in drag-and-drop flows, but no corresponding JUnit tests were added/updated to cover this behavior. • The existing FileUtilTest covers file extension handling, yet does not include .lnk cases, leaving key edge cases (non-Windows behavior, failure modes) unverified.
Agent Prompt
## Issue description
New Windows shortcut (`.lnk`) drag-and-drop support introduces new `FileUtil` behavior without corresponding test coverage.
## Issue Context
At minimum, tests should cover:
- `isShortcutFile` correctly detecting `.lnk` (case handling if intended)
- `resolveWindowsShortcuts` returning the original paths for non-shortcut inputs
- non-Windows behavior: `resolveWindowsShortcut` should return `Optional.empty()` without throwing
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[524-596]
- jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java[179-210]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| /// Test if the file is a shortcut file by simply checking the extension to be ".lnk" | ||
| /// | ||
| /// @param file The file to check | ||
| /// @return True if file extension is ".lnk", false otherwise | ||
| public static boolean isShortcutFile(Path file) { | ||
| return getFileExtension(file).filter("lnk"::equals).isPresent(); | ||
| } |
There was a problem hiding this comment.
3. Case-sensitive .lnk check 🐞 Bug ✓ Correctness
• isShortcutFile uses exact string match ("lnk"::equals) but getFileExtension does not
actually lowercase the extension, despite its documentation.
• As a result, shortcuts like FILE.LNK may not be recognized, breaking the new drag-and-drop
behavior on Windows.
Agent Prompt
### Issue description
`.lnk` detection is effectively case-sensitive because `getFileExtension` returns the raw extension and `isShortcutFile` compares using `"lnk"::equals`.
### Issue Context
Windows file extensions are case-insensitive and `.LNK` is common. The current implementation can fail to detect valid shortcuts.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[74-94]
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[524-530]
### Suggested approach
- Update `getFileExtension` to return `extension.toLowerCase(Locale.ROOT)` (and adjust imports/tests accordingly), aligning with its Javadoc.
- Alternatively, change `isShortcutFile` to `filter(ext -> ext.equalsIgnoreCase("lnk"))` (but prefer central normalization in `getFileExtension`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (exitCode == 0 && targetPath != null && !targetPath.trim().isEmpty()) { | ||
| Path resolvedPath = Path.of(targetPath.trim()); | ||
| if (Files.exists(resolvedPath)) { | ||
| LOGGER.debug("Resolved shortcut {} to {}", shortcutPath, resolvedPath); | ||
| return Optional.of(resolvedPath); | ||
| } else { |
There was a problem hiding this comment.
4. Shortcut returns non-file 🐞 Bug ✓ Correctness
• resolveWindowsShortcut returns any existing target path, including directories, because it only checks Files.exists. • Downstream import/link logic in ImportHandler treats unknown paths by creating an empty entry with a file link, which can create incorrect entries if a shortcut points to a directory.
Agent Prompt
### Issue description
Shortcut resolution currently returns any existing path, including directories. This can leak directory targets into file-import/link flows, creating empty entries with folder links.
### Issue Context
`ImportHandler` treats unknown paths as generic files and creates an empty entry with a link, which is incorrect when the path is a folder.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[568-576]
- jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java[183-199]
### Suggested approach
- Replace `Files.exists(resolvedPath)` with `Files.isRegularFile(resolvedPath)` (or `Files.isRegularFile(resolvedPath, LinkOption.NOFOLLOW_LINKS)` if appropriate).
- Consider logging a clear warning when the shortcut target is not a regular file.
- Optionally: for callers that can accept directories, add an explicit parameter/alternate method rather than returning directories by default.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
github-actions
bot
added
the
status: changes-required
✅ All tests passed ✅🏷️ Commit: 510012a Learn more about TestLens at testlens.app. |
|
There is another java library which apparently can parse this stuf https://github.com/DmitriiShamrikov/mslinks |
|
Note that resolving works at JabRef (!) If I double click a .lnk file - JabRef opens. Thus "only" the drag and drop disallowance is wrong currently. |
Who works on DmitriiShamrikov/mslinks#29 ? |
|
We need the newver version, because of JDK11 compatibility - https://github.com/DmitriiShamrikov/mslinks/releases/tag/1.0.8 |
I tested it by only allowing the .lnk extension. While the file is accepted by the UI, it currently results in an empty table because the Drag and Drop path doesn't seem to trigger the same resolution logic as double-clicking or the file picker. I am currently looking into how CommandLineProcessor or the main entry point handles .lnk files to see if I can reuse that existing resolution logic here without adding new dependencies like mslinks. |
|
You ticked that you modified If you made changes that are visible to the user, please add a brief description along with the issue number to the |
|
@koppor I was able to resolve it by synchronizing the two seperate drag and drop code paths by integrating Windows shortcut resolution into the MainTable handlers. Previously dropping a .lnk file directly onto the table was bypassing the resolution logic in OpenDatabaseAction which resulted in "misc" entries. I think applying FileUtil : : resolveIfShortcut within both handleOnDragDroppedTable view and handleOnDragDropped in MainTable.java, and refining the resolution logic in FileUtil.java using mslinks.ShellLink now correctly resolves shortcuts. Let me know what you think! |
|
You modified Markdown ( You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Source Code Tests" (on the left), subsection "Markdown". |
|
recent |
| } | ||
|
|
||
| private boolean isAcceptedFile(Path path) { | ||
| return FileUtil.isBibFile(path) || FileUtil.isShortcutFile(path); |
There was a problem hiding this comment.
Condition wrong - i think, resolving should take place
| return FileUtil.isBibFile(path) || FileUtil.isShortcutFile(path); | |
| return FileUtil.isBibFile(FileUtils.resolveIfShortcut(path)); |
| // Resolve any shortcuts to their targets | ||
| List<Path> resolvedFiles = filesToOpen.stream() | ||
| .map(FileUtil::resolveIfShortcut) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
Add comment that list needs to be modifiable.
|
|
||
| ### Fixed | ||
|
|
||
| - Allowed drag and drop of Windows shortcut (.lnk) files to open libraries [#15036] (https://github.com/JabRef/jabref/issues/15036) |
| import org.jabref.model.entry.LinkedFile; | ||
| import org.jabref.model.entry.field.StandardField; | ||
|
|
||
| import mslinks.ShellLink; |
There was a problem hiding this comment.
@Siedlerchr Some dependency seems to already include mslinks :)
|
Your pull request conflicts with the target branch. Please merge with your code. For a step-by-step guide to resolve merge conflicts, see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line. |
4 participants
Description
Closes #15036
I have implemented support for dragging and dropping Windows shortcut (.lnk) files into JabRef to open libraries. To ensure a seamless user experience and prevent the invalidation of binary data parsing, I added a PowerShell-based resolution step in FileUtil that follows the shortcut to its target .bib file before the application attempts to open it. This allows users to manage their libraries using native Windows shortcuts without encountering empty file errors.
Steps to test
Create a .bib file on your desktop (e.g., test.bib).
Right-click the file and select Create shortcut.
Open JabRef and drag the newly created .lnk file onto the main window.
Observation: You should see the "Open files..." indicator appear, and upon dropping, the library should open with all entries visible.
Mandatory checks
[x] I own the copyright of the code submitted and I license it under the MIT license
[x] I manually tested my changes in running JabRef (always required)
[/] I added JUnit tests for changes (if applicable)
[x] I added screenshots in the PR description (if change is visible to the user)
[x] I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number.
[x] I described the change in CHANGELOG.md in a way that is understandable for the average user (if change is visible to the user)
[x] I checked the user documentation: Is the information available and up to date?