This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-io.git
commit 0be56985e1e7d42f9ad4396e94a735530e21a305
Author: Gary Gregory <garydgreg...@gmail.com>
AuthorDate: Thu Nov 14 10:53:17 2024 -0500
[IO-862] FileUtils.deleteDirectory fails for a directory containing a
broken symbolic link
---
src/changes/changes.xml | 1 +
src/main/java/org/apache/commons/io/FileUtils.java | 55 +++++++++++++++++-----
.../java/org/apache/commons/io/FileUtilsTest.java | 28 +++++++++--
3 files changed, 67 insertions(+), 17 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 84e0b4bd5..7468e3ee7 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -57,6 +57,7 @@ The <action> type attribute can be add,update,fix,remove.
<action dev="ggregory" type="fix" issue="IO-859" due-to="JD Dean, Gary
Gregory">FileUtils.forceDelete on non-existent file on Windows throws
IOException rather than FileNotFoundException.</action>
<action dev="ggregory" type="fix" due-to="Éamonn
McManus">Use Unicode escapes for superscript characters. #701.</action>
<action dev="ggregory" type="fix" issue="IO-863" due-to="Éamonn McManus,
Gary Gregory">Recent incompatible change to FileUtils.listFiles re extensions,
see IO-856.</action>
+ <action dev="ggregory" type="fix" issue="IO-862" due-to="Éamonn McManus,
Gary Gregory">FileUtils.deleteDirectory fails for a directory containing a
broken symbolic link.</action>
<!-- ADD -->
<action dev="ggregory" type="add" due-to="Gary
Gregory">Add @FunctionalInterface to ClassNameMatcher.</action>
<action dev="ggregory" type="add" due-to="Gary
Gregory">Add ValidatingObjectInputStream.Builder and
ValidatingObjectInputStream.builder().</action>
diff --git a/src/main/java/org/apache/commons/io/FileUtils.java
b/src/main/java/org/apache/commons/io/FileUtils.java
index 55a717c6b..63345f113 100644
--- a/src/main/java/org/apache/commons/io/FileUtils.java
+++ b/src/main/java/org/apache/commons/io/FileUtils.java
@@ -285,19 +285,28 @@ public class FileUtils {
}
/**
- * Requires that the given {@link File} exists, and throws a {@link
FileNotFoundException} if it doesn't.
+ * Requires that the given {@link File} is non-null and exists (if strict
is true).
*
* @param file The {@link File} to check.
- * @throws FileNotFoundException if the file does not exist
+ * @param strict whether to check that the {@code file} exists.
+ * @throws FileNotFoundException if the file does not exist.
* @throws NullPointerException if the given {@link File} is {@code null}.
*/
- private static void checkExists(final File file) throws
FileNotFoundException {
- Objects.requireNonNull(file, "file");
- if (!file.exists()) {
+ private static void checkExists(final File file, final boolean strict)
throws FileNotFoundException {
+ Objects.requireNonNull(file, PROTOCOL_FILE);
+ if (strict && !file.exists()) {
throw new FileNotFoundException(file.toString());
}
}
+ /**
+ * Requires that the given {@link File} exists, and throws a {@link
FileNotFoundException} if it doesn't.
+ *
+ * @param file The {@link File} to check.
+ * @param name The NullPointerException message.
+ * @throws FileNotFoundException if the file does not exist.
+ * @throws NullPointerException if the given {@link File} is {@code null}.
+ */
private static void checkFileExists(final File file, final String name)
throws FileNotFoundException {
Objects.requireNonNull(file, name);
if (!file.isFile()) {
@@ -350,8 +359,8 @@ public class FileUtils {
*
* @param file the file to checksum, must not be {@code null}
* @return the checksum value
- * @throws NullPointerException if the given {@link File} is {@code null}.
- * @throws IllegalArgumentException if the given {@link File} does not
exist or is not a file.
+ * @throws NullPointerException if the {@code file} is {@code null}.
+ * @throws IllegalArgumentException if the {@code file} does not exist or
is not a file.
* @throws IOException if an IO error occurs reading the file.
* @since 1.3
*/
@@ -364,12 +373,12 @@ public class FileUtils {
*
* @param directory directory to clean
* @throws NullPointerException if the given {@link File} is {@code null}.
- * @throws IllegalArgumentException if directory does not exist or is not
a directory.
+ * @throws IllegalArgumentException if the {@code directory} does not
exist or is not a directory.
* @throws IOException if an I/O error occurs.
* @see #forceDelete(File)
*/
public static void cleanDirectory(final File directory) throws IOException
{
- IOConsumer.forAll(FileUtils::forceDelete, listFiles(directory, null));
+ IOConsumer.forAll(f -> forceDelete(f, false), listFiles(directory,
null));
}
/**
@@ -377,7 +386,7 @@ public class FileUtils {
*
* @param directory directory to clean, must not be {@code null}
* @throws NullPointerException if the given {@link File} is {@code null}.
- * @throws IllegalArgumentException if directory does not exist or is not
a directory.
+ * @throws IllegalArgumentException if the {@code directory} does not
exist or is not a directory.
* @throws IOException if an I/O error occurs.
* @see #forceDeleteOnExit(File)
*/
@@ -1386,8 +1395,28 @@ public class FileUtils {
* @throws IOException in case deletion is unsuccessful.
*/
public static void forceDelete(final File file) throws IOException {
+ forceDelete(file, true);
+ }
+
+ /**
+ * Deletes a file or directory. For a directory, delete it and all
subdirectories.
+ * <p>
+ * The difference between File.delete() and this method are:
+ * </p>
+ * <ul>
+ * <li>The directory does not have to be empty.</li>
+ * <li>You get an exception when a file or directory cannot be
deleted.</li>
+ * </ul>
+ *
+ * @param file file or directory to delete, must not be {@code null}.
+ * @param strict whether to throw a FileNotFoundException.
+ * @throws NullPointerException if the file is {@code null}.
+ * @throws FileNotFoundException if the file was not found.
+ * @throws IOException in case deletion is unsuccessful.
+ */
+ private static void forceDelete(final File file, final boolean strict)
throws IOException {
Objects.requireNonNull(file, PROTOCOL_FILE);
- checkExists(file); // fail-fast
+ checkExists(file, strict); // fail-fast
final Counters.PathCounters deleteCounters;
try {
deleteCounters = PathUtils.delete(file.toPath(),
PathUtils.EMPTY_LINK_OPTION_ARRAY, StandardDeleteOption.OVERRIDE_READ_ONLY);
@@ -2265,11 +2294,11 @@ public class FileUtils {
/**
* Lists files in a directory, asserting that the supplied directory
exists and is a directory.
*
- * @param directory The directory to list
+ * @param directory The directory to list.
* @param fileFilter Optional file filter, may be null.
* @return The files in the directory, never {@code null}.
* @throws NullPointerException if directory is {@code null}.
- * @throws IllegalArgumentException if {@link directory} exists but is not
a directory
+ * @throws IllegalArgumentException if {@link directory} exists but is not
a directory.
* @throws IOException if an I/O error occurs.
*/
private static File[] listFiles(final File directory, final FileFilter
fileFilter) throws IOException {
diff --git a/src/test/java/org/apache/commons/io/FileUtilsTest.java
b/src/test/java/org/apache/commons/io/FileUtilsTest.java
index f2f8e422f..4e1ef0b8e 100644
--- a/src/test/java/org/apache/commons/io/FileUtilsTest.java
+++ b/src/test/java/org/apache/commons/io/FileUtilsTest.java
@@ -248,10 +248,10 @@ public class FileUtilsTest extends AbstractTempDirTest {
private ImmutablePair<Path, Path> createTempSymbolicLinkedRelativeDir()
throws IOException {
final Path targetDir = tempDirPath.resolve("subdir");
- final Path symlinkDir = tempDirPath.resolve("symlinked-dir");
+ final Path symLinkedDir = tempDirPath.resolve("symlinked-dir");
Files.createDirectory(targetDir);
- Files.createSymbolicLink(symlinkDir, targetDir);
- return ImmutablePair.of(symlinkDir, targetDir);
+ Files.createSymbolicLink(symLinkedDir, targetDir);
+ return ImmutablePair.of(symLinkedDir, targetDir);
}
private Set<String> getFilePathSet(final List<File> files) {
@@ -318,7 +318,6 @@ public class FileUtilsTest extends AbstractTempDirTest {
public void setUp() throws Exception {
testFile1 = new File(tempDirFile, "file1-test.txt");
testFile2 = new File(tempDirFile, "file1a-test.txt");
-
testFile1Size = testFile1.length();
testFile2Size = testFile2.length();
if (!testFile1.getParentFile().exists()) {
@@ -1586,6 +1585,7 @@ public class FileUtilsTest extends AbstractTempDirTest {
final Path symlinkedDir = pair.getLeft();
final Path targetDir = pair.getRight();
assertTrue(Files.exists(symlinkedDir), symlinkedDir::toString);
+ // remove target directory, keeping symbolic link
Files.delete(targetDir);
assertFalse(Files.exists(targetDir), targetDir::toString);
assertFalse(Files.exists(symlinkedDir), symlinkedDir::toString);
@@ -1594,6 +1594,26 @@ public class FileUtilsTest extends AbstractTempDirTest {
assertFalse(Files.exists(symlinkedDir), symlinkedDir::toString);
}
+ @Test
+ public void testDeleteDirectorySymbolicLinkAbsentDeepTarget() throws
IOException {
+ final ImmutablePair<Path, Path> pair =
createTempSymbolicLinkedRelativeDir();
+ final Path symLinkedDir = pair.getLeft();
+ final Path targetDir = pair.getRight();
+ // more setup
+ final Path targetDir2 = targetDir.resolve("subdir2");
+ final Path symLinkedDir2 = targetDir.resolve("symlinked-dir2");
+ Files.createDirectory(targetDir2);
+ Files.createSymbolicLink(symLinkedDir2, targetDir2);
+ assertTrue(Files.exists(symLinkedDir2), symLinkedDir2::toString);
+ // remove target directory, keeping symbolic link
+ Files.delete(targetDir2);
+ assertFalse(Files.exists(targetDir2), targetDir2::toString);
+ assertFalse(Files.exists(symLinkedDir2), symLinkedDir2::toString);
+ // actual test
+ FileUtils.deleteDirectory(targetDir.toFile());
+ assertFalse(Files.exists(targetDir), targetDir::toString);
+ }
+
@Test
public void testDeleteQuietlyDir() throws IOException {
final File testDirectory = new File(tempDirFile,
"testDeleteQuietlyDir");
