From af3163d1d1e144a55fc448110a6ba6cdca7204c0 Mon Sep 17 00:00:00 2001 From: Robert Joseph Evans Date: Fri, 16 Mar 2012 16:08:00 +0000 Subject: [PATCH] HADOOP-8176. Disambiguate the destination of FsShell copies (Daryn Sharp via bobby) git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1301612 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 3 ++ .../fs/shell/CommandWithDestination.java | 2 + .../org/apache/hadoop/fs/shell/PathData.java | 13 ++++- .../org/apache/hadoop/fs/TestFsShellCopy.java | 48 +++++++++++++++++++ 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index acf3a3db9f..03964522c6 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -381,6 +381,9 @@ Release 0.23.2 - UNRELEASED HADOOP-8175. Add -p option to mkdir in FsShell. (Daryn Sharp via szetszwo) + HADOOP-8176. Disambiguate the destination of FsShell copies (Daryn Sharp + via bobby) + Release 0.23.1 - 2012-02-17 INCOMPATIBLE CHANGES diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java index 82c4391f36..a18b6e1736 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java @@ -200,6 +200,8 @@ protected PathData getTargetPath(PathData src) throws IOException { // a child path if dst is a dir; after recursion, it's always a dir if ((getDepth() > 0) || (dst.exists && dst.stat.isDirectory())) { target = dst.getPathDataForChild(src); + } else if (dst.representsDirectory()) { // see if path looks like a dir + target = dst.getPathDataForChild(src); } else { target = dst; } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/PathData.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/PathData.java index cb481c8e0e..1e3dfd2c4a 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/PathData.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/PathData.java @@ -188,12 +188,21 @@ public PathData createTempFile(String extension) throws IOException { * @throws IOException upon unexpected error */ public boolean parentExists() throws IOException { + return representsDirectory() + ? fs.exists(path) : fs.exists(path.getParent()); + } + + /** + * Check if the path represents a directory as determined by the basename + * being "." or "..", or the path ending with a directory separator + * @return boolean if this represents a directory + */ + public boolean representsDirectory() { String uriPath = uri.getPath(); String name = uriPath.substring(uriPath.lastIndexOf("/")+1); // Path will munch off the chars that indicate a dir, so there's no way // to perform this test except by examining the raw basename we maintain - return (name.isEmpty() || name.equals(".") || name.equals("..")) - ? fs.exists(path) : fs.exists(path.getParent()); + return (name.isEmpty() || name.equals(".") || name.equals("..")); } /** diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java index 1ba1f91555..5b4f585c19 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java @@ -225,6 +225,54 @@ private void checkPut(int exitCode, Path src, Path dest) throws Exception { assertEquals(exitCode, gotExit); } + @Test + public void testRepresentsDir() throws Exception { + Path subdirDstPath = new Path(dstPath, srcPath.getName()); + String argv[] = null; + lfs.delete(dstPath, true); + assertFalse(lfs.exists(dstPath)); + + argv = new String[]{ "-put", srcPath.toString(), dstPath.toString() }; + assertEquals(0, shell.run(argv)); + assertTrue(lfs.exists(dstPath) && lfs.isFile(dstPath)); + + lfs.delete(dstPath, true); + assertFalse(lfs.exists(dstPath)); + + // since dst path looks like a dir, it should not copy the file and + // rename it to what looks like a directory + lfs.delete(dstPath, true); // make copy fail + for (String suffix : new String[]{ "/", "/." } ) { + argv = new String[]{ + "-put", srcPath.toString(), dstPath.toString()+suffix }; + assertEquals(1, shell.run(argv)); + assertFalse(lfs.exists(dstPath)); + assertFalse(lfs.exists(subdirDstPath)); + } + + // since dst path looks like a dir, it should not copy the file and + // rename it to what looks like a directory + for (String suffix : new String[]{ "/", "/." } ) { + // empty out the directory and create to make copy succeed + lfs.delete(dstPath, true); + lfs.mkdirs(dstPath); + argv = new String[]{ + "-put", srcPath.toString(), dstPath.toString()+suffix }; + assertEquals(0, shell.run(argv)); + assertTrue(lfs.exists(subdirDstPath)); + assertTrue(lfs.isFile(subdirDstPath)); + } + + // ensure .. is interpreted as a dir + String dotdotDst = dstPath+"/foo/.."; + lfs.delete(dstPath, true); + lfs.mkdirs(new Path(dstPath, "foo")); + argv = new String[]{ "-put", srcPath.toString(), dotdotDst }; + assertEquals(0, shell.run(argv)); + assertTrue(lfs.exists(subdirDstPath)); + assertTrue(lfs.isFile(subdirDstPath)); + } + @Test public void testCopyMerge() throws Exception { Path root = new Path(testRootDir, "TestMerge");