From 2c0598a9646bb8a43375d2dee83c0c60cbfc52df Mon Sep 17 00:00:00 2001 From: Amareshwari Sri Ramadasu Date: Tue, 20 Jul 2010 05:21:56 +0000 Subject: [PATCH] HADOOP-6536. Fixes FileUtil.fullyDelete() not to delete the contents of the sym-linked directory. Contributed by Ravi Gummadi git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@965725 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 3 + src/java/org/apache/hadoop/fs/FileUtil.java | 22 ++++++- .../org/apache/hadoop/fs/TestFileUtil.java | 63 +++++++++++++++++++ 3 files changed, 85 insertions(+), 3 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index f132f891f49..306ee004604 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -142,6 +142,9 @@ Trunk (unreleased changes) HADOOP-6670. Use the UserGroupInformation's Subject as the criteria for equals and hashCode. (Owen O'Malley and Kan Zhang via ddas) + HADOOP-6536. Fixes FileUtil.fullyDelete() not to delete the contents of + the sym-linked directory. (Ravi Gummadi via amareshwari) + Release 0.21.0 - Unreleased INCOMPATIBLE CHANGES diff --git a/src/java/org/apache/hadoop/fs/FileUtil.java b/src/java/org/apache/hadoop/fs/FileUtil.java index d5537504586..5492284b3d8 100644 --- a/src/java/org/apache/hadoop/fs/FileUtil.java +++ b/src/java/org/apache/hadoop/fs/FileUtil.java @@ -79,8 +79,22 @@ public class FileUtil { /** * Delete a directory and all its contents. If * we return false, the directory may be partially-deleted. + * (1) If dir is symlink to a file, the symlink is deleted. The file pointed + * to by the symlink is not deleted. + * (2) If dir is symlink to a directory, symlink is deleted. The directory + * pointed to by symlink is not deleted. + * (3) If dir is a normal file, it is deleted. + * (4) If dir is a normal directory, then dir and all its contents recursively + * are deleted. */ public static boolean fullyDelete(File dir) throws IOException { + if (dir.delete()) { + // dir is (a) normal file, (b) symlink to a file, (c) empty directory or + // (d) symlink to a directory + return true; + } + + // handle nonempty directory deletion if (!fullyDeleteContents(dir)) { return false; } @@ -90,6 +104,8 @@ public class FileUtil { /** * Delete the contents of a directory, not the directory itself. If * we return false, the directory may be partially-deleted. + * If dir is a symlink to a directory, all the contents of the actual + * directory pointed to by dir will be deleted. */ public static boolean fullyDeleteContents(File dir) throws IOException { boolean deletionSucceeded = true; @@ -97,13 +113,13 @@ public class FileUtil { if (contents != null) { for (int i = 0; i < contents.length; i++) { if (contents[i].isFile()) { - if (!contents[i].delete()) { + if (!contents[i].delete()) {// normal file or symlink to another file deletionSucceeded = false; continue; // continue deletion of other files/dirs under dir } } else { - //try deleting the directory - // this might be a symlink + // Either directory or symlink to another directory. + // Try deleting the directory as this might be a symlink boolean b = false; b = contents[i].delete(); if (b){ diff --git a/src/test/core/org/apache/hadoop/fs/TestFileUtil.java b/src/test/core/org/apache/hadoop/fs/TestFileUtil.java index 886ef81fd63..ccb237c117d 100644 --- a/src/test/core/org/apache/hadoop/fs/TestFileUtil.java +++ b/src/test/core/org/apache/hadoop/fs/TestFileUtil.java @@ -95,6 +95,69 @@ public class TestFileUtil { validateTmpDir(); } + /** + * Tests if fullyDelete deletes + * (a) symlink to file only and not the file pointed to by symlink. + * (b) symlink to dir only and not the dir pointed to by symlink. + * @throws IOException + */ + @Test + public void testFullyDeleteSymlinks() throws IOException { + setupDirs(); + + File link = new File(del, LINK); + Assert.assertEquals(5, del.list().length); + // Since tmpDir is symlink to tmp, fullyDelete(tmpDir) should not + // delete contents of tmp. See setupDirs for details. + boolean ret = FileUtil.fullyDelete(link); + Assert.assertTrue(ret); + Assert.assertFalse(link.exists()); + Assert.assertEquals(4, del.list().length); + validateTmpDir(); + + File linkDir = new File(del, "tmpDir"); + // Since tmpDir is symlink to tmp, fullyDelete(tmpDir) should not + // delete contents of tmp. See setupDirs for details. + ret = FileUtil.fullyDelete(linkDir); + Assert.assertTrue(ret); + Assert.assertFalse(linkDir.exists()); + Assert.assertEquals(3, del.list().length); + validateTmpDir(); + } + + /** + * Tests if fullyDelete deletes + * (a) dangling symlink to file properly + * (b) dangling symlink to directory properly + * @throws IOException + */ + @Test + public void testFullyDeleteDanglingSymlinks() throws IOException { + setupDirs(); + // delete the directory tmp to make tmpDir a dangling link to dir tmp and + // to make y as a dangling link to file tmp/x + boolean ret = FileUtil.fullyDelete(tmp); + Assert.assertTrue(ret); + Assert.assertFalse(tmp.exists()); + + // dangling symlink to file + File link = new File(del, LINK); + Assert.assertEquals(5, del.list().length); + // Even though 'y' is dangling symlink to file tmp/x, fullyDelete(y) + // should delete 'y' properly. + ret = FileUtil.fullyDelete(link); + Assert.assertTrue(ret); + Assert.assertEquals(4, del.list().length); + + // dangling symlink to directory + File linkDir = new File(del, "tmpDir"); + // Even though tmpDir is dangling symlink to tmp, fullyDelete(tmpDir) should + // delete tmpDir properly. + ret = FileUtil.fullyDelete(linkDir); + Assert.assertTrue(ret); + Assert.assertEquals(3, del.list().length); + } + @Test public void testFullyDeleteContents() throws IOException { setupDirs();