diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 35b5fff516..00e206d00a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -350,6 +350,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 diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java index 9523a50a47..532a2bfb21 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java @@ -2093,6 +2093,10 @@ private void verifyQuotaForRename(INode[] src, INode[] dst) /** 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); 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 c84af965b7..27228bd048 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 @@ -44,6 +44,7 @@ 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.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; @@ -341,6 +343,37 @@ public void testUpdateDirectory() throws Exception { assertEquals(oldStatus.getAccessTime(), snapshotStatus.getAccessTime()); } + /** + * 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. */ diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotRename.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotRename.java index ab1752a166..386563bea1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotRename.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotRename.java @@ -20,6 +20,7 @@ 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.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 void testRenameToExistingSnapshot() throws Exception { 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); + } + } + } }