Commons IO
  1. Commons IO
  2. IO-22

[io] FileUtils.cleanDirectory throws NullPointerException when File.list() fails

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0
    • Fix Version/s: 1.1
    • Component/s: None
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

      Description

      java.io.File.list() returns null if an I/O error occurs. In practice this can
      be triggered by trying to list the contents of a directory to which the user
      does not have sufficient permissions (e.g. a directory with mode 0 on a
      unix-like filesystem).

      In this case, FileUtils.cleanDirectory currently throws NPE because it assumes
      that File.list() will always return successfully. The correct behavior should
      be to throw IOException instead.

      Following inline is a patch against trunk which resolves the issue.

      Index: src/test/org/apache/commons/io/FileUtilsCleanDirectoryTestCase.java
      ===================================================================
      — src/test/org/apache/commons/io/FileUtilsCleanDirectoryTestCase.java (revision 0)
      +++ src/test/org/apache/commons/io/FileUtilsCleanDirectoryTestCase.java (revision 0)
      @@ -0,0 +1,143 @@
      +/*
      + * Copyright 2003,2004 The Apache Software Foundation.
      + *
      + * Licensed under the Apache License, Version 2.0 (the "License");
      + * you may not use this file except in compliance with the License.
      + * You may obtain a copy of the License at
      + *
      + * http://www.apache.org/licenses/LICENSE-2.0
      + *
      + * Unless required by applicable law or agreed to in writing, software
      + * distributed under the License is distributed on an "AS IS" BASIS,
      + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
      + * See the License for the specific language governing permissions and
      + * limitations under the License.
      + */
      +package org.apache.commons.io;
      +
      +import java.util.ArrayList;
      +import java.util.List;
      +
      +import java.io.File;
      +import java.io.IOException;
      +
      +import org.apache.commons.io.testtools.FileBasedTestCase;
      +
      +/**
      + * Test cases for FileUtils.cleanDirectory() method.
      + */
      +public class FileUtilsCleanDirectoryTestCase extends FileBasedTestCase {
      + final File top = getLocalTestDirectory();
      +
      + public FileUtilsCleanDirectoryTestCase(String name)

      { + super(name); + }

      +
      + private File getLocalTestDirectory()

      { + return new File(getTestDirectory(), "list-files"); + }

      +
      + /**
      + * @see junit.framework.TestCase#setUp()
      + */
      + protected void setUp() throws Exception

      { + top.mkdirs(); + }

      +
      + /**
      + * @see junit.framework.TestCase#tearDown()
      + */
      + protected void tearDown() throws Exception

      { + chmod(top, 775, true); + FileUtils.deleteDirectory(top); + }

      +
      + public void testCleanEmpty() throws Exception

      { + assertEquals(0, top.list().length); + + FileUtils.cleanDirectory(top); + + assertEquals(0, top.list().length); + }

      +
      + public void testDeletesRegular() throws Exception

      { + FileUtils.touch(new File(top, "regular")); + FileUtils.touch(new File(top, ".hidden")); + + assertEquals(2, top.list().length); + + FileUtils.cleanDirectory(top); + + assertEquals(0, top.list().length); + }

      +
      + public void testDeletesNested() throws Exception

      { + final File nested = new File(top, "nested"); + + assertTrue(nested.mkdirs()); + + FileUtils.touch(new File(nested, "file")); + + assertEquals(1, top.list().length); + + FileUtils.cleanDirectory(top); + + assertEquals(0, top.list().length); + }

      +
      + public void testThrowsOnNullList() throws Exception {
      + if (!chmod(top, 0, false))

      { + // test wont work if we can't restrict permissions on the directory; skip it. + return; + }
      +
      + try { + FileUtils.cleanDirectory(top); + fail("expected IOException"); + } catch (IOException e) { + assertEquals("Failed to list contents of " + top.getAbsolutePath(), e.getMessage()); + }
      + }
      +
      + public void testThrowsOnCannotDeleteFile() throws Exception {
      + final File file = new File(top, "restricted");
      + FileUtils.touch(file);
      +
      + if (!chmod(top, 500, false)) {+ // test wont work if we can't restrict permissions on thedirectory; skip it.+ return;+ }

      +
      + try

      { + FileUtils.cleanDirectory(top); + fail("expected IOException"); + }

      catch (IOException e)

      { + assertEquals("Unable to delete file: " + file.getAbsolutePath(), e.getMessage()); + }

      + }
      +
      + private boolean chmod(File file, int mode, boolean recurse) throws
      IOException, InterruptedException {
      + List args = new ArrayList();
      + args.add("chmod");
      +
      + if (recurse)

      { + args.add("-R"); + }

      +
      + args.add(Integer.toString(mode));
      + args.add(file.getAbsolutePath());
      +
      + Process proc;
      +
      + try

      { + proc = Runtime.getRuntime().exec((String[]) args.toArray(new String[args.size()])); + }

      catch (IOException e)

      { + return false; + }

      +
      + int result = proc.waitFor();
      +
      + assertEquals(0, result);
      + return true;
      + }
      +}
      Index: src/java/org/apache/commons/io/FileUtils.java
      ===================================================================
      — src/java/org/apache/commons/io/FileUtils.java (revision 291328)
      +++ src/java/org/apache/commons/io/FileUtils.java (working copy)
      @@ -701,6 +701,11 @@
      IOException exception = null;

      File[] files = directory.listFiles();
      +
      + if (files == null)

      { + throw new IOException("Failed to list contents of " + directory); + }

      +
      for (int i = 0; i < files.length; i++) {
      File file = files[i];
      try {

        Activity

        Hide
        Stephen Colebourne added a comment -

        Fixed, thanks

        Show
        Stephen Colebourne added a comment - Fixed, thanks

          People

          • Assignee:
            Unassigned
            Reporter:
            Chris Eldredge
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development