HADOOP-9155. FsPermission should have different default value, 777 for directory and 666 for file. Contributed by Binglin Chang.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1431148 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Aaron Myers 2013-01-09 23:30:41 +00:00
parent b16dfc125d
commit 7e599d9e3b
13 changed files with 88 additions and 22 deletions

View File

@ -525,6 +525,9 @@ Release 2.0.3-alpha - Unreleased
HADOOP-9181. Set daemon flag for HttpServer's QueuedThreadPool.
(Liang Xie via suresh)
HADOOP-9155. FsPermission should have different default value, 777 for
directory and 666 for file. (Binglin Chang via atm)
Release 2.0.2-alpha - 2012-09-07
INCOMPATIBLE CHANGES

View File

@ -172,7 +172,25 @@
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.
@ -656,7 +674,7 @@ public FSDataOutputStream create(final Path f,
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 =
@ -704,7 +722,7 @@ public void mkdir(final Path dir, final FsPermission permission,
IOException {
final Path absDir = fixRelativePart(dir);
final FsPermission absFerms = (permission == null ?
FsPermission.getDefault() : permission).applyUMask(umask);
FsPermission.getDirDefault() : permission).applyUMask(umask);
new FSLinkResolver<Void>() {
@Override
public Void next(final AbstractFileSystem fs, final Path p)
@ -2157,7 +2175,7 @@ public boolean copy(final Path src, final Path dst, boolean deleteSource,
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,

View File

@ -79,8 +79,15 @@ public FileStatus(long length, boolean isdir,
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 void setPath(final Path p) {
*/
protected void setPermission(FsPermission permission) {
this.permission = (permission == null) ?
FsPermission.getDefault() : permission;
FsPermission.getFileDefault() : permission;
}
/**

View File

@ -850,7 +850,7 @@ public FSDataOutputStream create(Path f,
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);
}
@ -1030,7 +1030,7 @@ public FSDataOutputStream createNonRecursive(Path f,
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);
}
@ -1866,7 +1866,7 @@ protected Path getInitialWorkingDirectory() {
* 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());
}
/**

View File

@ -224,7 +224,7 @@ public FSDataOutputStream create(Path file, FsPermission permission,
}
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);
@ -484,7 +484,7 @@ private boolean mkdirs(FTPClient client, Path file, FsPermission permission)
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);

View File

@ -85,7 +85,7 @@ public void createSymlink(Path target, Path link, boolean createParent)
"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 {

View File

@ -275,11 +275,34 @@ public static void setUMask(Configuration conf, FsPermission umask) {
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-"

View File

@ -95,7 +95,7 @@ public void testCreatePermission() throws IOException {
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());
}

View File

@ -121,7 +121,7 @@ public void constructorNoOwner() throws IOException {
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 void constructorNoOwner() throws IOException {
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);
}
/**

View File

@ -24,6 +24,8 @@
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 {
@ -47,4 +49,14 @@ public void testFileContextNoCache() throws UnsupportedFileSystemException {
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());
}
}

View File

@ -73,7 +73,7 @@ public void testLocalFSsetPermission() throws IOException {
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));

View File

@ -1157,7 +1157,7 @@ public OutputStream create(String src, boolean overwrite, short replication,
/**
* Call {@link #create(String, FsPermission, EnumSet, short, long,
* Progressable, int, ChecksumOpt)} with default <code>permission</code>
* {@link FsPermission#getDefault()}.
* {@link FsPermission#getFileDefault()}.
*
* @param src File name
* @param overwrite overwrite an existing file if true
@ -1175,7 +1175,7 @@ public OutputStream create(String src,
Progressable progress,
int buffersize)
throws IOException {
return create(src, FsPermission.getDefault(),
return create(src, FsPermission.getFileDefault(),
overwrite ? EnumSet.of(CreateFlag.CREATE, CreateFlag.OVERWRITE)
: EnumSet.of(CreateFlag.CREATE), replication, blockSize, progress,
buffersize, null);
@ -1206,7 +1206,7 @@ public DFSOutputStream create(String src,
*
* @param src File name
* @param permission The permission of the directory being created.
* If null, use default permission {@link FsPermission#getDefault()}
* If null, use default permission {@link FsPermission#getFileDefault()}
* @param flag indicates create a new file or create/overwrite an
* existing file or append to an existing file
* @param createParent create missing parent directory if true
@ -1232,7 +1232,7 @@ public DFSOutputStream create(String src,
ChecksumOpt checksumOpt) throws IOException {
checkOpen();
if (permission == null) {
permission = FsPermission.getDefault();
permission = FsPermission.getFileDefault();
}
FsPermission masked = permission.applyUMask(dfsClientConf.uMask);
if(LOG.isDebugEnabled()) {

View File

@ -67,7 +67,10 @@ public HdfsFileStatus(long length, boolean isdir, int block_replication,
this.modification_time = modification_time;
this.access_time = access_time;
this.permission = (permission == null) ?
FsPermission.getDefault() : permission;
((isdir || symlink!=null) ?
FsPermission.getDefault() :
FsPermission.getFileDefault()) :
permission;
this.owner = (owner == null) ? "" : owner;
this.group = (group == null) ? "" : group;
this.symlink = symlink;