You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2024/11/14 18:29:01 UTC

(commons-io) branch master updated (3a6190485 -> 3fbb4f311)

This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/commons-io.git


    from 3a6190485 [IO-862] Add FileUtils.deleteDirectory(File)
     new 0be56985e [IO-862] FileUtils.deleteDirectory fails for a directory containing a broken symbolic link
     new 644870b74 FileUtils.forceDelete(File) does not chain the exception cause
     new 0390d8124 Remove redundant call to Objects.requireNonNull()
     new e313a622d Javadoc
     new 3fbb4f311 It is OK to call java.io.File.listFiles(FileFilter) with a null argument

The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/changes/changes.xml                            |  1 +
 .../org/apache/commons/io/DirectoryWalker.java     |  2 +-
 src/main/java/org/apache/commons/io/FileUtils.java | 74 ++++++++++++++++------
 .../java/org/apache/commons/io/FileUtilsTest.java  | 28 ++++++--
 4 files changed, 79 insertions(+), 26 deletions(-)


(commons-io) 05/05: It is OK to call java.io.File.listFiles(FileFilter) with a null argument

Posted by gg...@apache.org.
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 3fbb4f311eb4128436e8161fbf03a03230d286f3
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Thu Nov 14 13:28:54 2024 -0500

    It is OK to call java.io.File.listFiles(FileFilter) with a null argument
    
    See the Javadoc
---
 src/main/java/org/apache/commons/io/DirectoryWalker.java | 2 +-
 src/main/java/org/apache/commons/io/FileUtils.java       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/apache/commons/io/DirectoryWalker.java b/src/main/java/org/apache/commons/io/DirectoryWalker.java
index e8a4a2e24..62298a50c 100644
--- a/src/main/java/org/apache/commons/io/DirectoryWalker.java
+++ b/src/main/java/org/apache/commons/io/DirectoryWalker.java
@@ -644,7 +644,7 @@ public abstract class DirectoryWalker<T> {
             final int childDepth = depth + 1;
             if (depthLimit < 0 || childDepth <= depthLimit) {
                 checkIfCancelled(directory, depth, results);
-                File[] childFiles = filter == null ? directory.listFiles() : directory.listFiles(filter);
+                File[] childFiles = directory.listFiles(filter);
                 childFiles = filterDirectoryContents(directory, depth, childFiles);
                 if (childFiles == null) {
                     handleRestricted(directory, childDepth, results);
diff --git a/src/main/java/org/apache/commons/io/FileUtils.java b/src/main/java/org/apache/commons/io/FileUtils.java
index 9b5bcd0b3..d9ce9a3e7 100644
--- a/src/main/java/org/apache/commons/io/FileUtils.java
+++ b/src/main/java/org/apache/commons/io/FileUtils.java
@@ -2306,7 +2306,7 @@ public class FileUtils {
      */
     private static File[] listFiles(final File directory, final FileFilter fileFilter) throws IOException {
         requireDirectoryExists(directory, "directory");
-        final File[] files = fileFilter == null ? directory.listFiles() : directory.listFiles(fileFilter);
+        final File[] files = directory.listFiles(fileFilter);
         if (files == null) {
             // null if the directory does not denote a directory, or if an I/O error occurs.
             throw new IOException("Unknown I/O error listing contents of directory: " + directory);


(commons-io) 01/05: [IO-862] FileUtils.deleteDirectory fails for a directory containing a broken symbolic link

Posted by gg...@apache.org.
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 <ga...@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");


(commons-io) 04/05: Javadoc

Posted by gg...@apache.org.
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 e313a622d41034ec601cf823656c0ae0cddce029
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Thu Nov 14 11:01:28 2024 -0500

    Javadoc
---
 src/main/java/org/apache/commons/io/FileUtils.java | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/main/java/org/apache/commons/io/FileUtils.java b/src/main/java/org/apache/commons/io/FileUtils.java
index b3f19f241..9b5bcd0b3 100644
--- a/src/main/java/org/apache/commons/io/FileUtils.java
+++ b/src/main/java/org/apache/commons/io/FileUtils.java
@@ -2295,12 +2295,14 @@ 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 IOException if an I/O error occurs.
+     * @throws NullPointerException     if the {@code directory} is {@code null}.
+     * @throws IllegalArgumentException if the {@code directory} exists but is not a directory.
+     * @throws IOException              if an I/O error occurs per {@link File#listFiles()} and {@link File#listFiles(FileFilter)}.
+     * @throws SecurityException        If a security manager exists and its {@link SecurityManager#checkRead(String)} method denies read access to the
+     *                                  directory.
      */
     private static File[] listFiles(final File directory, final FileFilter fileFilter) throws IOException {
         requireDirectoryExists(directory, "directory");


(commons-io) 02/05: FileUtils.forceDelete(File) does not chain the exception cause

Posted by gg...@apache.org.
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 644870b748e432d107c759e35c847ffc905fbeb3
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Thu Nov 14 10:56:20 2024 -0500

    FileUtils.forceDelete(File) does not chain the exception cause
---
 src/main/java/org/apache/commons/io/FileUtils.java | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/main/java/org/apache/commons/io/FileUtils.java b/src/main/java/org/apache/commons/io/FileUtils.java
index 63345f113..f65b4f6f6 100644
--- a/src/main/java/org/apache/commons/io/FileUtils.java
+++ b/src/main/java/org/apache/commons/io/FileUtils.java
@@ -1420,11 +1420,13 @@ public class FileUtils {
         final Counters.PathCounters deleteCounters;
         try {
             deleteCounters = PathUtils.delete(file.toPath(), PathUtils.EMPTY_LINK_OPTION_ARRAY, StandardDeleteOption.OVERRIDE_READ_ONLY);
-        } catch (final NoSuchFileException ex) {
+        } catch (final NoSuchFileException e) {
             // Map NIO to IO exception
-            throw new FileNotFoundException("Cannot delete file: " + file);
-        } catch (final IOException ex) {
-            throw new IOException("Cannot delete file: " + file, ex);
+            final FileNotFoundException nioEx = new FileNotFoundException("Cannot delete file: " + file);
+            nioEx.initCause(e);
+            throw nioEx;
+        } catch (final IOException e) {
+            throw new IOException("Cannot delete file: " + file, e);
         }
         if (deleteCounters.getFileCounter().get() < 1 && deleteCounters.getDirectoryCounter().get() < 1) {
             // didn't find a file to delete.


(commons-io) 03/05: Remove redundant call to Objects.requireNonNull()

Posted by gg...@apache.org.
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 0390d812488ce50419cad50d970167f32fd42cfb
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Thu Nov 14 10:56:44 2024 -0500

    Remove redundant call to Objects.requireNonNull()
---
 src/main/java/org/apache/commons/io/FileUtils.java | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/main/java/org/apache/commons/io/FileUtils.java b/src/main/java/org/apache/commons/io/FileUtils.java
index f65b4f6f6..b3f19f241 100644
--- a/src/main/java/org/apache/commons/io/FileUtils.java
+++ b/src/main/java/org/apache/commons/io/FileUtils.java
@@ -1415,7 +1415,6 @@ public class FileUtils {
      * @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, strict); // fail-fast
         final Counters.PathCounters deleteCounters;
         try {