Mailing-List: contact hdfs-commits-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: hdfs-dev@hadoop.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1514802 - in
 /hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs: ./
 src/main/java/org/apache/hadoop/hdfs/server/namenode/
 src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/
Date: Fri, 16 Aug 2013 17:19:06 -0000
To: hdfs-commits@hadoop.apache.org
From: jing9@apache.org
Message-Id: <20130816171906.DE35123888E2@eris.apache.org>

Author: jing9
Date: Fri Aug 16 17:19:06 2013
New Revision: 1514802

URL: http://svn.apache.org/r1514802
Log:
HDFS-5102. Merge change r1514797 from trunk.

Modified:
    hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
    hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
    hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java
    hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotRename.java

Modified: hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt?rev=1514802&r1=1514801&r2=1514802&view=diff
==============================================================================
--- hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt (original)
+++ hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt Fri Aug 16 17:19:06 2013
@@ -124,6 +124,9 @@ Release 2.1.1-beta - UNRELEASED
 
     HDFS-5103. TestDirectoryScanner fails on Windows. (Chuan Liu via cnauroth)
 
+    HDFS-5102. Snapshot names should not be allowed to contain slash characters.
+    (jing9)
+
 Release 2.1.0-beta - 2013-08-22
 
   INCOMPATIBLE CHANGES

