diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt
index db9efb6944..83d44a07de 100644
--- a/hadoop-common-project/hadoop-common/CHANGES.txt
+++ b/hadoop-common-project/hadoop-common/CHANGES.txt
@@ -660,6 +660,9 @@ Release 2.1.0-beta - 2013-07-02
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
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShellPermissions.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShellPermissions.java
index 41c9f4b367..5ac10ceda7 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShellPermissions.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShellPermissions.java
@@ -31,7 +31,7 @@
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 @@ protected void processPath(PathData item) throws IOException {
}
// 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 static class Chown extends FsShellPermissions {
"\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" +
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
index d64292b39d..dcc19df3d4 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
@@ -20,11 +20,13 @@
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.InterruptedIOException;
import java.io.PrintStream;
+import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedList;
@@ -38,6 +40,7 @@
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;
@@ -407,6 +410,65 @@ public void testInterrupt() throws Exception {
assertEquals(2, InterruptCommand.processed);
assertEquals(130, exitCode);
}
+
+ /**
+ * 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() {
@@ -480,4 +542,37 @@ protected void processPath(PathData item) throws IOException {
}
}
}
+
+ /**
+ * 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);
+ }
+ }
}
diff --git a/hadoop-common-project/hadoop-common/src/test/resources/testConf.xml b/hadoop-common-project/hadoop-common/src/test/resources/testConf.xml
index 13459094d6..69886abb84 100644
--- a/hadoop-common-project/hadoop-common/src/test/resources/testConf.xml
+++ b/hadoop-common-project/hadoop-common/src/test/resources/testConf.xml
@@ -779,15 +779,11 @@
RegexpComparator
- ^( |\t)*The owner and group names may only cosists of digits, alphabet,( )*
+ ^( |\t)*The owner and group names may only consist of digits, alphabet,( )*
RegexpComparator
- ^( |\t)*and any of '-_.@/' i.e. \[-_.@/a-zA-Z0-9\]. The names are case( )*
-
-
- RegexpComparator
- ^( |\t)*sensitive.( )*
+ ^( |\t)*and any of .+?. The names are case sensitive.( )*
RegexpComparator