Author: cnauroth Date: Tue Jul 30 05:56:01 2013 New Revision: 1508310 URL: http://svn.apache.org/r1508310 Log: HADOOP-9768. chown and chgrp reject users and groups with spaces on platforms where spaces are otherwise acceptable. Contributed by Chris Nauroth.
Modified: hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FsShell.java hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FsShellPermissions.java hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFsShellReturnCode.java Modified: hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt?rev=1508310&r1=1508309&r2=1508310&view=diff ============================================================================== --- hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt (original) +++ hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt Tue Jul 30 05:56:01 2013 @@ -475,3 +475,6 @@ Branch-hadoop-1-win (branched from branc HADOOP-9665. Fixed BlockDecompressorStream#decompress to return -1 rather than throw EOF at end of file. (Zhijie Shen via acmurthy) + + HADOOP-9768. chown and chgrp reject users and groups with spaces on platforms + where spaces are otherwise acceptable. (cnauroth) Modified: hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FsShell.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FsShell.java?rev=1508310&r1=1508309&r2=1508310&view=diff ============================================================================== --- hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FsShell.java (original) +++ hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FsShell.java Tue Jul 30 05:56:01 2013 @@ -1435,9 +1435,8 @@ public class FsShell extends Configured "\t\tcurrently supported.\n\n" + "\t\tIf only owner or group is specified then only owner or\n" + "\t\tgroup is modified.\n\n" + - "\t\tThe owner and group names may only cosists of digits, alphabet,\n"+ - "\t\tand any of '-_.@/' i.e. [-_.@/a-zA-Z0-9]. The names are case\n" + - "\t\tsensitive.\n\n" + + "\t\tThe owner and group names may only consist of digits, alphabet,\n"+ + "\t\tand any of " + FsShellPermissions.allowedChars + ". The names are case sensitive.\n\n" + "\t\tWARNING: Avoid using '.' to separate user name and group though\n" + "\t\tLinux allows it. If user names have dots in them and you are\n" + "\t\tusing local file system, you might see surprising results since\n" + Modified: hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FsShellPermissions.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FsShellPermissions.java?rev=1508310&r1=1508309&r2=1508310&view=diff ============================================================================== --- hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FsShellPermissions.java (original) +++ hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FsShellPermissions.java Tue Jul 30 05:56:01 2013 @@ -24,7 +24,7 @@ import java.util.regex.Pattern; import org.apache.hadoop.fs.FsShell.CmdHandler; import org.apache.hadoop.fs.permission.ChmodParser; import org.apache.hadoop.fs.permission.FsPermission; - +import org.apache.hadoop.util.Shell; /** * This class is the home for file permissions related commands. @@ -72,7 +72,8 @@ class FsShellPermissions { /*========== chown ==========*/ - static private String allowedChars = "[-_./@a-zA-Z0-9]"; + static String allowedChars = Shell.WINDOWS ? "[-_./@a-zA-Z0-9 ]" : + "[-_./@a-zA-Z0-9]"; ///allows only "allowedChars" above in names for owner and group static private Pattern chownPattern = Pattern.compile("^\\s*(" + allowedChars + "+)?" + @@ -83,7 +84,7 @@ class FsShellPermissions { static String CHOWN_USAGE = "-chown [-R] [OWNER][:[GROUP]] PATH..."; static String CHGRP_USAGE = "-chgrp [-R] GROUP PATH..."; - private static class ChownHandler extends CmdHandler { + static class ChownHandler extends CmdHandler { protected String owner = null; protected String group = null; @@ -125,7 +126,7 @@ class FsShellPermissions { /*========== chgrp ==========*/ - private static class ChgrpHandler extends ChownHandler { + static class ChgrpHandler extends ChownHandler { ChgrpHandler(FileSystem fs, String groupStr) throws IOException { super("chgrp", fs); Modified: hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFsShellReturnCode.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFsShellReturnCode.java?rev=1508310&r1=1508309&r2=1508310&view=diff ============================================================================== --- hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFsShellReturnCode.java (original) +++ hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFsShellReturnCode.java Tue Jul 30 05:56:01 2013 @@ -19,6 +19,9 @@ package org.apache.hadoop.fs; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.io.IOException; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -259,4 +262,110 @@ public class TestFsShellReturnCode { int run = shell.run(args); assertTrue("Return code should be 0", run == 0); } -} \ No newline at end of file + + /** + * Tests combinations of valid and invalid user and group arguments to chown. + */ + @Test + public void testChownUserAndGroupValidity() throws IOException { + // The following are valid (no exception expected). + new FsShellPermissions.ChownHandler(fs, "user"); + new FsShellPermissions.ChownHandler(fs, "user:group"); + new FsShellPermissions.ChownHandler(fs, ":group"); + + // The following are valid only on Windows. + assertChownValidArgumentsOnWindows(fs, "User With Spaces"); + assertChownValidArgumentsOnWindows(fs, "User With Spaces:group"); + assertChownValidArgumentsOnWindows(fs, "User With Spaces:Group With Spaces"); + assertChownValidArgumentsOnWindows(fs, "user:Group With Spaces"); + assertChownValidArgumentsOnWindows(fs, ":Group With Spaces"); + + // The following are invalid (exception expected). + assertChownIllegalArguments(fs, "us!er"); + assertChownIllegalArguments(fs, "us^er"); + assertChownIllegalArguments(fs, "user:gr#oup"); + assertChownIllegalArguments(fs, "user:gr%oup"); + assertChownIllegalArguments(fs, ":gr#oup"); + assertChownIllegalArguments(fs, ":gr%oup"); + } + + /** + * Tests valid and invalid group arguments to chgrp. + */ + @Test + public void testChgrpGroupValidity() throws IOException { + // The following are valid (no exception expected). + new FsShellPermissions.ChgrpHandler(fs, "group"); + + // The following are valid only on Windows. + assertChgrpValidArgumentsOnWindows(fs, "Group With Spaces"); + + // The following are invalid (exception expected). + assertChgrpIllegalArguments(fs, ":gr#oup"); + assertChgrpIllegalArguments(fs, ":gr%oup"); + } + + /** + * Asserts that chgrp considers the given arguments invalid. The expectation + * is that the command will throw IOException. + * + * @param fs FileSystem argument + * @param group String argument + */ + private static void assertChgrpIllegalArguments(FileSystem fs, String group) { + try { + new FsShellPermissions.ChgrpHandler(fs, group); + fail("Expected IOException from group: " + group); + } catch (IOException e) { + } + } + + /** + * Asserts that chgrp considers the given arguments valid on Windows, but + * invalid elsewhere. + * + * @param fs FileSystem argument + * @param group String argument + * @throws IOException if there is an I/O error running the command + */ + private static void assertChgrpValidArgumentsOnWindows(FileSystem fs, + String group) throws IOException { + if (Shell.WINDOWS) { + new FsShellPermissions.ChgrpHandler(fs, group); + } else { + assertChgrpIllegalArguments(fs, group); + } + } + + /** + * Asserts that chown considers the given arguments invalid. The expectation + * is that the command will throw IOException. + * + * @param fs FileSystem argument + * @param owner String argument + */ + private static void assertChownIllegalArguments(FileSystem fs, String owner) { + try { + new FsShellPermissions.ChownHandler(fs, owner); + fail("Expected IOException from owner: " + owner); + } catch (IOException e) { + } + } + + /** + * Asserts that chown considers the given arguments valid on Windows, but + * invalid elsewhere. + * + * @param fs FileSystem argument + * @param owner String argument + * @throws IOException if there is an I/O error running the command + */ + private static void assertChownValidArgumentsOnWindows(FileSystem fs, + String owner) throws IOException { + if (Shell.WINDOWS) { + new FsShellPermissions.ChownHandler(fs, owner); + } else { + assertChownIllegalArguments(fs, owner); + } + } +}