Fix: Make FileUtil.relativize symlink-aware #13553

Muskan244 · 2025-07-18T09:50:13Z

This PR to make FileUtil.relativize symlink-aware by:

Updating FileUtil::relativize to handle symlinks when resolving relative paths.
Ensuring files inside symlinked directories are correctly referenced by their relative path.
Adding unit tests to verify correct behavior with symlinked and regular directories.
Adding ignored test for symlink escape scenario (/lib/things -> /share/expensive -> /veryprivate).

Steps to test

Build JabRef and run the full test suite:
Gradle: ./gradlew test
Ensure all tests in FileUtilTest related to symlink handling pass without errors.

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
[/] Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
[/] Screenshots added in PR description (if change is visible to the user)
[/] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
[/] Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

…ion and OpenSelectedEntriesFilesAction - Introduced OpenSingleExternalFileAction to handle single entry with one linked file - Introduced OpenSelectedEntriesFilesAction to handle multiple selection cases Fixes JabRef#13431

…laining code

…13505) Bumps [org.junit.platform:junit-platform-launcher](https://github.com/junit-team/junit-framework) from 1.13.1 to 1.13.3. - [Release notes](https://github.com/junit-team/junit-framework/releases) - [Commits](https://github.com/junit-team/junit-framework/commits) --- updated-dependencies: - dependency-name: org.junit.platform:junit-platform-launcher dependency-version: 1.13.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>

…ional fallback

…resolution - Use real/canonical paths to ensure files are correctly recognized as within a directory, even through symlinks - Add and document unit tests covering symlinked and normal directory scenarios fixes JabRef#12995

jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java

…Path and remove unnecessary comments

…bref into fix-for-issue-12995

Muskan244 · 2025-07-20T05:57:21Z

Hi! I’ve pushed the requested changes, and it looks like all checks have passed. Just checking in to see if there's anything more I should do. Thanks again for guiding me through this!

koppor

I don't see a test I've put in the issue description.

I am not sture if the algorithm works. I am not sure how symlinks are really ersolved on Linux on edge cases.

jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java

… simplicity, and ensured all edge cases from issue JabRef#12995 are now covered.

…bref into fix-for-issue-12995

koppor · 2025-08-17T10:33:10Z

Note that there is no test for #12995 (comment) - it would be OK to have it ignored. Not sure how important this is, but it should be noted down somewhere. Maybe a link js enough.

Muskan244 · 2025-08-18T07:05:55Z

Hi @koppor, Do you suggest we add this as an ignored test and just link to your comment in #12995, or, try to change FileUtil.relativize to explicitly detect and prevent such escapes? Just want to be sure.

koppor · 2025-08-18T07:27:13Z

Hi @koppor, Do you suggest we add this as an ignored test and just link to your comment in #12995, or, try to change FileUtil.relativize to explicitly detect and prevent such escapes? Just want to be sure.

The former is enough.

koppor · 2025-08-19T10:25:31Z

I used RefactoringMiner to review

The logic diff is as follows

And test case

There were some syntax changes; I try to look. Refs JabRef#672.

koppor · 2025-08-19T12:54:45Z

@Muskan244 Why didn't you add a CHANGELOG.md entry?!

trag-bot · 2025-08-19T12:58:39Z

jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java

+    private static Optional<Path> tryRealPath(Path path) {
+        if (Files.exists(path)) {
+            try {
+                return Optional.of(path.toRealPath());
+            } catch (IOException e) {
+                return Optional.empty();
+            }


The method catches IOException and returns Optional.empty() instead of letting the exception propagate. This uses exceptions for control flow which is against best practices.

trag-bot · 2025-08-19T13:11:38Z

@trag-bot didn't find any issues in the code! ✅✨

koppor · 2025-08-19T13:59:44Z

Follow-up would be to spleit the parameterized test into seperate test cases - the method producing the arguments is not very readable.

Muskan244 · 2025-08-19T14:47:05Z

Thanks! I’ll prepare a follow-up PR for splitting the parameterized test into separate test cases to make it more readable.

* upstream/main: New translations jabref_en.properties (Italian) (#13725) Minor code style updates (#13722) Fix: Make FileUtil.relativize symlink-aware (#13553) New Crowdin updates (#13720) Bump org.glassfish.jersey.core:jersey-server in /versions (#13714) Enable UseObjectNotifyAll (#13718) Bump com.dlsc.gemsfx:gemsfx from 3.3.5 to 3.4.2 in /versions (#13717) Update on-issue-comment.yml Bump org.openrewrite.recipe:rewrite-recipe-bom from 3.12.1 to 3.13.0 (#13716) Bump org.openrewrite.rewrite from 7.12.1 to 7.14.0 (#13715) Bump org.glassfish.jersey.inject:jersey-hk2 in /versions (#13713) feat(git): add “Share to GitHub” flow (#13677) Bump jablib/src/main/resources/csl-styles from `292aec3` to `1194364` (#13712) Bump jablib/src/main/abbrv.jabref.org from `cfe719f` to `a97f9c6` (#13711) Bump jablib/src/main/resources/csl-locales from `e2de1e3` to `fa56de1` (#13710) Add noop Git Config System Reader to prevent usage of real world stuff into jgit (#13703) Added static (stream & preferences) constructors to BibDatabaseContext (#13694) New Crowdin updates (#13698) fix git modules requires and uses (#13696) Focus "Specify Bib(La)TeX" when Bib(La)TeX is in clipboard (#13633)

Muskan244 and others added 13 commits July 8, 2025 13:23

Merge branch 'main' into fix-for-issue-13431

91da0cf

Cleaned up constant name, dialog text, and removed extra comment

bcf3b76

Merge branch 'main' into fix-for-issue-13431

a8a1235

Add null checks to OpenSingleExternalFileAction constructor

6ccf927

Add @nonnull annotations and fix localization key warnings

4dd6694

Merge branch 'main' into fix-for-issue-13431

90233e4

fix submodules

dc0d67f

Use database latest context, update button text, and add comments exp…

dd9ca53

…laining code

Merge branch 'main' into fix-for-issue-13431

87b7ca9

Fix: clarify fallback logic, update dialog labels, and avoid null Opt…

77fd7a1

…ional fallback

Muskan244 closed this Jul 18, 2025

Muskan244 deleted the fix-for-issue-12995 branch July 18, 2025 09:54

Muskan244 restored the fix-for-issue-12995 branch July 18, 2025 09:55

remove unnecessary comments and make test more clearer and maintainable

22522dc

Muskan244 reopened this Jul 19, 2025

Merge branch 'main' into fix-for-issue-12995

9f38281

trag-bot bot reviewed Jul 19, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java Outdated Show resolved Hide resolved

trag-bot bot reviewed Jul 19, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java Outdated Show resolved Hide resolved

trag-bot bot reviewed Jul 19, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java Outdated Show resolved Hide resolved

trag-bot bot reviewed Jul 19, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java Outdated Show resolved Hide resolved

Muskan244 added 2 commits July 19, 2025 10:57

Refactor to avoid using exceptions for normal control flow in tryReal…

8c9257d

…Path and remove unnecessary comments

Merge branch 'fix-for-issue-12995' of https://github.com/Muskan244/ja…

bb3b3b2

…bref into fix-for-issue-12995

Merge branch 'main' into fix-for-issue-12995

b394a3c

koppor mentioned this pull request Jul 20, 2025

FileUtil::relativize doesn't take symlinks into account #12995

Closed

2 tasks

koppor requested changes Jul 20, 2025

View reviewed changes

jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java Outdated Show resolved Hide resolved

jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java Outdated Show resolved Hide resolved

Refactored symlink tests for independence, used Files.writeString for…

1d236aa

… simplicity, and ensured all edge cases from issue JabRef#12995 are now covered.

Muskan244 and others added 4 commits August 17, 2025 15:40

Merge branch 'fix-for-issue-12995' of https://github.com/Muskan244/ja…

e90cafb

…bref into fix-for-issue-12995

Merge branch 'main' into fix-for-issue-12995

9e1952d

Removed unneccesary comment

618e12a

Merge branch 'fix-for-issue-12995' of https://github.com/Muskan244/ja…

3f88a1d

…bref into fix-for-issue-12995

Add ignored test for symlink escape case (JabRef#12995 comment)

19d7cb3

koppor mentioned this pull request Aug 19, 2025

Highlight should higlight all removed lettrs tsantalis/RefactoringMiner#887

Closed

Format complete file

bf94ce8

koppor added 4 commits August 19, 2025 14:56

Add CHANGELOG.md entry

77985e1

Simplify code

9814427

Have the simple case first

da97e5f

Consistent method naming

5f12089

trag-bot bot reviewed Aug 19, 2025

View reviewed changes

koppor added 2 commits August 19, 2025 15:01

Warn samehow if real path conversion fails

824734f

Refine link

cf2bc0f

koppor approved these changes Aug 19, 2025

View reviewed changes

koppor enabled auto-merge August 19, 2025 13:10

koppor added this pull request to the merge queue Aug 19, 2025

Merged via the queue into JabRef:main with commit d06248f Aug 19, 2025
1 check passed

Muskan244 deleted the fix-for-issue-12995 branch August 28, 2025 18:24

Muskan244 mentioned this pull request Aug 28, 2025

Split relativizeSymlinks parameterized tests in separate tests #13782

Merged

3 tasks

Uh oh!

Fix: Make FileUtil.relativize symlink-aware #13553

Fix: Make FileUtil.relativize symlink-aware #13553

Uh oh!

Conversation

Muskan244 commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Steps to test

Mandatory checks

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Muskan244 commented Jul 20, 2025

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

koppor commented Aug 17, 2025

Uh oh!

Muskan244 commented Aug 18, 2025

Uh oh!

koppor commented Aug 18, 2025

Uh oh!

koppor commented Aug 19, 2025

Uh oh!

koppor commented Aug 19, 2025

Uh oh!

trag-bot bot Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot commented Aug 19, 2025

Uh oh!

Uh oh!

koppor commented Aug 19, 2025

Uh oh!

Muskan244 commented Aug 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Muskan244 commented Jul 18, 2025 •

edited

Loading