You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by cn...@apache.org on 2013/07/30 07:28:44 UTC
svn commit: r1508305 - in
/hadoop/common/trunk/hadoop-common-project/hadoop-common: CHANGES.txt
src/main/java/org/apache/hadoop/fs/FsShellPermissions.java
src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
src/test/resources/testConf.xml
Author: cnauroth
Date: Tue Jul 30 05:28:43 2013
New Revision: 1508305
URL: http://svn.apache.org/r1508305
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/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShellPermissions.java
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/resources/testConf.xml
Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1508305&r1=1508304&r2=1508305&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt (original)
+++ hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt Tue Jul 30 05:28:43 2013
@@ -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
Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShellPermissions.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShellPermissions.java?rev=1508305&r1=1508304&r2=1508305&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShellPermissions.java (original)
+++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShellPermissions.java Tue Jul 30 05:28:43 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/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java?rev=1508305&r1=1508304&r2=1508305&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java (original)
+++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java Tue Jul 30 05:28:43 2013
@@ -20,11 +20,13 @@ 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.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.FsComm
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 class TestFsShellReturnCode {
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 @@ public class TestFsShellReturnCode {
}
}
}
+
+ /**
+ * 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/trunk/hadoop-common-project/hadoop-common/src/test/resources/testConf.xml
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/resources/testConf.xml?rev=1508305&r1=1508304&r2=1508305&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/resources/testConf.xml (original)
+++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/resources/testConf.xml Tue Jul 30 05:28:43 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>