Abort before delete in FileUtils.delete EMPTY_DIRECTORIES_ONLY|RECURSIVE

Depending on the order in which items are traversed for RECURSIVE, an
empty directory may come first before detecting that there is a file and
aborting.

This fixes it by traversing files first.

Bug: 405558
Change-Id: I638b7da58e33ffeb0fee172b96f4c823943d29e9
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This commit is contained in:
Robin Stocker 2013-04-12 14:21:02 +02:00 committed by Matthias Sohn
parent 46a3863f21
commit 8bd1e86bb7
2 changed files with 131 additions and 2 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright (C) 2010, Matthias Sohn <matthias.sohn@sap.com>
* Copyright (C) 2010, 2013 Matthias Sohn <matthias.sohn@sap.com>
* and other copyright owners as documented in the project's IP log.
*
* This program and the accompanying materials are made available
@ -118,6 +118,121 @@ public void testDeleteRecursive() throws IOException {
}
}
@Test
public void testDeleteRecursiveEmpty() throws IOException {
File f1 = new File(trash, "test/test/a");
File f2 = new File(trash, "test/a");
File d1 = new File(trash, "test");
File d2 = new File(trash, "test/test");
File d3 = new File(trash, "test/b");
FileUtils.mkdirs(f1.getParentFile());
FileUtils.createNewFile(f2);
FileUtils.createNewFile(f1);
FileUtils.mkdirs(d3);
// Cannot delete hierarchy since files exist
try {
FileUtils.delete(d1, FileUtils.EMPTY_DIRECTORIES_ONLY);
fail("delete should fail");
} catch (IOException e1) {
try {
FileUtils.delete(d1, FileUtils.EMPTY_DIRECTORIES_ONLY|FileUtils.RECURSIVE);
fail("delete should fail");
} catch (IOException e2) {
// Everything still there
assertTrue(f1.exists());
assertTrue(f2.exists());
assertTrue(d1.exists());
assertTrue(d2.exists());
assertTrue(d3.exists());
}
}
// setup: delete files, only directories left
assertTrue(f1.delete());
assertTrue(f2.delete());
// Shall not delete hierarchy without recursive
try {
FileUtils.delete(d1, FileUtils.EMPTY_DIRECTORIES_ONLY);
fail("delete should fail");
} catch (IOException e2) {
// Everything still there
assertTrue(d1.exists());
assertTrue(d2.exists());
assertTrue(d3.exists());
}
// Now delete the empty hierarchy
FileUtils.delete(d2, FileUtils.EMPTY_DIRECTORIES_ONLY
| FileUtils.RECURSIVE);
assertFalse(d2.exists());
// Will fail to delete non-existing without SKIP_MISSING
try {
FileUtils.delete(d2, FileUtils.EMPTY_DIRECTORIES_ONLY);
fail("Cannot delete non-existent entity");
} catch (IOException e) {
// ok
}
// ..with SKIP_MISSING there is no exception
FileUtils.delete(d2, FileUtils.EMPTY_DIRECTORIES_ONLY
| FileUtils.SKIP_MISSING);
FileUtils.delete(d2, FileUtils.EMPTY_DIRECTORIES_ONLY
| FileUtils.RECURSIVE | FileUtils.SKIP_MISSING);
// essentially the same, using IGNORE_ERRORS
FileUtils.delete(d2, FileUtils.EMPTY_DIRECTORIES_ONLY
| FileUtils.IGNORE_ERRORS);
FileUtils.delete(d2, FileUtils.EMPTY_DIRECTORIES_ONLY
| FileUtils.RECURSIVE | FileUtils.IGNORE_ERRORS);
}
@Test
public void testDeleteRecursiveEmptyNeedsToCheckFilesFirst()
throws IOException {
File d1 = new File(trash, "test");
File d2 = new File(trash, "test/a");
File d3 = new File(trash, "test/b");
File f1 = new File(trash, "test/c");
File d4 = new File(trash, "test/d");
FileUtils.mkdirs(d1);
FileUtils.mkdirs(d2);
FileUtils.mkdirs(d3);
FileUtils.mkdirs(d4);
FileUtils.createNewFile(f1);
// Cannot delete hierarchy since file exists
try {
FileUtils.delete(d1, FileUtils.EMPTY_DIRECTORIES_ONLY
| FileUtils.RECURSIVE);
fail("delete should fail");
} catch (IOException e) {
// Everything still there
assertTrue(f1.exists());
assertTrue(d1.exists());
assertTrue(d2.exists());
assertTrue(d3.exists());
assertTrue(d4.exists());
}
}
@Test
public void testDeleteRecursiveEmptyDirectoriesOnlyButIsFile()
throws IOException {
File f1 = new File(trash, "test/test/a");
FileUtils.mkdirs(f1.getParentFile());
FileUtils.createNewFile(f1);
try {
FileUtils.delete(f1, FileUtils.EMPTY_DIRECTORIES_ONLY);
fail("delete should fail");
} catch (IOException e) {
assertTrue(f1.exists());
}
}
@Test
public void testMkdir() throws IOException {
File d = new File(trash, "test");

View File

@ -49,6 +49,8 @@
import java.io.IOException;
import java.nio.channels.FileLock;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.List;
import org.eclipse.jgit.internal.JGitText;
@ -130,8 +132,20 @@ public static void delete(final File f, int options) throws IOException {
if ((options & RECURSIVE) != 0 && f.isDirectory()) {
final File[] items = f.listFiles();
if (items != null) {
List<File> files = new ArrayList<File>();
List<File> dirs = new ArrayList<File>();
for (File c : items)
delete(c, options);
if (c.isFile())
files.add(c);
else
dirs.add(c);
// Try to delete files first, otherwise options
// EMPTY_DIRECTORIES_ONLY|RECURSIVE will delete empty
// directories before aborting, depending on order.
for (File file : files)
delete(file, options);
for (File d : dirs)
delete(d, options);
}
}