Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 4900E200AEF for ; Wed, 4 May 2016 00:05:53 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 47C721609F9; Wed, 4 May 2016 00:05:53 +0200 (CEST) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 54B521609F7 for ; Wed, 4 May 2016 00:05:52 +0200 (CEST) Received: (qmail 37303 invoked by uid 500); 3 May 2016 22:05:48 -0000 Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list common-commits@hadoop.apache.org Received: (qmail 36856 invoked by uid 99); 3 May 2016 22:05:48 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 03 May 2016 22:05:48 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 6F5E0E78C5; Tue, 3 May 2016 22:05:48 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: aw@apache.org To: common-commits@hadoop.apache.org Date: Tue, 03 May 2016 22:06:02 -0000 Message-Id: In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [15/50] hadoop git commit: HADOOP-13052. ChecksumFileSystem mishandles crc file permissions. Contributed by Daryn Sharp. archived-at: Tue, 03 May 2016 22:05:53 -0000 HADOOP-13052. ChecksumFileSystem mishandles crc file permissions. Contributed by Daryn Sharp. (cherry picked from commit 9dbdc8e12d009e76635b2d20ce940851725cb069) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/989cd895 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/989cd895 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/989cd895 Branch: refs/heads/branch-2.8 Commit: 989cd895e367ba2c716951fef3c6a0a430eecc23 Parents: 3d3ed9b Author: Kihwal Lee Authored: Fri Apr 22 15:06:25 2016 -0500 Committer: Kihwal Lee Committed: Fri Apr 22 15:06:25 2016 -0500 ---------------------------------------------------------------------- .../apache/hadoop/fs/ChecksumFileSystem.java | 130 ++++++++++++++++--- .../hadoop/fs/TestChecksumFileSystem.java | 19 +++ 2 files changed, 134 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/989cd895/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java ---------------------------------------------------------------------- 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 641fdc2..46a235a 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,11 +24,13 @@ import java.io.IOException; import java.io.InputStream; import java.nio.channels.ClosedChannelException; import java.util.Arrays; +import java.util.List; import com.google.common.base.Preconditions; 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; @@ -155,11 +157,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); } } @@ -478,6 +483,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 @@ -488,16 +590,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); } /** http://git-wip-us.apache.org/repos/asf/hadoop/blob/989cd895/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java ---------------------------------------------------------------------- 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 923d219..41ae2e9 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.*; @@ -280,4 +283,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()); + } + } } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org For additional commands, e-mail: common-commits-help@hadoop.apache.org