From 5c5209a569b8ced42064fa200515c0528638db35 Mon Sep 17 00:00:00 2001 From: AhmedYoussef98 Date: Wed, 30 Sep 2026 16:05:36 +0300 Subject: [PATCH 1/2] fix: stop tests from deleting the system temp dir and FileUtils.delete from following symlinks --- .../apache/fesod/sheet/util/FileUtils.java | 7 +++++ .../fesod/sheet/format/CompatibilityTest.java | 6 ++-- .../fesod/sheet/util/FileUtilsTest.java | 29 +++++++++++++++++++ 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FileUtils.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FileUtils.java index eecc83758..452793fb3 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FileUtils.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FileUtils.java @@ -32,6 +32,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.file.Files; import java.util.UUID; import org.apache.fesod.common.util.IoUtils; import org.apache.fesod.sheet.exception.ExcelAnalysisException; @@ -188,10 +189,16 @@ public static File createDirectory(File directory) { /** * delete file + *

+ * Symbolic links are deleted themselves and never followed, so the deletion cannot escape the given directory. * * @param file */ public static void delete(File file) { + if (Files.isSymbolicLink(file.toPath())) { + file.delete(); + return; + } if (file.isFile()) { file.delete(); return; diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/format/CompatibilityTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/format/CompatibilityTest.java index 0ecd7d3d9..7518e5657 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/format/CompatibilityTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/format/CompatibilityTest.java @@ -40,7 +40,6 @@ import org.apache.fesod.sheet.testkit.builders.TestDataBuilder; import org.apache.fesod.sheet.testkit.models.SimpleData; import org.apache.fesod.sheet.util.FileUtils; -import org.apache.poi.util.TempFile; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -139,10 +138,11 @@ public void readWithCacheAfterTempDeletion() throws Exception { FesodSheet.read(file).readCache(new Ehcache(null, 20)).sheet().doReadSync(); Assertions.assertEquals(10L, list.size()); - // Save file content before deleting the system temp dir (which also removes @TempDir) + // Save file content before deleting Fesod's temp dir byte[] fileContent = Files.readAllBytes(file.toPath()); - FileUtils.delete(new File(System.getProperty(TempFile.JAVA_IO_TMPDIR))); + // Only delete Fesod's own temp dir, never the whole system temp dir shared with other processes + FileUtils.delete(new File(FileUtils.getTempFilePrefix())); // Recreate the file after temp dir deletion to continue the test file.getParentFile().mkdirs(); diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/FileUtilsTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/FileUtilsTest.java index 01fad8edf..ccedcb8eb 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/FileUtilsTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/FileUtilsTest.java @@ -27,6 +27,7 @@ import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.util.UUID; import org.apache.fesod.sheet.exception.ExcelAnalysisException; @@ -36,6 +37,7 @@ import org.apache.poi.util.TempFileCreationStrategy; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -270,6 +272,33 @@ void test_delete_directory() { Assertions.assertFalse(root.exists()); } + @Test + void test_delete_doesNotFollowSymbolicLinks() throws IOException { + // outside/ + // - keep.txt + // root/ + // - link -> outside/ + File outside = tempDir.resolve("outside").toFile(); + FileUtils.createDirectory(outside); + File keep = new File(outside, "keep.txt"); + Files.write(keep.toPath(), "content".getBytes()); + + File root = tempDir.resolve("root").toFile(); + FileUtils.createDirectory(root); + Path link = root.toPath().resolve("link"); + try { + Files.createSymbolicLink(link, outside.toPath()); + } catch (IOException | UnsupportedOperationException e) { + Assumptions.assumeTrue(false, "Symbolic links are not supported: " + e.getMessage()); + } + + FileUtils.delete(root); + + Assertions.assertFalse(root.exists()); + Assertions.assertFalse(Files.exists(link, LinkOption.NOFOLLOW_LINKS)); + Assertions.assertTrue(keep.exists()); + } + @Test void test_GetterSetter() { String originalPrefix = FileUtils.getTempFilePrefix(); From 3433cc01e82fdb0ddfc2814fb52ef10788e9c831 Mon Sep 17 00:00:00 2001 From: Mikkey-f <178465103+Mikkey-f@users.noreply.github.com> Date: Tue, 20 Oct 2026 23:40:19 +0800 Subject: [PATCH 2/2] fix: treat directory junctions as leaves when deleting (#1169) FileUtils.delete inspected the path through the File API, so a Windows directory junction was seen as a directory and the recursive delete followed it into the target. Read BasicFileAttributes with NOFOLLOW_LINKS and delete symbolic links and junctions as leaf entries instead. Add a Windows-only regression test that creates a junction with mklink /J and verifies the target is left untouched. --- .../apache/fesod/sheet/util/FileUtils.java | 19 ++++++-- .../fesod/sheet/util/FileUtilsTest.java | 47 +++++++++++++++++++ 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FileUtils.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FileUtils.java index 452793fb3..39ea501e6 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FileUtils.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FileUtils.java @@ -33,6 +33,8 @@ import java.io.InputStream; import java.io.OutputStream; import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.attribute.BasicFileAttributes; import java.util.UUID; import org.apache.fesod.common.util.IoUtils; import org.apache.fesod.sheet.exception.ExcelAnalysisException; @@ -190,20 +192,29 @@ public static File createDirectory(File directory) { /** * delete file *

- * Symbolic links are deleted themselves and never followed, so the deletion cannot escape the given directory. + * Symbolic links and directory junctions are deleted themselves and never followed, so the deletion cannot + * escape the given directory. * * @param file */ public static void delete(File file) { - if (Files.isSymbolicLink(file.toPath())) { + BasicFileAttributes attributes; + try { + attributes = Files.readAttributes(file.toPath(), BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS); + } catch (IOException e) { + return; + } + // Read attributes without following links: File#isDirectory() resolves a Windows directory junction, while + // "no follow" attributes report it as "other" (a regular directory reports isOther() = false). + if (attributes.isSymbolicLink() || attributes.isOther()) { file.delete(); return; } - if (file.isFile()) { + if (attributes.isRegularFile()) { file.delete(); return; } - if (file.isDirectory()) { + if (attributes.isDirectory()) { File[] childFiles = file.listFiles(); if (childFiles == null || childFiles.length == 0) { file.delete(); diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/FileUtilsTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/FileUtilsTest.java index ccedcb8eb..5b1860772 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/FileUtilsTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/FileUtilsTest.java @@ -30,6 +30,7 @@ import java.nio.file.LinkOption; import java.nio.file.Path; import java.util.UUID; +import java.util.concurrent.TimeUnit; import org.apache.fesod.sheet.exception.ExcelAnalysisException; import org.apache.fesod.sheet.exception.ExcelCommonException; import org.apache.fesod.sheet.testkit.Tags; @@ -299,6 +300,52 @@ void test_delete_doesNotFollowSymbolicLinks() throws IOException { Assertions.assertTrue(keep.exists()); } + @Test + void test_delete_doesNotFollowDirectoryJunctions() throws IOException { + Assumptions.assumeTrue(isWindows(), "Directory junctions are only available on Windows"); + // outside/ + // - keep.txt + // root/ + // - junction -> outside/ + File outside = tempDir.resolve("junction-outside").toFile(); + FileUtils.createDirectory(outside); + File keep = new File(outside, "keep.txt"); + Files.write(keep.toPath(), "content".getBytes(StandardCharsets.UTF_8)); + + File root = tempDir.resolve("junction-root").toFile(); + FileUtils.createDirectory(root); + Path junction = root.toPath().resolve("junction"); + Assumptions.assumeTrue(createJunction(junction, outside), "Directory junctions are not supported"); + + FileUtils.delete(root); + + Assertions.assertFalse(root.exists()); + Assertions.assertFalse(Files.exists(junction, LinkOption.NOFOLLOW_LINKS)); + Assertions.assertTrue(keep.exists()); + } + + private static boolean isWindows() { + return System.getProperty("os.name").startsWith("Windows"); + } + + private static boolean createJunction(Path link, File target) { + try { + Process process = new ProcessBuilder("cmd", "/c", "mklink", "/J", link.toString(), target.getAbsolutePath()) + .redirectErrorStream(true) + .start(); + if (!process.waitFor(30, TimeUnit.SECONDS)) { + process.destroyForcibly(); + return false; + } + return process.exitValue() == 0; + } catch (IOException e) { + return false; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return false; + } + } + @Test void test_GetterSetter() { String originalPrefix = FileUtils.getTempFilePrefix();