From a3d0cba81071977ffc22a42f60b2ca07da25ae15 Mon Sep 17 00:00:00 2001 From: Lei Xu Date: Thu, 28 Jul 2016 13:52:14 -0700 Subject: [PATCH] HDFS-10689. Hdfs dfs chmod should reset sticky bit permission when the bit is omitted in the octal mode. (Manoj Govindassamy via Lei Xu) --- .../fs/permission/PermissionParser.java | 23 +++++--- .../hadoop/fs/permission/TestStickyBit.java | 58 +++++++++++++++++-- .../org/apache/hadoop/hdfs/TestDFSShell.java | 20 ++++--- 3 files changed, 81 insertions(+), 20 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/PermissionParser.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/PermissionParser.java index 798fb5e12c..9156700751 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/PermissionParser.java +++ b/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) { - userType = groupType = othersType = '='; + private void applyOctalPattern(final Matcher matcher) { + // 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); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/permission/TestStickyBit.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/permission/TestStickyBit.java index d5cece4195..99f1b1b2f7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/permission/TestStickyBit.java +++ b/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.AclEntryType.*; -import static org.apache.hadoop.fs.permission.FsAction.*; -import static org.apache.hadoop.hdfs.server.namenode.AclTestHelpers.*; +import static org.apache.hadoop.fs.permission.AclEntryScope.ACCESS; +import static org.apache.hadoop.fs.permission.AclEntryScope.DEFAULT; +import static org.apache.hadoop.fs.permission.AclEntryType.USER; +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... diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java index 9cae762f34..dcf683e4c1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java +++ b/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); - - confirmPermissionChange("777", "rwxrwxrwt", fs, shell, dir2); - + // sticky bit explicit set along with x permission + 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");