HADOOP-6703. Prevent renaming a file, directory or symbolic link to itself. Contributed by Eli Collins.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@938788 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -361,6 +361,9 @@ Trunk (unreleased changes)
     HADOOP-6690. FilterFileSystem correctly handles setTimes call.
     (Rodrigo Schmidt via dhruba)
 
+    HADOOP-6703. Prevent renaming a file, directory or symbolic link to
+    itself. (Eli Collins via suresh)
+
     HADOOP-6710. Symbolic umask for file creation is not conformant with posix.
     (suresh)
     

src/java/org/apache/hadoop/fs/AbstractFileSystem.java

@@ -603,6 +603,14 @@ public abstract class AbstractFileSystem {
       dstStatus = null;
     }
     if (dstStatus != null) {
+      if (dst.equals(src)) {
+        throw new FileAlreadyExistsException(
+            "The source "+src+" and destination "+dst+" are the same");
+      }
+      if (srcStatus.isSymlink() && dst.equals(srcStatus.getSymlink())) {
+        throw new FileAlreadyExistsException(
+            "Cannot rename symlink "+src+" to its target "+dst);
+      }
       if (srcStatus.isDir() != dstStatus.isDir()) {
         throw new IOException("Source " + src + " Destination " + dst
             + " both should be either file or directory");

src/test/core/org/apache/hadoop/fs/FileContextMainOperationsBaseTest.java

@@ -824,6 +824,26 @@ public abstract class FileContextMainOperationsBaseTest  {
     rename(src, dst, true, false, true, Rename.OVERWRITE);
   }
 
+  @Test
+  public void testRenameFileToItself() throws Exception {
+    if (!renameSupported()) return;
+    Path src = getTestRootPath(fc, "test/hadoop/file");
+    createFile(src);
+    try {
+      rename(src, src, false, true, false, Rename.NONE);
+      Assert.fail("Renamed file to itself");
+    } catch (IOException e) {
+      Assert.assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);
+    }
+    // Also fails with overwrite
+    try {
+      rename(src, src, false, true, false, Rename.OVERWRITE);
+      Assert.fail("Renamed file to itself");
+    } catch (IOException e) {
+      Assert.assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);
+    }
+  }
+  
   @Test
   public void testRenameFileAsExistingFile() throws Exception {
     if (!renameSupported()) return;
@@ -869,6 +889,26 @@ public abstract class FileContextMainOperationsBaseTest  {
     }
   }
 
