Author: cnauroth
Date: Mon Jan 20 18:00:06 2014
New Revision: 1559793
URL: http://svn.apache.org/r1559793
Log:
HADOOP-10213. Fix bugs parsing ACL spec in FsShell setfacl. Contributed by
Vinay.
Modified:
hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntry.java
hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/AclCommands.java
hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestAclCommands.java
Modified:
hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntry.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntry.java?rev=1559793&r1=1559792&r2=1559793&view=diff
==============================================================================
---
hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntry.java
(original)
+++
hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntry.java
Mon Jan 20 18:00:06 2014
@@ -17,12 +17,16 @@
*/
package org.apache.hadoop.fs.permission;
-import static org.apache.hadoop.fs.permission.AclEntryScope.*;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
import com.google.common.base.Objects;
+import org.apache.hadoop.HadoopIllegalArgumentException;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.util.StringUtils;
/**
* Defines a single entry in an ACL. An ACL entry has a type (user, group,
@@ -193,4 +197,74 @@ public class AclEntry {
this.permission = permission;
this.scope = scope;
}
+
+ /**
+ * Parses a string representation of an ACL spec into a list of AclEntry
+ * objects. Example: "user::rwx,user:foo:rw-,group::r--,other::---"
+ *
+ * @param aclSpec
+ * String representation of an ACL spec.
+ * @param includePermission
+ * for setAcl operations this will be true. i.e. AclSpec should
+ * include permissions.<br>
+ * But for removeAcl operation it will be false. i.e. AclSpec should
+ * not contain permissions.<br>
+ * Example: "user:foo,group:bar"
+ * @return Returns list of AclEntries parsed
+ */
+ public static List<AclEntry> parseAclSpec(String aclSpec,
+ boolean includePermission) {
+ List<AclEntry> aclEntries = new ArrayList<AclEntry>();
+ Collection<String> aclStrings = StringUtils.getStringCollection(aclSpec,
+ ",");
+ for (String aclStr : aclStrings) {
+ AclEntry.Builder builder = new AclEntry.Builder();
+ // Here "::" represent one empty string.
+ // StringUtils.getStringCollection() will ignore this.
+ String[] split = aclStr.split(":");
+ int expectedAclSpecLength = 2;
+ if (includePermission) {
+ expectedAclSpecLength = 3;
+ }
+ if (split.length != expectedAclSpecLength
+ && !(split.length == expectedAclSpecLength + 1 && "default"
+ .equals(split[0]))) {
+ throw new HadoopIllegalArgumentException("Invalid <aclSpec> : "
+ + aclStr);
+ }
+ int index = 0;
+ if (split.length == expectedAclSpecLength + 1) {
+ assert "default".equals(split[0]);
+ // default entry
+ index++;
+ builder.setScope(AclEntryScope.DEFAULT);
+ }
+ String type = split[index++];
+ AclEntryType aclType = null;
+ try {
+ aclType = Enum.valueOf(AclEntryType.class, type.toUpperCase());
+ builder.setType(aclType);
+ } catch (IllegalArgumentException iae) {
+ throw new HadoopIllegalArgumentException(
+ "Invalid type of acl in <aclSpec> :" + aclStr);
+ }
+
+ String name = split[index++];
+ if (!name.isEmpty()) {
+ builder.setName(name);
+ }
+
+ if (expectedAclSpecLength == 3) {
+ String permission = split[index++];
+ FsAction fsAction = FsAction.getFsAction(permission);
+ if (null == fsAction) {
+ throw new HadoopIllegalArgumentException(
+ "Invalid permission in <aclSpec> : " + aclStr);
+ }
+ builder.setPermission(fsAction);
+ }
+ aclEntries.add(builder.build());
+ }
+ return aclEntries;
+ }
}
Modified:
hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/AclCommands.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/AclCommands.java?rev=1559793&r1=1559792&r2=1559793&view=diff
==============================================================================
---
hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/AclCommands.java
(original)
+++
hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/AclCommands.java
Mon Jan 20 18:00:06 2014
@@ -18,8 +18,6 @@
package org.apache.hadoop.fs.shell;
import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collection;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
@@ -27,14 +25,12 @@ import java.util.List;
import org.apache.hadoop.HadoopIllegalArgumentException;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
-import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.permission.AclEntry;
import org.apache.hadoop.fs.permission.AclEntryScope;
import org.apache.hadoop.fs.permission.AclEntryType;
import org.apache.hadoop.fs.permission.AclStatus;
import org.apache.hadoop.fs.permission.FsAction;
import org.apache.hadoop.fs.permission.FsPermission;
-import org.apache.hadoop.util.StringUtils;
/**
* Acl related operations
@@ -201,9 +197,6 @@ class AclCommands extends FsCommand {
+ "<acl_spec>: Comma separated list of ACL entries.\n"
+ "<path>: File or directory to modify.\n";
- private static final String DEFAULT = "default";
-
- Path path = null;
CommandFormat cf = new CommandFormat(0, Integer.MAX_VALUE, "b", "k", "R",
"m", "x", "-set");
List<AclEntry> aclEntries = null;
@@ -230,7 +223,7 @@ class AclCommands extends FsCommand {
if (args.size() < 2) {
throw new HadoopIllegalArgumentException("<acl_spec> is missing");
}
- aclEntries = parseAclSpec(args.removeFirst());
+ aclEntries = AclEntry.parseAclSpec(args.removeFirst(),
!cf.getOpt("x"));
}
if (args.isEmpty()) {
@@ -239,7 +232,6 @@ class AclCommands extends FsCommand {
if (args.size() > 1) {
throw new HadoopIllegalArgumentException("Too many arguments");
}
- path = new Path(args.removeFirst());
}
@Override
@@ -253,59 +245,8 @@ class AclCommands extends FsCommand {
} else if (cf.getOpt("x")) {
item.fs.removeAclEntries(item.path, aclEntries);
} else if (cf.getOpt("-set")) {
- item.fs.setAcl(path, aclEntries);
- }
- }
-
- /**
- * Parse the aclSpec and returns the list of AclEntry objects.
- *
- * @param aclSpec
- * @return
- */
- private List<AclEntry> parseAclSpec(String aclSpec) {
- List<AclEntry> aclEntries = new ArrayList<AclEntry>();
- Collection<String> aclStrings = StringUtils.getStringCollection(aclSpec,
- ",");
- for (String aclStr : aclStrings) {
- AclEntry.Builder builder = new AclEntry.Builder();
- // Here "::" represent one empty string.
- // StringUtils.getStringCollection() will ignore this.
- String[] split = aclSpec.split(":");
- if (split.length != 3
- && !(split.length == 4 && DEFAULT.equals(split[0]))) {
- throw new HadoopIllegalArgumentException("Invalid <aclSpec> : "
- + aclStr);
- }
- int index = 0;
- if (split.length == 4) {
- assert DEFAULT.equals(split[0]);
- // default entry
- index++;
- builder.setScope(AclEntryScope.DEFAULT);
- }
- String type = split[index++];
- AclEntryType aclType = null;
- try {
- aclType = Enum.valueOf(AclEntryType.class, type.toUpperCase());
- builder.setType(aclType);
- } catch (IllegalArgumentException iae) {
- throw new HadoopIllegalArgumentException(
- "Invalid type of acl in <aclSpec> :" + aclStr);
- }
-
- builder.setName(split[index++]);
-
- String permission = split[index++];
- FsAction fsAction = FsAction.getFsAction(permission);
- if (null == fsAction) {
- throw new HadoopIllegalArgumentException(
- "Invalid permission in <aclSpec> : " + aclStr);
- }
- builder.setPermission(fsAction);
- aclEntries.add(builder.build());
+ item.fs.setAcl(item.path, aclEntries);
}
- return aclEntries;
}
}
}
Modified:
hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestAclCommands.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestAclCommands.java?rev=1559793&r1=1559792&r2=1559793&view=diff
==============================================================================
---
hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestAclCommands.java
(original)
+++
hadoop/common/branches/HDFS-4685/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestAclCommands.java
Mon Jan 20 18:00:06 2014
@@ -17,12 +17,18 @@
*/
package org.apache.hadoop.fs.shell;
-import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.*;
import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FsShell;
+import org.apache.hadoop.fs.permission.AclEntry;
+import org.apache.hadoop.fs.permission.AclEntryScope;
+import org.apache.hadoop.fs.permission.AclEntryType;
+import org.apache.hadoop.fs.permission.FsAction;
import org.apache.hadoop.util.ToolRunner;
import org.junit.Before;
import org.junit.Test;
@@ -57,6 +63,37 @@ public class TestAclCommands {
assertFalse("setfacl should fail with extra arguments",
0 == runCommand(new String[] { "-setfacl", "--set",
"default:user::rwx", "/path", "extra" }));
+ assertFalse("setfacl should fail with permissions for -x",
+ 0 == runCommand(new String[] { "-setfacl", "-x", "user:user1:rwx",
+ "/path" }));
+ assertFalse("setfacl should fail with permissions for -x",
+ 0 == runCommand(new String[] { "-setfacl", "-m",
+ "", "/path" }));
+ }
+
+ @Test
+ public void testMultipleAclSpecParsing() throws Exception {
+ List<AclEntry> parsedList = AclEntry.parseAclSpec(
+
"group::rwx,user:user1:rwx,user:user2:rw-,group:group1:rw-,default:group:group1:rw-",
true);
+
+ AclEntry basicAcl = new AclEntry.Builder().setType(AclEntryType.GROUP)
+ .setPermission(FsAction.ALL).build();
+ AclEntry user1Acl = new AclEntry.Builder().setType(AclEntryType.USER)
+ .setPermission(FsAction.ALL).setName("user1").build();
+ AclEntry user2Acl = new AclEntry.Builder().setType(AclEntryType.USER)
+ .setPermission(FsAction.READ_WRITE).setName("user2").build();
+ AclEntry group1Acl = new AclEntry.Builder().setType(AclEntryType.GROUP)
+ .setPermission(FsAction.READ_WRITE).setName("group1").build();
+ AclEntry defaultAcl = new AclEntry.Builder().setType(AclEntryType.GROUP)
+ .setPermission(FsAction.READ_WRITE).setName("group1")
+ .setScope(AclEntryScope.DEFAULT).build();
+ List<AclEntry> expectedList = new ArrayList<AclEntry>();
+ expectedList.add(basicAcl);
+ expectedList.add(user1Acl);
+ expectedList.add(user2Acl);
+ expectedList.add(group1Acl);
+ expectedList.add(defaultAcl);
+ assertEquals("Parsed Acl not correct", expectedList, parsedList);
}
private int runCommand(String[] commands) throws Exception {