HADOOP-9768. chown and chgrp reject users and groups with spaces on platforms where spaces are otherwise acceptable. Contributed by Chris Nauroth.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-1@1508309 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -108,6 +108,9 @@ Release 1.3.0 - unreleased
     MAPREDUCE-4366. mapred metrics shows negative count of waiting maps and 
     reduces. (sandyr via tucu)
 
+    HADOOP-9768. chown and chgrp reject users and groups with spaces on platforms
+    where spaces are otherwise acceptable. (cnauroth)
+
 Release 1.2.1 - 2013.07.06
 
   INCOMPATIBLE CHANGES

src/core/org/apache/hadoop/fs/FsShell.java

@@ -1442,9 +1442,8 @@ public class FsShell extends Configured implements Tool {
       "\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" +

src/core/org/apache/hadoop/fs/FsShellPermissions.java

@@ -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);
 

src/test/org/apache/hadoop/fs/TestFsShellReturnCode.java

@@ -19,11 +19,15 @@
 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;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.util.Shell;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -255,4 +259,110 @@ public class TestFsShellReturnCode {
     int run = shell.run(args);
     assertTrue("Return code should be 0", run == 0);
   }
-}
+
+  /**
+   * 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);
+    }
+  }
+}