Author: tgraves Date: Thu Jan 17 18:55:47 2013 New Revision: 1434864 URL: http://svn.apache.org/viewvc?rev=1434864&view=rev Log: HADOOP-9155. FsPermission should have different default value, 777 for directory and 666 for file (Binglin Chang via tgraves)
Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileContext.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileStatus.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ftp/FTPFileSystem.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/local/RawLocalFs.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/FsPermission.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextPermissionBase.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileStatus.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFSFileContextMainOperations.java hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystemPermission.java Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1434864&r1=1434863&r2=1434864&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt Thu Jan 17 18:55:47 2013 @@ -18,7 +18,10 @@ Release 0.23.7 - UNRELEASED HADOOP-8157. Fix race condition in Configuration that could cause spurious ClassNotFoundExceptions after a GC. (todd) - HADOOP-7886 Add toString to FileStatus (SreeHari via tgraves) + HADOOP-7886. Add toString to FileStatus (SreeHari via tgraves) + + HADOOP-9155. FsPermission should have different default value, 777 for + directory and 666 for file (Binglin Chang via tgraves) Release 0.23.6 - UNRELEASED Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileContext.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileContext.java?rev=1434864&r1=1434863&r2=1434864&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileContext.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileContext.java Thu Jan 17 18:55:47 2013 @@ -172,7 +172,25 @@ import org.apache.hadoop.util.ShutdownHo public final class FileContext { public static final Log LOG = LogFactory.getLog(FileContext.class); + /** + * Default permission for directory and symlink + * In previous versions, this default permission was also used to + * create files, so files created end up with ugo+x permission. + * See HADOOP-9155 for detail. + * Two new constants are added to solve this, please use + * {@link FileContext#DIR_DEFAULT_PERM} for directory, and use + * {@link FileContext#FILE_DEFAULT_PERM} for file. + * This constant is kept for compatibility. + */ public static final FsPermission DEFAULT_PERM = FsPermission.getDefault(); + /** + * Default permission for directory + */ + public static final FsPermission DIR_DEFAULT_PERM = FsPermission.getDirDefault(); + /** + * Default permission for file + */ + public static final FsPermission FILE_DEFAULT_PERM = FsPermission.getFileDefault(); /** * Priority of the FileContext shutdown hook. @@ -654,7 +672,7 @@ public final class FileContext { CreateOpts.Perms permOpt = (CreateOpts.Perms) CreateOpts.getOpt(CreateOpts.Perms.class, opts); FsPermission permission = (permOpt != null) ? permOpt.getValue() : - FsPermission.getDefault(); + FILE_DEFAULT_PERM; permission = permission.applyUMask(umask); final CreateOpts[] updatedOpts = @@ -701,7 +719,7 @@ public final class FileContext { IOException { final Path absDir = fixRelativePart(dir); final FsPermission absFerms = (permission == null ? - FsPermission.getDefault() : permission).applyUMask(umask); + FsPermission.getDirDefault() : permission).applyUMask(umask); new FSLinkResolver<Void>() { public Void next(final AbstractFileSystem fs, final Path p) throws IOException, UnresolvedLinkException { @@ -2135,7 +2153,7 @@ public final class FileContext { FileStatus fs = FileContext.this.getFileStatus(qSrc); if (fs.isDirectory()) { checkDependencies(qSrc, qDst); - mkdir(qDst, FsPermission.getDefault(), true); + mkdir(qDst, FsPermission.getDirDefault(), true); FileStatus[] contents = listStatus(qSrc); for (FileStatus content : contents) { copy(makeQualified(content.getPath()), makeQualified(new Path(qDst, Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileStatus.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileStatus.java?rev=1434864&r1=1434863&r2=1434864&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileStatus.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileStatus.java Thu Jan 17 18:55:47 2013 @@ -79,8 +79,15 @@ public class FileStatus implements Writa this.blocksize = blocksize; this.modification_time = modification_time; this.access_time = access_time; - this.permission = (permission == null) ? - FsPermission.getDefault() : permission; + if (permission != null) { + this.permission = permission; + } else if (isdir) { + this.permission = FsPermission.getDirDefault(); + } else if (symlink!=null) { + this.permission = FsPermission.getDefault(); + } else { + this.permission = FsPermission.getFileDefault(); + } this.owner = (owner == null) ? "" : owner; this.group = (group == null) ? "" : group; this.symlink = symlink; @@ -217,7 +224,7 @@ public class FileStatus implements Writa */ protected void setPermission(FsPermission permission) { this.permission = (permission == null) ? - FsPermission.getDefault() : permission; + FsPermission.getFileDefault() : permission; } /** Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java?rev=1434864&r1=1434863&r2=1434864&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java Thu Jan 17 18:55:47 2013 @@ -835,7 +835,7 @@ public abstract class FileSystem extends long blockSize, Progressable progress ) throws IOException { - return this.create(f, FsPermission.getDefault().applyUMask( + return this.create(f, FsPermission.getFileDefault().applyUMask( FsPermission.getUMask(getConf())), overwrite, bufferSize, replication, blockSize, progress); } @@ -993,7 +993,7 @@ public abstract class FileSystem extends boolean overwrite, int bufferSize, short replication, long blockSize, Progressable progress) throws IOException { - return this.createNonRecursive(f, FsPermission.getDefault(), + return this.createNonRecursive(f, FsPermission.getFileDefault(), overwrite, bufferSize, replication, blockSize, progress); } @@ -1804,7 +1804,7 @@ public abstract class FileSystem extends * Call {@link #mkdirs(Path, FsPermission)} with default permission. */ public boolean mkdirs(Path f) throws IOException { - return mkdirs(f, FsPermission.getDefault()); + return mkdirs(f, FsPermission.getDirDefault()); } /** Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ftp/FTPFileSystem.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ftp/FTPFileSystem.java?rev=1434864&r1=1434863&r2=1434864&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ftp/FTPFileSystem.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ftp/FTPFileSystem.java Thu Jan 17 18:55:47 2013 @@ -213,7 +213,7 @@ public class FTPFileSystem extends FileS } Path parent = absolute.getParent(); - if (parent == null || !mkdirs(client, parent, FsPermission.getDefault())) { + if (parent == null || !mkdirs(client, parent, FsPermission.getDirDefault())) { parent = (parent == null) ? new Path("/") : parent; disconnect(client); throw new IOException("create(): Mkdirs failed to create: " + parent); @@ -472,7 +472,7 @@ public class FTPFileSystem extends FileS if (!exists(client, absolute)) { Path parent = absolute.getParent(); created = (parent == null || mkdirs(client, parent, FsPermission - .getDefault())); + .getDirDefault())); if (created) { String parentDir = parent.toUri().getPath(); client.changeWorkingDirectory(parentDir); Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/local/RawLocalFs.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/local/RawLocalFs.java?rev=1434864&r1=1434863&r2=1434864&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/local/RawLocalFs.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/local/RawLocalFs.java Thu Jan 17 18:55:47 2013 @@ -85,7 +85,7 @@ public class RawLocalFs extends Delegate "system: "+target.toString()); } if (createParent) { - mkdir(link.getParent(), FsPermission.getDefault(), true); + mkdir(link.getParent(), FsPermission.getDirDefault(), true); } // NB: Use createSymbolicLink in java.nio.file.Path once available try { Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/FsPermission.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/FsPermission.java?rev=1434864&r1=1434863&r2=1434864&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/FsPermission.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/FsPermission.java Thu Jan 17 18:55:47 2013 @@ -263,12 +263,35 @@ public class FsPermission implements Wri conf.setInt(DEPRECATED_UMASK_LABEL, umask.toShort()); } - /** Get the default permission. */ + /** + * Get the default permission for directory and symlink. + * In previous versions, this default permission was also used to + * create files, so files created end up with ugo+x permission. + * See HADOOP-9155 for detail. + * Two new methods are added to solve this, please use + * {@link FsPermission#getDirDefault()} for directory, and use + * {@link FsPermission#getFileDefault()} for file. + * This method is kept for compatibility. + */ public static FsPermission getDefault() { return new FsPermission((short)00777); } /** + * Get the default permission for directory. + */ + public static FsPermission getDirDefault() { + return new FsPermission((short)00777); + } + + /** + * Get the default permission for file. + */ + public static FsPermission getFileDefault() { + return new FsPermission((short)00666); + } + + /** * Create a FsPermission from a Unix symbolic permission string * @param unixSymbolicPermission e.g. "-rw-rw-rw-" */ Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextPermissionBase.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextPermissionBase.java?rev=1434864&r1=1434863&r2=1434864&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextPermissionBase.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextPermissionBase.java Thu Jan 17 18:55:47 2013 @@ -95,7 +95,7 @@ public abstract class FileContextPermiss String filename = "foo"; Path f = getTestRootPath(fc, filename); createFile(fc, filename); - doFilePermissionCheck(FileContext.DEFAULT_PERM.applyUMask(fc.getUMask()), + doFilePermissionCheck(FileContext.FILE_DEFAULT_PERM.applyUMask(fc.getUMask()), fc.getFileStatus(f).getPermission()); } Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileStatus.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileStatus.java?rev=1434864&r1=1434863&r2=1434864&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileStatus.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileStatus.java Thu Jan 17 18:55:47 2013 @@ -121,7 +121,7 @@ public class TestFileStatus { FileStatus fileStatus = new FileStatus(LENGTH, isdir, REPLICATION, BLKSIZE, MTIME, PATH); validateAccessors(fileStatus, LENGTH, isdir, REPLICATION, BLKSIZE, MTIME, - 0, FsPermission.getDefault(), "", "", null, PATH); + 0, FsPermission.getDirDefault(), "", "", null, PATH); } /** @@ -131,7 +131,7 @@ public class TestFileStatus { public void constructorBlank() throws IOException { FileStatus fileStatus = new FileStatus(); validateAccessors(fileStatus, 0, false, 0, 0, 0, - 0, FsPermission.getDefault(), "", "", null, null); + 0, FsPermission.getFileDefault(), "", "", null, null); } /** Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFSFileContextMainOperations.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFSFileContextMainOperations.java?rev=1434864&r1=1434863&r2=1434864&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFSFileContextMainOperations.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFSFileContextMainOperations.java Thu Jan 17 18:55:47 2013 @@ -24,6 +24,8 @@ import org.apache.hadoop.conf.Configurat import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.apache.hadoop.fs.FileContextTestHelper; +import org.apache.hadoop.fs.permission.FsPermission; public class TestLocalFSFileContextMainOperations extends FileContextMainOperationsBaseTest { @@ -45,4 +47,14 @@ public class TestLocalFSFileContextMainO FileContext fc1 = FileContext.getLocalFSFileContext(); Assert.assertTrue(fc1 != fc); } + + @Test + public void testDefaultFilePermission() throws IOException { + Path file = FileContextTestHelper.getTestRootPath(fc, + "testDefaultFilePermission"); + FileContextTestHelper.createFile(fc, file); + FsPermission expect = FileContext.FILE_DEFAULT_PERM.applyUMask(fc.getUMask()); + Assert.assertEquals(expect, fc.getFileStatus(file) + .getPermission()); + } } Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystemPermission.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystemPermission.java?rev=1434864&r1=1434863&r2=1434864&view=diff ============================================================================== --- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystemPermission.java (original) +++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystemPermission.java Thu Jan 17 18:55:47 2013 @@ -73,7 +73,7 @@ public class TestLocalFileSystemPermissi try { FsPermission initialPermission = getPermission(localfs, f); System.out.println(filename + ": " + initialPermission); - assertEquals(FsPermission.getDefault().applyUMask(FsPermission.getUMask(conf)), initialPermission); + assertEquals(FsPermission.getFileDefault().applyUMask(FsPermission.getUMask(conf)), initialPermission); } catch(Exception e) { System.out.println(StringUtils.stringifyException(e));