HADOOP-10220. Add ACL indicator bit to FsPermission. Contributed by Chris Nauroth.

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

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java

@@ -560,9 +560,6 @@ public class RawLocalFileSystem extends FileSystem {
         //expected format
         //-rw-------    1 username groupname ...
         String permission = t.nextToken();
-        if (permission.length() > 10) { //files with ACLs might have a '+'
-          permission = permission.substring(0, 10);
-        }
         setPermission(FsPermission.valueOf(permission));
         t.nextToken();
 

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/FsPermission.java

@@ -58,6 +58,7 @@ public class FsPermission implements Writable {
   private FsAction groupaction = null;
   private FsAction otheraction = null;
   private boolean stickyBit = false;
+  private boolean aclBit = false;
 
   private FsPermission() {}
 
@@ -72,7 +73,20 @@ public class FsPermission implements Writable {
   }
 
   public FsPermission(FsAction u, FsAction g, FsAction o, boolean sb) {
-    set(u, g, o, sb);
+    this(u, g, o, sb, false);
+  }
+
+  /**
+   * Construct by the given {@link FsAction} and special bits.
+   * @param u user action
+   * @param g group action
+   * @param o other action
+   * @param sb sticky bit
+   * @param ab ACL bit
+   */
+  public FsPermission(FsAction u, FsAction g, FsAction o, boolean sb,
+      boolean ab) {
+    set(u, g, o, sb, ab);
   }
 
   /**
@@ -92,6 +106,7 @@ public class FsPermission implements Writable {
     this.groupaction = other.groupaction;
     this.otheraction = other.otheraction;
     this.stickyBit = other.stickyBit;
+    this.aclBit = other.aclBit;
   }
   
   /**
@@ -112,17 +127,19 @@ public class FsPermission implements Writable {
   /** Return other {@link FsAction}. */
   public FsAction getOtherAction() {return otheraction;}
 
-  private void set(FsAction u, FsAction g, FsAction o, boolean sb) {
+  private void set(FsAction u, FsAction g, FsAction o, boolean sb, boolean ab) {
     useraction = u;
     groupaction = g;
     otheraction = o;
     stickyBit = sb;
+    aclBit = ab;
   }
 
   public void fromShort(short n) {
     FsAction[] v = FsAction.values();
 
-    set(v[(n >>> 6) & 7], v[(n >>> 3) & 7], v[n & 7], (((n >>> 9) & 1) == 1) );
+    set(v[(n >>> 6) & 7], v[(n >>> 3) & 7], v[n & 7], (((n >>> 9) & 1) == 1),
+      (((n >>> 10) & 1) == 1) );
   }
 
   @Override
@@ -148,7 +165,8 @@ public class FsPermission implements Writable {
    * Encode the object to a short.
    */
   public short toShort() {
-    int s =  (stickyBit ? 1 << 9 : 0)     |
+    int s =  (aclBit ? 1 << 10 : 0)       |
+             (stickyBit ? 1 << 9 : 0)     |
              (useraction.ordinal() << 6)  |
              (groupaction.ordinal() << 3) |
              otheraction.ordinal();
@@ -163,7 +181,8 @@ public class FsPermission implements Writable {
       return this.useraction == that.useraction
           && this.groupaction == that.groupaction
           && this.otheraction == that.otheraction
-          && this.stickyBit == that.stickyBit;
+          && this.stickyBit == that.stickyBit
+          && this.aclBit == that.aclBit;
     }
     return false;
   }
@@ -173,15 +192,19 @@ public class FsPermission implements Writable {
 
   @Override
   public String toString() {
-    String str = useraction.SYMBOL + groupaction.SYMBOL + otheraction.SYMBOL;
+    StringBuilder sb = new StringBuilder();
-    if(stickyBit) {
+    sb.append(useraction.SYMBOL);
-      StringBuilder str2 = new StringBuilder(str);
+    sb.append(groupaction.SYMBOL);
-      str2.replace(str2.length() - 1, str2.length(),
+    sb.append(otheraction.SYMBOL);
+    if (stickyBit) {
+      sb.replace(sb.length() - 1, sb.length(),
            otheraction.implies(FsAction.EXECUTE) ? "t" : "T");
-      str = str2.toString();
+    }
+    if (aclBit) {
+      sb.append('+');
     }
 
-    return str;
+    return sb.toString();
   }
 
   /**
@@ -269,6 +292,15 @@ public class FsPermission implements Writable {
     return stickyBit;
   }
 
+  /**
+   * Returns true if there is also an ACL (access control list).
+   *
+   * @return boolean true if there is also an ACL (access control list).
+   */
+  public boolean getAclBit() {
+    return aclBit;
+  }
+
   /** Set the user file creation mask (umask) */
   public static void setUMask(Configuration conf, FsPermission umask) {
     conf.set(UMASK_LABEL, String.format("%1$03o", umask.toShort()));
@@ -318,13 +350,14 @@ public class FsPermission implements Writable {
     if (unixSymbolicPermission == null) {
       return null;
     }
-    else if (unixSymbolicPermission.length() != 10) {
+    else if (unixSymbolicPermission.length() != 10 &&
-      throw new IllegalArgumentException("length != 10(unixSymbolicPermission="
+        unixSymbolicPermission.length() != 11) {
+      throw new IllegalArgumentException("invalid length(unixSymbolicPermission="
           + unixSymbolicPermission + ")");
     }
 
     int n = 0;
-    for(int i = 1; i < unixSymbolicPermission.length(); i++) {
+    for(int i = 1; i < 10; i++) {
       n = n << 1;
       char c = unixSymbolicPermission.charAt(i);
       n += (c == '-' || c == 'T' || c == 'S') ? 0: 1;
@@ -335,6 +368,11 @@ public class FsPermission implements Writable {
         unixSymbolicPermission.charAt(9) == 'T')
       n += 01000;
 
+    // Add ACL bit value if set
+    if (unixSymbolicPermission.length() == 11 &&
+         unixSymbolicPermission.charAt(10) == '+')
+      n += (1 << 10);
+
     return new FsPermission((short)n);
   }
   

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/permission/TestFsPermission.java

@@ -54,7 +54,7 @@ public class TestFsPermission extends TestCase {
    * the expected values back out for all combinations
    */
   public void testConvertingPermissions() {
-    for(short s = 0; s < 01777; s++) {
+    for(short s = 0; s <= 03777; s++) {
       assertEquals(s, new FsPermission(s).toShort());
     }
 
@@ -64,10 +64,21 @@ public class TestFsPermission extends TestCase {
       for(FsAction u : FsAction.values()) {
         for(FsAction g : FsAction.values()) {
           for(FsAction o : FsAction.values()) {
+            // Cover constructor with sticky bit.
             FsPermission f = new FsPermission(u, g, o, sb);
             assertEquals(s, f.toShort());
             FsPermission f2 = new FsPermission(f);
             assertEquals(s, f2.toShort());
+
+            // Cover constructor with sticky bit and ACL bit.
+            for(boolean ab : new boolean [] { false, true }) {
+              short s2 = (short)(ab ? s | (1 << 10) : s);
+              FsPermission f3 = new FsPermission(u, g, o, sb, ab);
+              assertEquals(s2, f3.toShort());
+              FsPermission f4 = new FsPermission(f3);
+              assertEquals(s2, f4.toShort());
+            }
+
             s++;
           }
         }
@@ -75,21 +86,33 @@ public class TestFsPermission extends TestCase {
     }
   }
 
-  public void testStickyBitToString() {
+  public void testSpecialBitsToString() {
-    // Check that every permission has its sticky bit represented correctly
+    for(boolean ab : new boolean [] { false, true }) {
-    for(boolean sb : new boolean [] { false, true }) {
+      for(boolean sb : new boolean [] { false, true }) {
-      for(FsAction u : FsAction.values()) {
+        for(FsAction u : FsAction.values()) {
-        for(FsAction g : FsAction.values()) {
+          for(FsAction g : FsAction.values()) {
-          for(FsAction o : FsAction.values()) {
+            for(FsAction o : FsAction.values()) {
-            FsPermission f = new FsPermission(u, g, o, sb);
+              FsPermission f = new FsPermission(u, g, o, sb, ab);
-            if(f.getStickyBit() && f.getOtherAction().implies(EXECUTE))
+              String fString = f.toString();
-              assertEquals('t', f.toString().charAt(8));
+
-            else if(f.getStickyBit() && !f.getOtherAction().implies(EXECUTE))
+              // Check that sticky bit is represented correctly.
-              assertEquals('T', f.toString().charAt(8));
+              if(f.getStickyBit() && f.getOtherAction().implies(EXECUTE))
-            else if(!f.getStickyBit()  && f.getOtherAction().implies(EXECUTE))
+                assertEquals('t', fString.charAt(8));
-              assertEquals('x', f.toString().charAt(8));
+              else if(f.getStickyBit() && !f.getOtherAction().implies(EXECUTE))
-            else
+                assertEquals('T', fString.charAt(8));
-              assertEquals('-', f.toString().charAt(8));
+              else if(!f.getStickyBit()  && f.getOtherAction().implies(EXECUTE))
+                assertEquals('x', fString.charAt(8));
+              else
+                assertEquals('-', fString.charAt(8));
+
+              // Check that ACL bit is represented correctly.
+              if (f.getAclBit()) {
+                assertEquals(10, fString.length());
+                assertEquals('+', fString.charAt(9));
+              } else {
+                assertEquals(9, fString.length());
+              }
+            }
           }
         }
       }
@@ -97,26 +120,35 @@ public class TestFsPermission extends TestCase {
   }
 
   public void testFsPermission() {
-      String symbolic = "-rwxrwxrwx";
+    String symbolic = "-rwxrwxrwx";
-      StringBuilder b = new StringBuilder("-123456789");
 
-      for(int i = 0; i < (1<<9); i++) {
+    for(int i = 0; i < (1 << 11); i++) {
-        for(int j = 1; j < 10; j++) {
+      StringBuilder b = new StringBuilder("----------");
-          b.setCharAt(j, '-');
+      String binary = String.format("%11s", Integer.toBinaryString(i));
+      String permBinary = binary.substring(2, binary.length());
+
+      int len = permBinary.length();
+      for(int j = 0; j < len; j++) {
+        if (permBinary.charAt(j) == '1') {
+          int k = 9 - (len - 1 - j);
+          b.setCharAt(k, symbolic.charAt(k));
         }
-        String binary = Integer.toBinaryString(i);
-
-        int len = binary.length();
-        for(int j = 0; j < len; j++) {
-          if (binary.charAt(j) == '1') {
-            int k = 9 - (len - 1 - j);
-            b.setCharAt(k, symbolic.charAt(k));
-          }
-        }
-
-        assertEquals(i, FsPermission.valueOf(b.toString()).toShort());
       }
+
+      // Check for sticky bit.
+      if (binary.charAt(1) == '1') {
+        char replacement = b.charAt(9) == 'x' ? 't' : 'T';
+        b.setCharAt(9, replacement);
+      }
+
+      // Check for ACL bit.
+      if (binary.charAt(0) == '1') {
+        b.append('+');
+      }
+
+      assertEquals(i, FsPermission.valueOf(b.toString()).toShort());
     }
+  }
 
   public void testUMaskParser() throws IOException {
     Configuration conf = new Configuration();

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

@@ -28,6 +28,8 @@ HDFS-4685 (Unreleased)
 
     HDFS-5673. Implement logic for modification of ACLs. (cnauroth)
 
+    HADOOP-10220. Add ACL indicator bit to FsPermission. (cnauroth)
+
   OPTIMIZATIONS
 
   BUG FIXES