Allow drag and drop of Windows shortcut (.lnk) files #15053
Merged
Merged
Conversation
Contributor
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
|
Contributor
Code Review by Qodo
1.
|
Comment on lines
+548
to
+584
| 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(); | ||
| } |
Contributor
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
Comment on lines
+524
to
+596
| /// 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(); | ||
| } |
Contributor
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
Comment on lines
+524
to
+530
| /// 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(); | ||
| } |
Contributor
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
This comment has been minimized.
This comment has been minimized.
Member
|
There is another java library which apparently can parse this stuf https://github.com/DmitriiShamrikov/mslinks |
Member
|
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. |
Member
Who works on DmitriiShamrikov/mslinks#29 ? |
Member
|
We need the newver version, because of JDK11 compatibility - https://github.com/DmitriiShamrikov/mslinks/releases/tag/1.0.8 |
Collaborator
Author
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. |
Collaborator
Author
|
@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! |
Member
|
recent |
koppor
requested changes
Feb 11, 2026
| } | ||
|
|
||
| private boolean isAcceptedFile(Path path) { | ||
| return FileUtil.isBibFile(path) || FileUtil.isShortcutFile(path); |
Member
There was a problem hiding this comment.
Condition wrong - i think, resolving should take place
Suggested change
| return FileUtil.isBibFile(path) || FileUtil.isShortcutFile(path); | |
| return FileUtil.isBibFile(FileUtils.resolveIfShortcut(path)); |
|
|
||
| ### Fixed | ||
|
|
||
| - Allowed drag and drop of Windows shortcut (.lnk) files to open libraries [#15036] (https://github.com/JabRef/jabref/issues/15036) |
koppor
reviewed
Feb 11, 2026
| import org.jabref.model.entry.LinkedFile; | ||
| import org.jabref.model.entry.field.StandardField; | ||
|
|
||
| import mslinks.ShellLink; |
Member
There was a problem hiding this comment.
@Siedlerchr Some dependency seems to already include mslinks :)
Contributor
|
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. |
Contributor
|
Do not mark a PR as ready-for-review while the CI is running. CI checks need to be completed and passed. |
Contributor
Review Summary by QodoSupport drag and drop of Windows shortcut (.lnk) files
WalkthroughsDescription• Added support for dragging and dropping Windows shortcut (.lnk) files • Implemented shortcut resolution using mslinks library for target path detection • Updated file acceptance logic in drag-and-drop handlers across UI components • Resolves shortcuts before opening libraries to prevent file errors Diagramflowchart LR
A["User drags .lnk file"] --> B["FrameDndHandler accepts .lnk files"]
B --> C["OpenDatabaseAction resolves shortcut"]
C --> D["FileUtil.resolveIfShortcut uses mslinks"]
D --> E["Target .bib file opens in JabRef"]
F["MainTable drag-drop also resolves shortcuts"] --> D
File Changes1. jabgui/src/main/java/org/jabref/gui/frame/FrameDndHandler.java
|
Contributor
|
Do not mark a PR as ready-for-review if changes are required. |
Contributor
|
Persistent review updated to latest commit 510012a |
Comment on lines
+242
to
+244
| private boolean isAcceptedFile(Path path) { | ||
| return FileUtil.isBibFile(path) || FileUtil.isShortcutFile(path); | ||
| } |
Contributor
There was a problem hiding this comment.
1. .lnk accepted on non-windows platforms 🐞 Bug ✓ Correctness
isAcceptedFile() calls FileUtil.isShortcutFile() with no OS guard. On macOS/Linux a dropped .lnk file passes the filter, but resolveIfShortcut() is a no-op there and returns the original .lnk path, which is then fed to the BibTeX parser producing confusing parse errors.
Agent Prompt
## Issue description
`isAcceptedFile()` in `FrameDndHandler` accepts `.lnk` files on all operating systems because `FileUtil.isShortcutFile()` only checks the file extension. On non-Windows systems `resolveIfShortcut()` is a no-op and returns the original `.lnk` binary path, which is then parsed as BibTeX.
## Issue Context
- `FileUtil.isShortcutFile()` has no OS check (extension-only test).
- `FileUtil.resolveIfShortcut()` guards with `!OS.WINDOWS` and returns the original path immediately on non-Windows.
- `openTheFile()` only checks `Files.exists()`, so the `.lnk` binary reaches the parser.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/frame/FrameDndHandler.java[242-244]
Change the method to:
```java
private boolean isAcceptedFile(Path path) {
return FileUtil.isBibFile(path) || (OS.WINDOWS && FileUtil.isShortcutFile(path));
}
```
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+168
to
+171
| // Resolve any shortcuts to their targets | ||
| List<Path> resolvedFiles = filesToOpen.stream() | ||
| .map(FileUtil::resolveIfShortcut) | ||
| .collect(Collectors.toList()); |
Contributor
There was a problem hiding this comment.
2. No .bib validation after shortcut resolution 🐞 Bug ✓ Correctness
After resolving a .lnk shortcut, the code never checks whether the resolved target is a .bib file. A shortcut pointing to a .pdf, .docx, or any other file type is silently forwarded to openTheFile() and parsed as a BibTeX database, producing confusing errors with no explanation.
Agent Prompt
## Issue description
After `resolveIfShortcut()` maps each path to its target, there is no check that the resolved target is actually a `.bib` file. A `.lnk` pointing to a `.pdf` or any other file type will be passed to `openTheFile()` and parsed as BibTeX.
## Issue Context
- The old code used `filter(FileUtil::isBibFile)` in `getBibFiles()` to guarantee only `.bib` files reached `openFiles()`.
- The new code removes that guarantee: `isAcceptedFile()` admits all `.lnk` files, and `openFiles()` does not re-validate after resolution.
- `openTheFile()` only checks `Files.exists()` (line 220), not the file extension.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java[168-171]
Add a `.bib` filter after resolution:
```java
List<Path> resolvedFiles = filesToOpen.stream()
.map(FileUtil::resolveIfShortcut)
.filter(FileUtil::isBibFile) // ← ensure resolved target is a .bib file
.collect(Collectors.toList());
```
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+556
to
+561
| } catch (IOException | ShellLinkException e) { | ||
| LOGGER.warn("Could not resolve shortcut {}", path, e); | ||
| } | ||
|
|
||
| return path; | ||
| } |
Contributor
There was a problem hiding this comment.
3. Failed resolution falls back to parsing .lnk binary 🐞 Bug ⛯ Reliability
When resolveIfShortcut() fails (exception thrown, or the shortcut target does not exist on disk), it returns the original .lnk path. That path passes openTheFile()'s Files.exists() guard (the .lnk file itself exists), so the binary .lnk content is fed to the BibTeX parser instead of surfacing a clear "shortcut target not found" error. Broken/orphaned shortcuts are common in real-world Windows usage.
Agent Prompt
## Issue description
When `resolveIfShortcut()` cannot resolve a shortcut (broken link, missing target, or parse exception), it returns the original `.lnk` path. This path passes `openTheFile()`'s `Files.exists()` check and the binary `.lnk` content is parsed as BibTeX, producing confusing errors instead of a clear failure message.
## Issue Context
- `resolveIfShortcut()` returns the original path on exception (line 556-560) and when the resolved target does not exist (line 552-554).
- The `.lnk` file itself exists on disk, so `openTheFile()`'s `Files.exists()` guard (line 220) does not filter it out.
- Broken shortcuts (target deleted or moved) are common in real-world Windows usage.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[549-561]
- jabgui/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java[168-171]
Preferred fix — add `filter(FileUtil::isBibFile)` after resolution in `openFiles()` (covers both this issue and Finding 2):
```java
List<Path> resolvedFiles = filesToOpen.stream()
.map(FileUtil::resolveIfShortcut)
.filter(FileUtil::isBibFile)
.collect(Collectors.toList());
```
Alternatively, change `resolveIfShortcut()` to return `Optional<Path>` so callers can explicitly handle resolution failure.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Member
|
Hi @faneeshh, can we get an update on this PR? There seems to be unresolved comments. |
Collaborator
Author
|
Ahh I think I accidentally pulled all upstream commits. Let me fix that. |
9e70c9d to
6c9ade4
Compare
This comment has been minimized.
This comment has been minimized.
Member
|
just do git fetch upstream && git merge upstream |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
koppor
reviewed
Mar 16, 2026
This comment has been minimized.
This comment has been minimized.
koppor
reviewed
Mar 16, 2026
Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
✅ All tests passed ✅🏷️ Commit: 9090384 Learn more about TestLens at testlens.app. |
koppor
approved these changes
Mar 16, 2026
koppor
left a comment
Member
There was a problem hiding this comment.
Thank you for the patience.
I tried it out. Works.
Will check the depencency later.
Comment on lines
+435
to
+448
| @Test | ||
| void isShortcutFileReturnsTrueForPathEndingInLnk() { | ||
| assertTrue(FileUtil.isShortcutFile(Path.of("test.lnk"))); | ||
| } | ||
|
|
||
| @Test | ||
| void isShortcutFileReturnsTrueForPathEndingInLnkCaseInsensitive() { | ||
| assertTrue(FileUtil.isShortcutFile(Path.of("test.LNK"))); | ||
| } | ||
|
|
||
| @Test | ||
| void isShortcutFileReturnsFalseForPathEndingInBib() { | ||
| assertFalse(FileUtil.isShortcutFile(Path.of("test.bib"))); | ||
| } |
Member
There was a problem hiding this comment.
These three could have been @ParamterizedTest with @ValueSource), but there should not be any more delay in getting this in.
| } | ||
|
|
||
| @Test | ||
| @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Shortcut resolution only supported on Windows") |
Member
There was a problem hiding this comment.
Not sure why the full import is needed.
Member
There was a problem hiding this comment.
OS.Windows of jabref and junit collide otherwise in this class
AnvitaPrasad
pushed a commit
to AnvitaPrasad/jabref
that referenced
this pull request
Mar 18, 2026
* Allow drag and drop of Windows shortcut (.lnk) files Fixes JabRef#15036 * Specified the issue in ChangeLog * fix(dnd): resolve .lnk shortcut targets in MainTable and FileUtil * Resolved Windows shortcuts in MainTable drag and drop * maint: revoled merge conflict in CHANGELOG * Added the recommended fixes and validations * Added tests * Changelog grammar * Apply suggestion from @koppor * Apply suggestions from code review Co-authored-by: Oliver Kopp <kopp.dev@gmail.com> --------- Co-authored-by: Subhramit Basu <subhramit.bb@live.in> Co-authored-by: Christoph <siedlerkiller@gmail.com> Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
FynnianB
pushed a commit
to FynnianB/jabref
that referenced
this pull request
Mar 19, 2026
* Allow drag and drop of Windows shortcut (.lnk) files Fixes JabRef#15036 * Specified the issue in ChangeLog * fix(dnd): resolve .lnk shortcut targets in MainTable and FileUtil * Resolved Windows shortcuts in MainTable drag and drop * maint: revoled merge conflict in CHANGELOG * Added the recommended fixes and validations * Added tests * Changelog grammar * Apply suggestion from @koppor * Apply suggestions from code review Co-authored-by: Oliver Kopp <kopp.dev@gmail.com> --------- Co-authored-by: Subhramit Basu <subhramit.bb@live.in> Co-authored-by: Christoph <siedlerkiller@gmail.com> Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
Member
|
Does not work on latest main - I reopened the issue. |
Member
|
Update: Works -- if the shortcut points to an existing file |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues and pull requests
Closes #15036
PR Description
JabRef already opens .lnk files but dragging and dropping them onto the main window wasn't working because the shortcut file itself was being passed to the BibTeX parser instead of the target .bib file. This fix resolves .lnk shortcuts using the mslinks library before passing them to the file open logic.
Steps to test
2026-02-27.13-35-50.mp4
Checklist
CHANGELOG.mdin a way that can be understood by the average user (if change is visible to the user)