diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index a67c7c1eab6..d33e5b33d8d 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -88,6 +88,9 @@ Release 2.7.3 - UNRELEASED HADOOP-12989. Some tests in org.apache.hadoop.fs.shell.find occasionally time out. (Takashi Ohnishi via aajisaka) + HADOOP-13052. ChecksumFileSystem mishandles crc file permissions. + (Daryn Sharp via kihwal) + Release 2.7.2 - 2016-01-25 INCOMPATIBLE CHANGES diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java index dddf0ce815f..c421a110ade 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java @@ -24,10 +24,12 @@ import java.io.IOException; import java.io.InputStream; import java.nio.channels.ClosedChannelException; import java.util.Arrays; +import java.util.List; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.permission.AclEntry; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.util.DataChecksum; import org.apache.hadoop.util.Progressable; @@ -151,11 +153,14 @@ public abstract class ChecksumFileSystem extends FilterFileSystem { throw new IOException("Not a checksum file: "+sumFile); this.bytesPerSum = sums.readInt(); set(fs.verifyChecksum, DataChecksum.newCrc32(), bytesPerSum, 4); - } catch (FileNotFoundException e) { // quietly ignore - set(fs.verifyChecksum, null, 1, 0); - } catch (IOException e) { // loudly ignore - LOG.warn("Problem opening checksum file: "+ file + - ". Ignoring exception: " , e); + } catch (IOException e) { + // mincing the message is terrible, but java throws permission + // exceptions as FNF because that's all the method signatures allow! + if (!(e instanceof FileNotFoundException) || + e.getMessage().endsWith(" (Permission denied)")) { + LOG.warn("Problem opening checksum file: "+ file + + ". Ignoring exception: " , e); + } set(fs.verifyChecksum, null, 1, 0); } } @@ -476,6 +481,103 @@ public abstract class ChecksumFileSystem extends FilterFileSystem { blockSize, progress); } + abstract class FsOperation { + boolean run(Path p) throws IOException { + boolean status = apply(p); + if (status) { + Path checkFile = getChecksumFile(p); + if (fs.exists(checkFile)) { + apply(checkFile); + } + } + return status; + } + abstract boolean apply(Path p) throws IOException; + } + + + @Override + public void setPermission(Path src, final FsPermission permission) + throws IOException { + new FsOperation(){ + @Override + boolean apply(Path p) throws IOException { + fs.setPermission(p, permission); + return true; + } + }.run(src); + } + + @Override + public void setOwner(Path src, final String username, final String groupname) + throws IOException { + new FsOperation(){ + @Override + boolean apply(Path p) throws IOException { + fs.setOwner(p, username, groupname); + return true; + } + }.run(src); + } + + @Override + public void setAcl(Path src, final List aclSpec) + throws IOException { + new FsOperation(){ + @Override + boolean apply(Path p) throws IOException { + fs.setAcl(p, aclSpec); + return true; + } + }.run(src); + } + + @Override + public void modifyAclEntries(Path src, final List aclSpec) + throws IOException { + new FsOperation(){ + @Override + boolean apply(Path p) throws IOException { + fs.modifyAclEntries(p, aclSpec); + return true; + } + }.run(src); + } + + @Override + public void removeAcl(Path src) throws IOException { + new FsOperation(){ + @Override + boolean apply(Path p) throws IOException { + fs.removeAcl(p); + return true; + } + }.run(src); + } + + @Override + public void removeAclEntries(Path src, final List aclSpec) + throws IOException { + new FsOperation(){ + @Override + boolean apply(Path p) throws IOException { + fs.removeAclEntries(p, aclSpec); + return true; + } + }.run(src); + } + + @Override + public void removeDefaultAcl(Path src) throws IOException { + new FsOperation(){ + @Override + boolean apply(Path p) throws IOException { + fs.removeDefaultAcl(p); + return true; + } + }.run(src); + } + /** * Set replication for an existing file. * Implement the abstract setReplication of FileSystem @@ -486,16 +588,14 @@ public abstract class ChecksumFileSystem extends FilterFileSystem { * false if file does not exist or is a directory */ @Override - public boolean setReplication(Path src, short replication) throws IOException { - boolean value = fs.setReplication(src, replication); - if (!value) - return false; - - Path checkFile = getChecksumFile(src); - if (exists(checkFile)) - fs.setReplication(checkFile, replication); - - return true; + public boolean setReplication(Path src, final short replication) + throws IOException { + return new FsOperation(){ + @Override + boolean apply(Path p) throws IOException { + return fs.setReplication(p, replication); + } + }.run(src); } /** diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java index 0c24ad59b69..f4181256f86 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java @@ -18,8 +18,11 @@ package org.apache.hadoop.fs; +import java.util.Arrays; + import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FSDataOutputStream; +import org.apache.hadoop.fs.permission.FsPermission; import static org.apache.hadoop.fs.FileSystemTestHelper.*; import org.apache.hadoop.conf.Configuration; import org.junit.*; @@ -257,4 +260,20 @@ public class TestChecksumFileSystem { assertTrue(localFs.rename(srcPath, dstPath)); assertTrue(localFs.exists(localFs.getChecksumFile(realDstPath))); } + + @Test + public void testSetPermissionCrc() throws Exception { + FileSystem rawFs = localFs.getRawFileSystem(); + Path p = new Path(TEST_ROOT_DIR, "testCrcPermissions"); + localFs.createNewFile(p); + Path crc = localFs.getChecksumFile(p); + assert(rawFs.exists(crc)); + + for (short mode : Arrays.asList((short)0666, (short)0660, (short)0600)) { + FsPermission perm = new FsPermission(mode); + localFs.setPermission(p, perm); + assertEquals(perm, localFs.getFileStatus(p).getPermission()); + assertEquals(perm, rawFs.getFileStatus(crc).getPermission()); + } + } }