+  @Test
+  public void testRenameDirectoryToItself() throws Exception {
+    if (!renameSupported()) return;
+    Path src = getTestRootPath(fc, "test/hadoop/dir");
+    fc.mkdir(src, FileContext.DEFAULT_PERM, true);
+    try {
+      rename(src, src, false, true, false, Rename.NONE);
+      Assert.fail("Renamed directory to itself");
+    } catch (IOException e) {
+      Assert.assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);
+    }
+    // Also fails with overwrite
+    try {
+      rename(src, src, false, true, false, Rename.OVERWRITE);
+      Assert.fail("Renamed directory to itself");
+    } catch (IOException e) {
+      Assert.assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);      
+    }
+  }
+
   @Test
   public void testRenameDirectoryToNonExistentParent() throws Exception {
     if (!renameSupported()) return;

src/test/core/org/apache/hadoop/fs/FileContextSymlinkBaseTest.java

@@ -51,6 +51,10 @@ public abstract class FileContextSymlinkBaseTest {
   abstract protected String testBaseDir2();
   abstract protected URI testURI();
 
+  protected IOException unwrapException(IOException e) {
+    return e;
+  }
+
   protected static void createAndWriteFile(FileContext fc, Path p) 
       throws IOException {
     FSDataOutputStream out;
@@ -766,7 +770,25 @@ public abstract class FileContextSymlinkBaseTest {
     assertFalse(fc.exists(file));
     assertTrue(fc.exists(fileNewViaLink));
   }
-  
+
+  @Test
+  /** Rename a symlink to itself */
+  public void testRenameSymlinkToItself() throws IOException {
+    Path link = new Path(testBaseDir1(), "linkToFile1");
+    fc.createSymlink(new Path("/doestNotExist"), link, false);
+    try {
+      fc.rename(link, link);
+    } catch (IOException e) {
+      assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);
+    }
+    // Fails with overwrite as well
+    try {
+      fc.rename(link, link, Rename.OVERWRITE);
+    } catch (IOException e) {
+      assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);
+    }
+  }
+
   @Test
   /** Rename a symlink */
   public void testRenameSymlink() throws IOException {
@@ -786,8 +808,84 @@ public abstract class FileContextSymlinkBaseTest {
     } catch (IOException x) {
       // Expected
     }
-  } 
-    
+  }
+
+  @Test
+  /** Rename a symlink to the file it links to */
+  public void testRenameSymlinkToFileItLinksTo() throws IOException {
+    /* NB: The rename is not atomic, so file is deleted before renaming
+     * linkToFile. In this interval linkToFile is dangling and local file 
+     * system does not handle dangling links because File.exists returns 
+     * false for dangling links. */
+    if ("file".equals(getScheme())) {
+      return;
+    }
+    Path file = new Path(testBaseDir1(), "file");
+    Path link = new Path(testBaseDir1(), "linkToFile");
+    createAndWriteFile(file);
+    fc.createSymlink(file, link, false);
+    try {
+      fc.rename(link, file);
+      fail("Renamed symlink to its target");
+    } catch (IOException e) {
+      assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);
+    }
+    // Check the rename didn't happen
+    assertTrue(fc.isFile(file));
+    assertTrue(fc.exists(link));
+    assertTrue(fc.getFileLinkStatus(link).isSymlink());
+    assertEquals(file, fc.getLinkTarget(link));
+    try {
+      fc.rename(link, file, Rename.OVERWRITE);
+      fail("Renamed symlink to its target");
+    } catch (IOException e) {
+      assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);
+    }
+    // Check the rename didn't happen
+    assertTrue(fc.isFile(file));
+    assertTrue(fc.exists(link));    
+    assertTrue(fc.getFileLinkStatus(link).isSymlink());
+    assertEquals(file, fc.getLinkTarget(link));    
+  }
+  
+  @Test
+  /** Rename a symlink to the directory it links to */
+  public void testRenameSymlinkToDirItLinksTo() throws IOException {
+    /* NB: The rename is not atomic, so dir is deleted before renaming
+     * linkToFile. In this interval linkToFile is dangling and local file 
+     * system does not handle dangling links because File.exists returns 
+     * false for dangling links. */
+    if ("file".equals(getScheme())) {
+      return;
+    }
+    Path dir  = new Path(testBaseDir1(), "dir");
+    Path link = new Path(testBaseDir1(), "linkToDir");
+    fc.mkdir(dir, FileContext.DEFAULT_PERM, false);
+    fc.createSymlink(dir, link, false);
+    try {
+      fc.rename(link, dir);
+      fail("Renamed symlink to its target");
+    } catch (IOException e) {
+      assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);
+    }
+    // Check the rename didn't happen
+    assertTrue(fc.isDirectory(dir));
+    assertTrue(fc.exists(link));
+    assertTrue(fc.getFileLinkStatus(link).isSymlink());
+    assertEquals(dir, fc.getLinkTarget(link));
+    try {
+      fc.rename(link, dir, Rename.OVERWRITE);
+      fail("Renamed symlink to its target");
+    } catch (IOException e) {
+      assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);
+    }
+    // Check the rename didn't happen
+    assertTrue(fc.isDirectory(dir));
+    assertTrue(fc.exists(link));
+    assertTrue(fc.getFileLinkStatus(link).isSymlink());
+    assertEquals(dir, fc.getLinkTarget(link));
+  }
+  
   @Test
   /** Test renaming symlink target */
   public void testMoveLinkTarget() throws IOException {