From 02b25ce4ef489dab88c4e0e7b1ce0213cf83186d Mon Sep 17 00:00:00 2001
From: Tsz-wo Sze <szetszwo@apache.org>
Date: Tue, 18 Mar 2014 18:34:19 +0000
Subject: [PATCH] HDFS-6068. Disallow snapshot names that are also invalid
 directory names.  Contributed by sathish

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1579004 13f79535-47bb-0310-9956-ffa450edef68
---
 hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt                 | 3 +++
 .../apache/hadoop/hdfs/server/namenode/FSNamesystem.java    | 6 ++++++
 .../hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java  | 6 +++---
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
index ae1cfa1f99..f7d4e21e20 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
+++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
@@ -415,6 +415,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
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
index 15931d0749..c26f964018 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
@@ -6933,6 +6933,12 @@ String createSnapshot(String snapshotRoot, String snapshotName)
       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 {
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java
index 20cc1351e8..5ae1cb7792 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java
@@ -337,12 +337,11 @@ public void testCreateSnapshotWithIllegalName() throws Exception {
       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 void testCreateSnapshotWithIllegalName() throws Exception {
         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);
       }
     }