HDFS-10689. Hdfs dfs chmod should reset sticky bit permission when the bit is omitted in the octal mode. (Manoj Govindassamy via Lei Xu)

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

@@ -55,7 +55,7 @@ public PermissionParser(String modeStr, Pattern symbolic, Pattern octal)
     if ((matcher = symbolic.matcher(modeStr)).find()) {
       applyNormalPattern(modeStr, matcher);
     } else if ((matcher = octal.matcher(modeStr)).matches()) {
-      applyOctalPattern(modeStr, matcher);
+      applyOctalPattern(matcher);
     } else {
       throw new IllegalArgumentException(modeStr);
     }
@@ -63,10 +63,10 @@ public PermissionParser(String modeStr, Pattern symbolic, Pattern octal)
 
   private void applyNormalPattern(String modeStr, Matcher matcher) {
     // Are there multiple permissions stored in one chmod?
-    boolean commaSeperated = false;
+    boolean commaSeparated = false;
 
     for (int i = 0; i < 1 || matcher.end() < modeStr.length(); i++) {
-      if (i > 0 && (!commaSeperated || !matcher.find())) {
+      if (i > 0 && (!commaSeparated || !matcher.find())) {
         throw new IllegalArgumentException(modeStr);
       }
 
@@ -144,19 +144,26 @@ private void applyNormalPattern(String modeStr, Matcher matcher) {
         stickyBitType = type;
       }
 
-      commaSeperated = matcher.group(4).contains(",");
+      commaSeparated = matcher.group(4).contains(",");
     }
     symbolic = true;
   }
 
-  private void applyOctalPattern(String modeStr, Matcher matcher) {
+  private void applyOctalPattern(final Matcher matcher) {
-    userType = groupType = othersType = '=';
+    // Matcher groups: 1: [01]  2: [0-7]{3}
+    final char typeApply = '=';
+    stickyBitType = typeApply;
+    userType = typeApply;
+    groupType = typeApply;
+    othersType = typeApply;
 
-    // Check if sticky bit is specified
+    // If sticky bit is specified get the bit, else
+    // default to reset for apply condition
     String sb = matcher.group(1);
     if (!sb.isEmpty()) {
       stickyMode = Short.valueOf(sb.substring(0, 1));
-      stickyBitType = '=';
+    } else {
+      stickyMode = 0;
     }
 
     String str = matcher.group(2);

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/permission/TestStickyBit.java

@@ -17,10 +17,11 @@
  */
 package org.apache.hadoop.fs.permission;
 
-import static org.apache.hadoop.fs.permission.AclEntryScope.*;
+import static org.apache.hadoop.fs.permission.AclEntryScope.ACCESS;
-import static org.apache.hadoop.fs.permission.AclEntryType.*;
+import static org.apache.hadoop.fs.permission.AclEntryScope.DEFAULT;
-import static org.apache.hadoop.fs.permission.FsAction.*;
+import static org.apache.hadoop.fs.permission.AclEntryType.USER;
-import static org.apache.hadoop.hdfs.server.namenode.AclTestHelpers.*;
+import static org.apache.hadoop.fs.permission.FsAction.ALL;
+import static org.apache.hadoop.hdfs.server.namenode.AclTestHelpers.aclEntry;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -46,6 +47,8 @@
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class TestStickyBit {
 
@@ -53,6 +56,7 @@ public class TestStickyBit {
     UserGroupInformation.createUserForTesting("theDoctor", new String[] {"tardis"});
   static final UserGroupInformation user2 =
     UserGroupInformation.createUserForTesting("rose", new String[] {"powellestates"});
+  static final Logger LOG = LoggerFactory.getLogger(TestStickyBit.class);
 
   private static MiniDFSCluster cluster;
   private static Configuration conf;
@@ -344,6 +348,52 @@ public void testStickyBitPersistence() throws Exception {
     assertFalse(hdfs.getFileStatus(sbSetOff).getPermission().getStickyBit());
   }
 
+  /**
+   * Sticky bit set on a directory can be reset either explicitly (like 0777)
+   * or by omitting the bit (like 777) in the permission. Ensure that the
+   * directory gets its sticky bit reset whenever it is omitted in permission.
+   */
+  @Test
+  public void testStickyBitReset() throws Exception {
+    Path sbExplicitTestDir = new Path("/DirToTestExplicitStickyBit");
+    Path sbOmittedTestDir = new Path("/DirToTestOmittedStickyBit");
+
+    // Creation of directories and verification of their existence
+    hdfs.mkdirs(sbExplicitTestDir);
+    hdfs.mkdirs(sbOmittedTestDir);
+    assertTrue(hdfs.exists(sbExplicitTestDir));
+    assertTrue(hdfs.exists(sbOmittedTestDir));
+
+    // Setting sticky bit explicitly on sbExplicitTestDir and verification
+    hdfs.setPermission(sbExplicitTestDir, new FsPermission((short) 01777));
+    LOG.info("Dir: {}, permission: {}", sbExplicitTestDir.getName(),
+            hdfs.getFileStatus(sbExplicitTestDir).getPermission());
+    assertTrue(hdfs.getFileStatus(sbExplicitTestDir).
+                  getPermission().getStickyBit());
+
+    // Sticky bit omitted on sbOmittedTestDir should behave like reset
+    hdfs.setPermission(sbOmittedTestDir, new FsPermission((short) 0777));
+    LOG.info("Dir: {}, permission: {}", sbOmittedTestDir.getName(),
+            hdfs.getFileStatus(sbOmittedTestDir).getPermission());
+    assertFalse(
+        hdfs.getFileStatus(sbOmittedTestDir).getPermission().getStickyBit());
+
+    // Resetting sticky bit explicitly on sbExplicitTestDir and verification
+    hdfs.setPermission(sbExplicitTestDir, new FsPermission((short) 00777));
+    LOG.info("Dir: {}, permission: {}", sbExplicitTestDir.getName(),
+            hdfs.getFileStatus(sbExplicitTestDir).getPermission());
+    assertFalse(
+        hdfs.getFileStatus(sbExplicitTestDir).getPermission().getStickyBit());
+
+    // Set the sticky bit and reset again by omitting in the permission
+    hdfs.setPermission(sbOmittedTestDir, new FsPermission((short) 01777));
+    hdfs.setPermission(sbOmittedTestDir, new FsPermission((short) 0777));
+    LOG.info("Dir: {}, permission: {}", sbOmittedTestDir.getName(),
+            hdfs.getFileStatus(sbOmittedTestDir).getPermission());
+    assertFalse(
+        hdfs.getFileStatus(sbOmittedTestDir).getPermission().getStickyBit());
+  }
+
   @Test
   public void testAclStickyBitPersistence() throws Exception {
     // A tale of three directories...

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java

@@ -1059,19 +1059,23 @@ void testChmod(Configuration conf, FileSystem fs, String chmodDir)
             fs.getFileStatus(dir2).getPermission());
 
         confirmPermissionChange("u=rwx,g=rx,o=rx", "rwxr-xr-x", fs, shell, dir2);
-
+        // sticky bit explicit set
         confirmPermissionChange("+t", "rwxr-xr-t", fs, shell, dir2);
-
+        // sticky bit explicit reset
         confirmPermissionChange("-t", "rwxr-xr-x", fs, shell, dir2);
-
         confirmPermissionChange("=t", "--------T", fs, shell, dir2);
-
+        // reset all permissions
         confirmPermissionChange("0000", "---------", fs, shell, dir2);
-
+        // turn on rw permissions for all
         confirmPermissionChange("1666", "rw-rw-rwT", fs, shell, dir2);
-
+        // sticky bit explicit set along with x permission
-        confirmPermissionChange("777", "rwxrwxrwt", fs, shell, dir2);
+        confirmPermissionChange("1777", "rwxrwxrwt", fs, shell, dir2);
-
+        // sticky bit explicit reset
+        confirmPermissionChange("0777", "rwxrwxrwx", fs, shell, dir2);
+        // sticky bit explicit set
+        confirmPermissionChange("1777", "rwxrwxrwt", fs, shell, dir2);
+        // sticky bit implicit reset
+        confirmPermissionChange("777", "rwxrwxrwx", fs, shell, dir2);
         fs.delete(dir2, true);
       } else {
         LOG.info("Skipped sticky bit tests on Windows");