Modified: hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java?rev=1514802&r1=1514801&r2=1514802&view=diff
==============================================================================
--- hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java (original)
+++ hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java Fri Aug 16 17:19:06 2013
@@ -2093,6 +2093,10 @@ public class FSDirectory implements Clos
   /** Verify if the snapshot name is legal. */
   void verifySnapshotName(String snapshotName, String path)
       throws PathComponentTooLongException {
+    if (snapshotName.contains(Path.SEPARATOR)) {
+      throw new HadoopIllegalArgumentException(
+          "Snapshot name cannot contain \"" + Path.SEPARATOR + "\"");
+    }
     final byte[] bytes = DFSUtil.string2Bytes(snapshotName);
     verifyINodeName(bytes);
     verifyMaxComponentLength(bytes, path, 0);

Modified: hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java?rev=1514802&r1=1514801&r2=1514802&view=diff
==============================================================================
--- hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java (original)
+++ hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java Fri Aug 16 17:19:06 2013
@@ -44,6 +44,7 @@ import org.apache.hadoop.hdfs.DFSTestUti
 import org.apache.hadoop.hdfs.DistributedFileSystem;
 import org.apache.hadoop.hdfs.MiniDFSCluster;
 import org.apache.hadoop.hdfs.client.HdfsDataOutputStream;
+import org.apache.hadoop.hdfs.protocol.HdfsConstants;
 import org.apache.hadoop.hdfs.protocol.HdfsConstants.SafeModeAction;
 import org.apache.hadoop.hdfs.server.namenode.FSDirectory;
 import org.apache.hadoop.hdfs.server.namenode.FSImageTestUtil;
@@ -54,6 +55,7 @@ import org.apache.hadoop.hdfs.server.nam
 import org.apache.hadoop.hdfs.server.namenode.snapshot.SnapshotTestHelper.TestDirectoryTree.Node;
 import org.apache.hadoop.hdfs.tools.offlineImageViewer.OfflineImageViewer;
 import org.apache.hadoop.hdfs.tools.offlineImageViewer.XmlImageVisitor;
+import org.apache.hadoop.ipc.RemoteException;
 import org.apache.hadoop.test.GenericTestUtils;
 import org.apache.hadoop.util.Time;
 import org.apache.log4j.Level;
@@ -342,6 +344,37 @@ public class TestSnapshot {
   }
   
   /**
+   * Test creating a snapshot with illegal name
+   */
+  @Test
+  public void testCreateSnapshotWithIllegalName() throws Exception {
+    final Path dir = new Path("/dir");
+    hdfs.mkdirs(dir);
+    
+    final String name1 = HdfsConstants.DOT_SNAPSHOT_DIR;
+    try {
+      hdfs.createSnapshot(dir, name1);
+      fail("Exception expected when an illegal name is given");
+    } catch (RemoteException e) {
+      String errorMsg = "\"" + HdfsConstants.DOT_SNAPSHOT_DIR
+          + "\" is a reserved name.";
+      GenericTestUtils.assertExceptionContains(errorMsg, e);
+    }
+    
+    String errorMsg = "Snapshot name cannot contain \"" + Path.SEPARATOR + "\"";
+    final String[] badNames = new String[] { "foo" + Path.SEPARATOR,
+        Path.SEPARATOR + "foo", Path.SEPARATOR, "foo" + Path.SEPARATOR + "bar" };
+    for (String badName : badNames) {
+      try {
+        hdfs.createSnapshot(dir, badName);
+        fail("Exception expected when an illegal name is given");
+      } catch (RemoteException e) {
+        GenericTestUtils.assertExceptionContains(errorMsg, e);
+      }
+    }
+  }
+  
+  /**
    * Creating snapshots for a directory that is not snapshottable must fail.
    */
   @Test (timeout=60000)

Modified: hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotRename.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotRename.java?rev=1514802&r1=1514801&r2=1514802&view=diff
==============================================================================
--- hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotRename.java (original)
+++ hadoop/common/branches/branch-2/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotRename.java Fri Aug 16 17:19:06 2013
@@ -20,6 +20,7 @@ package org.apache.hadoop.hdfs.server.na
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.util.List;
 
@@ -29,11 +30,14 @@ import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hdfs.DFSTestUtil;
 import org.apache.hadoop.hdfs.DistributedFileSystem;
 import org.apache.hadoop.hdfs.MiniDFSCluster;
+import org.apache.hadoop.hdfs.protocol.HdfsConstants;
 import org.apache.hadoop.hdfs.protocol.SnapshotException;
 import org.apache.hadoop.hdfs.server.namenode.FSDirectory;
 import org.apache.hadoop.hdfs.server.namenode.FSNamesystem;
 import org.apache.hadoop.hdfs.server.namenode.snapshot.INodeDirectoryWithSnapshot.DirectoryDiff;
 import org.apache.hadoop.hdfs.util.ReadOnlyList;
+import org.apache.hadoop.ipc.RemoteException;
+import org.apache.hadoop.test.GenericTestUtils;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
@@ -190,4 +194,36 @@ public class TestSnapshotRename {
     exception.expectMessage(error);
     hdfs.renameSnapshot(sub1, "s1", "s2");
   }
+  
+  /**
+   * Test renaming a snapshot with illegal name
+   */
+  @Test
+  public void testRenameWithIllegalName() throws Exception {
+    DFSTestUtil.createFile(hdfs, file1, BLOCKSIZE, REPLICATION, seed);
+    // Create snapshots for sub1
+    SnapshotTestHelper.createSnapshot(hdfs, sub1, "s1");
+    
+    final String name1 = HdfsConstants.DOT_SNAPSHOT_DIR;
+    try {
+      hdfs.renameSnapshot(sub1, "s1", name1);
+      fail("Exception expected when an illegal name is given for rename");
+    } catch (RemoteException e) {
+      String errorMsg = "\"" + HdfsConstants.DOT_SNAPSHOT_DIR
+          + "\" is a reserved name.";
+      GenericTestUtils.assertExceptionContains(errorMsg, e);
+    }
+    
+    String errorMsg = "Snapshot name cannot contain \"" + Path.SEPARATOR + "\"";
+    final String[] badNames = new String[] { "foo" + Path.SEPARATOR,
+        Path.SEPARATOR + "foo", Path.SEPARATOR, "foo" + Path.SEPARATOR + "bar" };
+    for (String badName : badNames) {
+      try {
+        hdfs.renameSnapshot(sub1, "s1", badName);
+        fail("Exception expected when an illegal name is given");
+      } catch (RemoteException e) {
+        GenericTestUtils.assertExceptionContains(errorMsg, e);
+      }
+    }
+  }
 }