From b98b344b9af99ce34657041b28a98cd3a8b5278d Mon Sep 17 00:00:00 2001 From: Chris Nauroth Date: Mon, 27 Jan 2014 18:03:30 +0000 Subject: [PATCH] HADOOP-10277. setfacl -x fails to parse ACL spec if trying to remove the mask entry. Contributed by Vinay. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/HDFS-4685@1561769 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/hadoop/fs/permission/AclEntry.java | 125 +++++++++++------- .../hadoop/fs/shell/TestAclCommands.java | 36 ++++- .../hadoop-hdfs/CHANGES-HDFS-4685.txt | 3 + 3 files changed, 115 insertions(+), 49 deletions(-) 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 7de61159558..e50be00528b 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 @@ public class AclEntry { * 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 class AclEntry { 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 6de73511873..7382bfff208 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 class TestAclCommands { 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 class TestAclCommands { @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 class TestAclCommands { 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 238b3410a8f..a42833ed79b 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)