diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntry.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntry.java
index 7de6115955..e50be00528 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntry.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntry.java
@@ -210,7 +210,7 @@ private AclEntry(AclEntryType type, String name, FsAction permission, AclEntrySc
* But for removeAcl operation it will be false. i.e. AclSpec should
* not contain permissions.
* Example: "user:foo,group:bar"
- * @return Returns list of AclEntries parsed
+ * @return Returns list of {@link AclEntry} parsed
*/
public static List parseAclSpec(String aclSpec,
boolean includePermission) {
@@ -218,53 +218,84 @@ public static List parseAclSpec(String aclSpec,
Collection 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 : "
- + 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 :" + 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 : " + aclStr);
- }
- builder.setPermission(fsAction);
- }
- aclEntries.add(builder.build());
+ AclEntry aclEntry = parseAclEntry(aclStr, includePermission);
+ aclEntries.add(aclEntry);
}
return aclEntries;
}
+
+ /**
+ * Parses a string representation of an ACL into a AclEntry object.
+ *
+ * @param aclStr
+ * String representation of an ACL.
+ * Example: "user:foo:rw-"
+ * @param includePermission
+ * for setAcl operations this will be true. i.e. Acl should include
+ * permissions.
+ * But for removeAcl operation it will be false. i.e. Acl should not
+ * contain permissions.
+ * Example: "user:foo,group:bar,mask::"
+ * @return Returns an {@link AclEntry} object
+ */
+ public static AclEntry parseAclEntry(String aclStr,
+ boolean includePermission) {
+ AclEntry.Builder builder = new AclEntry.Builder();
+ // Here "::" represent one empty string.
+ // StringUtils.getStringCollection() will ignore this.
+ String[] split = aclStr.split(":");
+
+ if (split.length == 0) {
+ throw new HadoopIllegalArgumentException("Invalid : " + aclStr);
+ }
+ int index = 0;
+ if ("default".equals(split[0])) {
+ // default entry
+ index++;
+ builder.setScope(AclEntryScope.DEFAULT);
+ }
+
+ if (split.length <= index) {
+ throw new HadoopIllegalArgumentException("Invalid : " + aclStr);
+ }
+
+ AclEntryType aclType = null;
+ try {
+ aclType = Enum.valueOf(AclEntryType.class, split[index].toUpperCase());
+ builder.setType(aclType);
+ index++;
+ } catch (IllegalArgumentException iae) {
+ throw new HadoopIllegalArgumentException(
+ "Invalid type of acl in :" + aclStr);
+ }
+
+ if (split.length > index) {
+ String name = split[index];
+ if (!name.isEmpty()) {
+ builder.setName(name);
+ }
+ index++;
+ }
+
+ if (includePermission) {
+ if (split.length < index) {
+ throw new HadoopIllegalArgumentException("Invalid : "
+ + aclStr);
+ }
+ String permission = split[index];
+ FsAction fsAction = FsAction.getFsAction(permission);
+ if (null == fsAction) {
+ throw new HadoopIllegalArgumentException(
+ "Invalid permission in : " + aclStr);
+ }
+ builder.setPermission(fsAction);
+ index++;
+ }
+
+ if (split.length > index) {
+ throw new HadoopIllegalArgumentException("Invalid : " + aclStr);
+ }
+ AclEntry aclEntry = builder.build();
+ return aclEntry;
+ }
}
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestAclCommands.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestAclCommands.java
index 6de7351187..7382bfff20 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestAclCommands.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestAclCommands.java
@@ -66,7 +66,7 @@ public void testSetfaclValidations() throws Exception {
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",
+ assertFalse("setfacl should fail ACL spec missing",
0 == runCommand(new String[] { "-setfacl", "-m",
"", "/path" }));
}
@@ -74,7 +74,8 @@ public void testSetfaclValidations() throws Exception {
@Test
public void testMultipleAclSpecParsing() throws Exception {
List parsedList = AclEntry.parseAclSpec(
- "group::rwx,user:user1:rwx,user:user2:rw-,group:group1:rw-,default:group:group1:rw-", true);
+ "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();
@@ -96,6 +97,37 @@ public void testMultipleAclSpecParsing() throws Exception {
assertEquals("Parsed Acl not correct", expectedList, parsedList);
}
+ @Test
+ public void testMultipleAclSpecParsingWithoutPermissions() throws Exception {
+ List parsedList = AclEntry.parseAclSpec(
+ "user::,user:user1:,group::,group:group1:,mask::,other::,"
+ + "default:user:user1::,default:mask::", false);
+
+ AclEntry owner = new AclEntry.Builder().setType(AclEntryType.USER).build();
+ AclEntry namedUser = new AclEntry.Builder().setType(AclEntryType.USER)
+ .setName("user1").build();
+ AclEntry group = new AclEntry.Builder().setType(AclEntryType.GROUP).build();
+ AclEntry namedGroup = new AclEntry.Builder().setType(AclEntryType.GROUP)
+ .setName("group1").build();
+ AclEntry mask = new AclEntry.Builder().setType(AclEntryType.MASK).build();
+ AclEntry other = new AclEntry.Builder().setType(AclEntryType.OTHER).build();
+ AclEntry defaultUser = new AclEntry.Builder()
+ .setScope(AclEntryScope.DEFAULT).setType(AclEntryType.USER)
+ .setName("user1").build();
+ AclEntry defaultMask = new AclEntry.Builder()
+ .setScope(AclEntryScope.DEFAULT).setType(AclEntryType.MASK).build();
+ List expectedList = new ArrayList();
+ expectedList.add(owner);
+ expectedList.add(namedUser);
+ expectedList.add(group);
+ expectedList.add(namedGroup);
+ expectedList.add(mask);
+ expectedList.add(other);
+ expectedList.add(defaultUser);
+ expectedList.add(defaultMask);
+ assertEquals("Parsed Acl not correct", expectedList, parsedList);
+ }
+
private int runCommand(String[] commands) throws Exception {
return ToolRunner.run(conf, new FsShell(), commands);
}
diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-4685.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-4685.txt
index 238b3410a8..a42833ed79 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-4685.txt
+++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-4685.txt
@@ -58,3 +58,6 @@ HDFS-4685 (Unreleased)
(Vinay via cnauroth)
HDFS-5799. Make audit logging consistent across ACL APIs. (cnauroth)
+
+ HADOOP-10277. setfacl -x fails to parse ACL spec if trying to remove the
+ mask entry. (Vinay via cnauroth)