Merge 1579004 and 1579005 from trunk for HDFS-6068. Disallow snapshot names that are also invalid directory names.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1579007 13f79535-47bb-0310-9956-ffa450edef68

hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt

@@ -171,6 +171,9 @@ Release 2.4.0 - UNRELEASED
     HDFS-6090. Use MiniDFSCluster.Builder instead of deprecated constructors.
     (Akira AJISAKA via jing9)
 
+    HDFS-6068. Disallow snapshot names that are also invalid directory names.
+    (sathish via szetszwo)
+
   OPTIMIZATIONS
 
     HDFS-5790. LeaseManager.findPath is very slow when many leases need recovery

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java

@@ -6913,6 +6913,12 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
       if (snapshotName == null || snapshotName.isEmpty()) {
         snapshotName = Snapshot.generateDefaultSnapshotName();
       }
+      if(snapshotName != null){
+        if (!DFSUtil.isValidNameForComponent(snapshotName)) {
+            throw new InvalidPathException("Invalid snapshot name: "
+                + snapshotName);
+        }
+      }
       dir.verifySnapshotName(snapshotName, snapshotRoot);
       dir.writeLock();
       try {

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java

@@ -337,12 +337,11 @@ public class TestSnapshot {
       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.";
+      String errorMsg = "Invalid path name Invalid snapshot name: " + name1; 
       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) {
@@ -350,6 +349,7 @@ public class TestSnapshot {
         hdfs.createSnapshot(dir, badName);
         fail("Exception expected when an illegal name is given");
       } catch (RemoteException e) {
+    String errorMsg = "Invalid path name Invalid snapshot name: " + badName ;
         GenericTestUtils.assertExceptionContains(errorMsg, e);
       }
     }

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotNameWithInvalidCharacters.java

@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.hadoop.hdfs.server.namenode.snapshot;
+
+import org.apache.hadoop.conf.Configuration;
+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.ipc.RemoteException;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestSnapshotNameWithInvalidCharacters {
+  private static final long SEED = 0;
+  private static final short REPLICATION = 1;
+  private static final int BLOCKSIZE = 1024;
+
+  private static Configuration conf = new Configuration();
+  private static MiniDFSCluster cluster;
+  private static DistributedFileSystem hdfs;
+
+  private final Path dir1 = new Path("/");
+  private final String file1Name = "file1";
+  private final String snapshot1 = "a:b:c";
+  private final String snapshot2 = "a/b/c";
+
+  @Before
+  public void setUp() throws Exception {
+    cluster = new MiniDFSCluster.Builder(conf).numDataNodes(REPLICATION)
+                                              .build();
+    cluster.waitActive();
+    hdfs = cluster.getFileSystem();
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    if (cluster != null) {
+      cluster.shutdown();
+      cluster = null;
+    }
+  }
+
+  @Test (timeout = 600000)
+  public void TestSnapshotWithInvalidName() throws Exception {
+
+    Path file1 = new Path(dir1,file1Name);
+    DFSTestUtil.createFile(hdfs,file1, BLOCKSIZE,REPLICATION,SEED);
+
+    hdfs.allowSnapshot(dir1);
+    try {
+        hdfs.createSnapshot(dir1, snapshot1);
+    } catch (RemoteException e) {
+    }
+  }
+
+  @Test(timeout = 60000)
+  public void TestSnapshotWithInvalidName1() throws Exception{
+    Path file1 = new Path(dir1, file1Name);
+    DFSTestUtil.createFile(hdfs, file1, BLOCKSIZE, REPLICATION, SEED);
+
+    hdfs.allowSnapshot(dir1);
+    try {
+        hdfs.createSnapshot(dir1, snapshot2);
+    } catch (RemoteException e) {
+   }
+  }
+}
+
+