[AhmedYoussef98]

Purpose of the pull request

Closed: #1169

What's changed?

• CompatibilityTest#readWithCacheAfterTempDeletion now deletes only Fesod's own temp directory (FileUtils.getTempFilePrefix()) and no longer deletes the whole java.io.tmpdir. That is still what the test means to check: Fesod recovers when its temp and cache files are removed while idle. The system temp directory is shared with other processes, so a test must never delete it.
• FileUtils.delete now deletes a symbolic link itself and does not follow it. Before, File.isDirectory()/listFiles() resolved links, so the recursive delete could leave the directory it was given. On macOS, an Applications -> /Applications link inside a mounted installer DMG under $TMPDIR led it to wipe application bundles (details in [Bug] CompatibilityTest deletes the whole system temp dir, and FileUtils.delete follows symlinks out of it #1169).
• Adds FileUtilsTest#test_delete_doesNotFollowSymbolicLinks. It fails on current main (the link target's contents are deleted) and passes with this change. It is skipped where symbolic links can't be created.

Verified with ./mvnw package -Dmaven.test.skip=false -Dtest='FileUtilsTest,CompatibilityTest' -pl fesod-common,fesod-shaded,fesod-sheet (27 tests, all passing) and ./mvnw spotless:check.

Checklist

• I have read the Contributor Guide.
• I have written the necessary doc or comment.
• I have added the necessary unit tests and all cases have passed.

[skytin1004]

Hi @AhmedYoussef98, while checking this on Windows, I noticed that it only catches symbolic links, not directory junctions. I tried deleting a directory containing a junction that pointed outside it, and FileUtils.delete() followed the junction and deleted the file in the target directory. Files.isSymbolicLink() returns false for the junction.

I think junctions should be treated as leaf entries before the isDirectory() branch so we never recurse into their targets. It would also be good to add a Windows-only test that creates a junction to an external directory and verifies that the target file is left untouched.

[Mikkey-f]

Hi @AhmedYoussef98 — I reproduced @skytin1004's junction finding on Windows and can confirm it: for a directory junction, Files.isSymbolicLink() returns false, so the current check doesn't catch it and the recursion walks through the junction.

I also verified the two pieces the fix needs, on Windows 11 + JDK 17:

• Detection: read with LinkOption.NOFOLLOW_LINKS — a junction reports isDirectory() = true and isOther() = true, while a regular directory reports isOther() = false. So attrs.isSymbolicLink() || attrs.isOther() identifies both symbolic links and junctions for the leaf-case treatment skytin1004 described.
• Deletion: File.delete() on a junction removes the junction itself and leaves the target untouched, so the existing file.delete(); return; shape works for both.

Red/green on the same layout (a directory containing a junction to an external folder with a keep.txt):

• current main: target file deleted
• this PR's patch: target file deleted (junction not caught)
• with the isOther() leaf check: target file survives

A Windows-only test is practical if you want one: mklink /J (junctions) doesn't require admin rights, so the test can create one via cmd /c and assert the target is untouched, skipped elsewhere.

If you don't have the bandwidth for this right now, I'd be glad to take it forward from here — your commit stays with your authorship — but no pressure either way; happy to just leave the findings here.

[Mikkey-f]

Hi @AhmedYoussef98 — following up on my note above.

There has been no update since @skytin1004's CHANGES_REQUESTED on Oct 4, and no reply to my offer on Oct 15, so I'll take this forward now to close the junction gap.

Plan, keeping your work credited:

• cherry-pick your commit 8b851ef7 onto current main with your authorship intact
• add the junction guard: read attributes with LinkOption.NOFOLLOW_LINKS and treat isSymbolicLink() || isOther() as leaf entries. On Windows 11 + JDK 17 I verified that a junction reports isDirectory=true, isOther=true (a regular directory reports isOther=false), and that File.delete() on a junction removes the junction itself and leaves the target untouched
• keep your POSIX symlink test, and add a Windows-only junction regression test created via mklink /J (no admin required), skipped where junctions can't be created
• update CompatibilityTest exactly as you did, and open a new PR referencing this one and [Bug] CompatibilityTest deletes the whole system temp dir, and FileUtils.delete follows symlinks out of it #1169

The new PR will note that it supersedes this one; nothing is needed from you. If you would rather finish it yourself, just say so and I'll hand it back.

[Mikkey-f]

Hi @AhmedYoussef98 — the new PR is up: #1248. It carries your commit with your authorship intact, plus the Windows junction guard and the mklink /J regression test discussed above. Thanks @skytin1004 for the junction analysis.

I'll leave this PR open for the maintainers to close once #1248 lands. Thanks again for the original fix and the detailed issue report.