HDFS-5849. Removing ACL from an inode fails if it has only a default ACL. Contributed by Chris Nauroth.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/HDFS-4685@1563205 13f79535-47bb-0310-9956-ffa450edef68

hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-4685.txt

@@ -67,3 +67,6 @@ HDFS-4685 (Unreleased)
 
     HADOOP-10277. setfacl -x fails to parse ACL spec if trying to remove the
     mask entry. (Vinay via cnauroth)
+
+    HDFS-5849. Removing ACL from an inode fails if it has only a default ACL.
+    (cnauroth)

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/AclStorage.java

@@ -26,6 +26,7 @@ import org.apache.hadoop.classification.InterfaceAudience;
 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.fs.permission.FsPermission;
 import org.apache.hadoop.hdfs.protocol.AclException;
 import org.apache.hadoop.hdfs.protocol.QuotaExceededException;
@@ -157,22 +158,27 @@ final class AclStorage {
       int snapshotId) throws QuotaExceededException {
     FsPermission perm = inode.getPermissionStatus(snapshotId).getPermission();
     if (perm.getAclBit()) {
-      // Restore group permissions from the feature's entry to permission bits,
-      // overwriting the mask, which is not part of a minimal ACL.
       List<AclEntry> featureEntries = inode.getAclFeature().getEntries();
-      AclEntry groupEntryKey = new AclEntry.Builder()
-        .setScope(AclEntryScope.ACCESS)
-        .setType(AclEntryType.GROUP)
-        .build();
-      int groupEntryIndex = Collections.binarySearch(featureEntries,
-        groupEntryKey, AclTransformation.ACL_ENTRY_COMPARATOR);
-      assert groupEntryIndex >= 0;
+      final FsAction groupPerm;
+      if (featureEntries.get(0).getScope() == AclEntryScope.ACCESS) {
+        // Restore group permissions from the feature's entry to permission
+        // bits, overwriting the mask, which is not part of a minimal ACL.
+        AclEntry groupEntryKey = new AclEntry.Builder()
+          .setScope(AclEntryScope.ACCESS)
+          .setType(AclEntryType.GROUP)
+          .build();
+        int groupEntryIndex = Collections.binarySearch(featureEntries,
+          groupEntryKey, AclTransformation.ACL_ENTRY_COMPARATOR);
+        assert groupEntryIndex >= 0;
+        groupPerm = featureEntries.get(groupEntryIndex).getPermission();
+      } else {
+        groupPerm = perm.getGroupAction();
+      }
 
       // Remove the feature and turn off the ACL bit.
       inode.removeAclFeature();
       FsPermission newPerm = new FsPermission(perm.getUserAction(),
-        featureEntries.get(groupEntryIndex).getPermission(),
-        perm.getOtherAction(),
+        groupPerm, perm.getOtherAction(),
         perm.getStickyBit(), false);
       inode.setPermission(newPerm, snapshotId);
     }

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeAcl.java

@@ -535,6 +535,23 @@ public class TestNameNodeAcl {
     assertAclFeature(false);
   }
 
+  @Test
+  public void testRemoveAclOnlyDefault() throws IOException {
+    FileSystem.mkdirs(fs, path, FsPermission.createImmutable((short)0750));
+    List<AclEntry> aclSpec = Lists.newArrayList(
+      aclEntry(ACCESS, USER, ALL),
+      aclEntry(ACCESS, GROUP, READ_EXECUTE),
+      aclEntry(ACCESS, OTHER, NONE),
+      aclEntry(DEFAULT, USER, "foo", ALL));
+    fs.setAcl(path, aclSpec);
+    fs.removeAcl(path);
+    AclStatus s = fs.getAclStatus(path);
+    AclEntry[] returned = s.getEntries().toArray(new AclEntry[0]);
+    assertArrayEquals(new AclEntry[] { }, returned);
+    assertPermission((short)0750);
+    assertAclFeature(false);
+  }
+
   @Test(expected=FileNotFoundException.class)
   public void testRemoveAclPathNotFound() throws IOException {
     // Path has not been created.