Author: cnauroth Date: Tue Jul 30 17:26:22 2013 New Revision: 1508535 URL: http://svn.apache.org/r1508535 Log: HADOOP-9768. Merging change r1508530 from branch-2 to branch-2.1-beta.
Modified: hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/CHANGES.txt hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShellPermissions.java hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/test/resources/testConf.xml Modified: hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1508535&r1=1508534&r2=1508535&view=diff ============================================================================== --- hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/CHANGES.txt (original) +++ hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/CHANGES.txt Tue Jul 30 17:26:22 2013 @@ -363,6 +363,9 @@ Release 2.1.0-beta - 2013-08-06 HADOOP-9507. LocalFileSystem rename() is broken in some cases when destination exists. (cnauroth) + HADOOP-9768. chown and chgrp reject users and groups with spaces on platforms + where spaces are otherwise acceptable. (cnauroth) + BREAKDOWN OF HADOOP-8562 SUBTASKS AND RELATED JIRAS HADOOP-8924. Hadoop Common creating package-info.java must not depend on Modified: hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShellPermissions.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShellPermissions.java?rev=1508535&r1=1508534&r2=1508535&view=diff ============================================================================== --- hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShellPermissions.java (original) +++ hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShellPermissions.java Tue Jul 30 17:26:22 2013 @@ -31,7 +31,7 @@ import org.apache.hadoop.fs.shell.Comman import org.apache.hadoop.fs.shell.CommandFormat; import org.apache.hadoop.fs.shell.FsCommand; import org.apache.hadoop.fs.shell.PathData; - +import org.apache.hadoop.util.Shell; /** * This class is the home for file permissions related commands. @@ -111,7 +111,8 @@ public class FsShellPermissions extends } // used by chown/chgrp - static private String allowedChars = "[-_./@a-zA-Z0-9]"; + static private String allowedChars = Shell.WINDOWS ? "[-_./@a-zA-Z0-9 ]" : + "[-_./@a-zA-Z0-9]"; /** * Used to change owner and/or group of files @@ -126,9 +127,8 @@ public class FsShellPermissions extends "\tcurrently supported.\n\n" + "\tIf only owner or group is specified then only owner or\n" + "\tgroup is modified.\n\n" + - "\tThe owner and group names may only cosists of digits, alphabet,\n"+ - "\tand any of '-_.@/' i.e. [-_.@/a-zA-Z0-9]. The names are case\n" + - "\tsensitive.\n\n" + + "\tThe owner and group names may only consist of digits, alphabet,\n"+ + "\tand any of " + allowedChars + ". The names are case sensitive.\n\n" + "\tWARNING: Avoid using '.' to separate user name and group though\n" + "\tLinux allows it. If user names have dots in them and you are\n" + "\tusing local file system, you might see surprising results since\n" + Modified: hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java?rev=1508535&r1=1508534&r2=1508535&view=diff ============================================================================== --- hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java (original) +++ hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java Tue Jul 30 17:26:22 2013 @@ -20,10 +20,12 @@ package org.apache.hadoop.fs; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.PrintStream; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.LinkedList; @@ -32,8 +34,11 @@ import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.shell.FsCommand; +import org.apache.hadoop.fs.shell.PathData; import org.apache.hadoop.io.IOUtils; import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY; +import org.apache.hadoop.util.Shell; import org.junit.BeforeClass; import org.junit.Test; @@ -377,6 +382,65 @@ public class TestFsShellReturnCode { } + /** + * Tests combinations of valid and invalid user and group arguments to chown. + */ + @Test + public void testChownUserAndGroupValidity() { + // This test only covers argument parsing, so override to skip processing. + FsCommand chown = new FsShellPermissions.Chown() { + @Override + protected void processArgument(PathData item) { + } + }; + chown.setConf(new Configuration()); + + // The following are valid (no exception expected). + chown.run("user", "/path"); + chown.run("user:group", "/path"); + chown.run(":group", "/path"); + + // The following are valid only on Windows. + assertValidArgumentsOnWindows(chown, "User With Spaces", "/path"); + assertValidArgumentsOnWindows(chown, "User With Spaces:group", "/path"); + assertValidArgumentsOnWindows(chown, "User With Spaces:Group With Spaces", + "/path"); + assertValidArgumentsOnWindows(chown, "user:Group With Spaces", "/path"); + assertValidArgumentsOnWindows(chown, ":Group With Spaces", "/path"); + + // The following are invalid (exception expected). + assertIllegalArguments(chown, "us!er", "/path"); + assertIllegalArguments(chown, "us^er", "/path"); + assertIllegalArguments(chown, "user:gr#oup", "/path"); + assertIllegalArguments(chown, "user:gr%oup", "/path"); + assertIllegalArguments(chown, ":gr#oup", "/path"); + assertIllegalArguments(chown, ":gr%oup", "/path"); + } + + /** + * Tests valid and invalid group arguments to chgrp. + */ + @Test + public void testChgrpGroupValidity() { + // This test only covers argument parsing, so override to skip processing. + FsCommand chgrp = new FsShellPermissions.Chgrp() { + @Override + protected void processArgument(PathData item) { + } + }; + chgrp.setConf(new Configuration()); + + // The following are valid (no exception expected). + chgrp.run("group", "/path"); + + // The following are valid only on Windows. + assertValidArgumentsOnWindows(chgrp, "Group With Spaces", "/path"); + + // The following are invalid (exception expected). + assertIllegalArguments(chgrp, ":gr#oup", "/path"); + assertIllegalArguments(chgrp, ":gr%oup", "/path"); + } + static class LocalFileSystemExtn extends LocalFileSystem { public LocalFileSystemExtn() { super(new RawLocalFileSystemExtn()); @@ -425,4 +489,37 @@ public class TestFsShellReturnCode { return stat; } } + + /** + * Asserts that for the given command, the given arguments are considered + * invalid. The expectation is that the command will throw + * IllegalArgumentException. + * + * @param cmd FsCommand to check + * @param args String... arguments to check + */ + private static void assertIllegalArguments(FsCommand cmd, String... args) { + try { + cmd.run(args); + fail("Expected IllegalArgumentException from args: " + + Arrays.toString(args)); + } catch (IllegalArgumentException e) { + } + } + + /** + * Asserts that for the given command, the given arguments are considered valid + * on Windows, but invalid elsewhere. + * + * @param cmd FsCommand to check + * @param args String... arguments to check + */ + private static void assertValidArgumentsOnWindows(FsCommand cmd, + String... args) { + if (Shell.WINDOWS) { + cmd.run(args); + } else { + assertIllegalArguments(cmd, args); + } + } } Modified: hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/test/resources/testConf.xml URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/test/resources/testConf.xml?rev=1508535&r1=1508534&r2=1508535&view=diff ============================================================================== --- hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/test/resources/testConf.xml (original) +++ hadoop/common/branches/branch-2.1-beta/hadoop-common-project/hadoop-common/src/test/resources/testConf.xml Tue Jul 30 17:26:22 2013 @@ -779,15 +779,11 @@ </comparator> <comparator> <type>RegexpComparator</type> - <expected-output>^( |\t)*The owner and group names may only cosists of digits, alphabet,( )*</expected-output> + <expected-output>^( |\t)*The owner and group names may only consist of digits, alphabet,( )*</expected-output> </comparator> <comparator> <type>RegexpComparator</type> - <expected-output>^( |\t)*and any of '-_.@/' i.e. \[-_.@/a-zA-Z0-9\]. The names are case( )*</expected-output> - </comparator> - <comparator> - <type>RegexpComparator</type> - <expected-output>^( |\t)*sensitive.( )*</expected-output> + <expected-output>^( |\t)*and any of .+?. The names are case sensitive.( )*</expected-output> </comparator> <comparator> <type>RegexpComparator</type>