From db6dfeca1a7d0ea863646384e31ca4f864f96b3f Mon Sep 17 00:00:00 2001 From: Kihwal Lee Date: Tue, 1 Nov 2016 08:26:25 -0500 Subject: [PATCH] HDFS-10997. Reduce number of path resolving methods. Contributed by Daryn Sharp. (cherry picked from commit 7c1a1834e49cf88c64837d92b78bbc07ea9e9efc) --- .../CacheReplicationMonitor.java | 14 +- .../hdfs/server/namenode/CacheManager.java | 8 +- .../namenode/EncryptionZoneManager.java | 3 +- .../hdfs/server/namenode/FSDirAclOp.java | 28 +-- .../hdfs/server/namenode/FSDirAppendOp.java | 3 +- .../hdfs/server/namenode/FSDirAttrOp.java | 20 +- .../hdfs/server/namenode/FSDirConcatOp.java | 8 +- .../hdfs/server/namenode/FSDirDeleteOp.java | 15 +- .../namenode/FSDirEncryptionZoneOp.java | 5 +- .../hdfs/server/namenode/FSDirMkdirOp.java | 18 +- .../hdfs/server/namenode/FSDirRenameOp.java | 24 +-- .../hdfs/server/namenode/FSDirSnapshotOp.java | 17 +- .../namenode/FSDirStatAndListingOp.java | 49 ++--- .../hdfs/server/namenode/FSDirSymlinkOp.java | 3 +- .../hdfs/server/namenode/FSDirTruncateOp.java | 9 +- .../server/namenode/FSDirWriteFileOp.java | 3 +- .../hdfs/server/namenode/FSDirXAttrOp.java | 12 +- .../hdfs/server/namenode/FSDirectory.java | 191 +++++++++++------- .../hdfs/server/namenode/FSEditLogLoader.java | 52 ++--- .../hdfs/server/namenode/FSImageFormat.java | 17 +- .../hdfs/server/namenode/FSNamesystem.java | 21 +- .../server/namenode/FSPermissionChecker.java | 165 ++++++++++++--- .../hdfs/server/namenode/INodesInPath.java | 90 +++------ .../namenode/snapshot/SnapshotManager.java | 5 +- .../apache/hadoop/hdfs/TestFileStatus.java | 4 +- .../hadoop/hdfs/TestReservedRawPaths.java | 5 +- .../hdfs/server/namenode/FSAclBaseTest.java | 7 +- .../hdfs/server/namenode/NameNodeAdapter.java | 5 +- .../hdfs/server/namenode/TestFSDirectory.java | 37 ++-- .../namenode/TestFSPermissionChecker.java | 5 +- .../server/namenode/TestFileTruncate.java | 5 +- .../hadoop/hdfs/server/namenode/TestFsck.java | 7 +- .../namenode/TestGetBlockLocations.java | 5 +- .../namenode/TestSnapshotPathINodes.java | 8 + .../namenode/snapshot/SnapshotTestHelper.java | 10 +- .../snapshot/TestSnapshotReplication.java | 3 +- .../security/TestPermissionSymlinks.java | 7 +- 37 files changed, 505 insertions(+), 383 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/CacheReplicationMonitor.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/CacheReplicationMonitor.java index ca8d72ac65e..3d13be06b7c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/CacheReplicationMonitor.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/CacheReplicationMonitor.java @@ -35,7 +35,6 @@ import java.util.concurrent.locks.ReentrantLock; import org.apache.hadoop.classification.InterfaceAudience; -import org.apache.hadoop.fs.UnresolvedLinkException; import org.apache.hadoop.hdfs.protocol.Block; import org.apache.hadoop.hdfs.protocol.CacheDirective; import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor.CachedBlocksList.Type; @@ -44,6 +43,7 @@ import org.apache.hadoop.hdfs.server.namenode.CachePool; import org.apache.hadoop.hdfs.server.namenode.CachedBlock; import org.apache.hadoop.hdfs.server.namenode.FSDirectory; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.FSNamesystem; import org.apache.hadoop.hdfs.server.namenode.INode; import org.apache.hadoop.hdfs.server.namenode.INodeDirectory; @@ -56,7 +56,6 @@ import org.slf4j.LoggerFactory; import com.google.common.base.Preconditions; -; /** * Scans the namesystem, scheduling blocks to be cached as appropriate. @@ -334,12 +333,11 @@ private void rescanCacheDirectives() { String path = directive.getPath(); INode node; try { - node = fsDir.getINode(path); - } catch (UnresolvedLinkException e) { - // We don't cache through symlinks - LOG.debug("Directive {}: got UnresolvedLinkException while resolving " - + "path {}", directive.getId(), path - ); + node = fsDir.getINode(path, DirOp.READ); + } catch (IOException e) { + // We don't cache through symlinks or invalid paths + LOG.debug("Directive {}: Failed to resolve path {} ({})", + directive.getId(), path, e.getMessage()); continue; } if (node == null) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/CacheManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/CacheManager.java index 4283b007d51..d628d608a6d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/CacheManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/CacheManager.java @@ -49,7 +49,6 @@ import org.apache.hadoop.fs.CacheFlag; import org.apache.hadoop.fs.InvalidRequestException; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.fs.UnresolvedLinkException; import org.apache.hadoop.fs.permission.FsAction; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hdfs.DFSUtil; @@ -72,6 +71,7 @@ import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor; import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor.CachedBlocksList; import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor.CachedBlocksList.Type; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.FsImageProto.CacheManagerSection; import org.apache.hadoop.hdfs.server.namenode.metrics.NameNodeMetrics; import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot; @@ -417,9 +417,9 @@ private CacheDirectiveStats computeNeeded(String path, short replication) { long requestedFiles = 0; CacheDirectiveStats.Builder builder = new CacheDirectiveStats.Builder(); try { - node = fsDir.getINode(path); - } catch (UnresolvedLinkException e) { - // We don't cache through symlinks + node = fsDir.getINode(path, DirOp.READ); + } catch (IOException e) { + // We don't cache through invalid paths return builder.build(); } if (node == null) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java index 0f6d4a6772f..323ebaba465 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java @@ -39,6 +39,7 @@ import org.apache.hadoop.hdfs.protocol.SnapshotAccessControlException; import org.apache.hadoop.hdfs.protocol.proto.HdfsProtos; import org.apache.hadoop.hdfs.protocolPB.PBHelperClient; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -371,7 +372,7 @@ BatchedListEntries listEncryptionZones(long prevId) contain a reference INode. */ final String pathName = getFullPathName(ezi); - INodesInPath iip = dir.getINodesInPath(pathName, false); + INodesInPath iip = dir.getINodesInPath(pathName, DirOp.READ_LINK); INode lastINode = iip.getLastINode(); if (lastINode == null || lastINode.getId() != ezi.getINodeId()) { continue; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAclOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAclOp.java index afafd789bb7..25ca09b9929 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAclOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAclOp.java @@ -26,6 +26,7 @@ import org.apache.hadoop.hdfs.DFSConfigKeys; import org.apache.hadoop.hdfs.protocol.AclException; import org.apache.hadoop.hdfs.protocol.HdfsFileStatus; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import java.io.IOException; import java.util.Collections; @@ -41,7 +42,7 @@ static HdfsFileStatus modifyAclEntries( INodesInPath iip; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, src); + iip = fsd.resolvePath(pc, src, DirOp.WRITE); src = iip.getPath(); fsd.checkOwner(pc, iip); INode inode = FSDirectory.resolveLastINode(iip); @@ -66,7 +67,7 @@ static HdfsFileStatus removeAclEntries( INodesInPath iip; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, src); + iip = fsd.resolvePath(pc, src, DirOp.WRITE); src = iip.getPath(); fsd.checkOwner(pc, iip); INode inode = FSDirectory.resolveLastINode(iip); @@ -90,7 +91,7 @@ static HdfsFileStatus removeDefaultAcl(FSDirectory fsd, final String srcArg) INodesInPath iip; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, src); + iip = fsd.resolvePath(pc, src, DirOp.WRITE); src = iip.getPath(); fsd.checkOwner(pc, iip); INode inode = FSDirectory.resolveLastINode(iip); @@ -114,7 +115,7 @@ static HdfsFileStatus removeAcl(FSDirectory fsd, final String srcArg) INodesInPath iip; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, src); + iip = fsd.resolvePath(pc, src, DirOp.WRITE); src = iip.getPath(); fsd.checkOwner(pc, iip); unprotectedRemoveAcl(fsd, iip); @@ -134,11 +135,10 @@ static HdfsFileStatus setAcl( INodesInPath iip; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, src); - src = iip.getPath(); + iip = fsd.resolvePath(pc, src, DirOp.WRITE); fsd.checkOwner(pc, iip); - List newAcl = unprotectedSetAcl(fsd, src, aclSpec, false); - fsd.getEditLog().logSetAcl(src, newAcl); + List newAcl = unprotectedSetAcl(fsd, iip, aclSpec, false); + fsd.getEditLog().logSetAcl(iip.getPath(), newAcl); } finally { fsd.writeUnlock(); } @@ -151,15 +151,12 @@ static AclStatus getAclStatus( FSPermissionChecker pc = fsd.getPermissionChecker(); fsd.readLock(); try { - INodesInPath iip = fsd.resolvePath(pc, src); + INodesInPath iip = fsd.resolvePath(pc, src, DirOp.READ); // There is no real inode for the path ending in ".snapshot", so return a // non-null, unpopulated AclStatus. This is similar to getFileInfo. if (iip.isDotSnapshotDir() && fsd.getINode4DotSnapshot(iip) != null) { return new AclStatus.Builder().owner("").group("").build(); } - if (fsd.isPermissionEnabled()) { - fsd.checkTraverse(pc, iip); - } INode inode = FSDirectory.resolveLastINode(iip); int snapshotId = iip.getPathSnapshotId(); List acl = AclStorage.readINodeAcl(fsd.getAttributes(iip)); @@ -174,12 +171,9 @@ static AclStatus getAclStatus( } } - static List unprotectedSetAcl( - FSDirectory fsd, String src, List aclSpec, boolean fromEdits) - throws IOException { + static List unprotectedSetAcl(FSDirectory fsd, INodesInPath iip, + List aclSpec, boolean fromEdits) throws IOException { assert fsd.hasWriteLock(); - final INodesInPath iip = fsd.getINodesInPath4Write( - FSDirectory.normalizePath(src), true); // ACL removal is logged to edits as OP_SET_ACL with an empty list. if (aclSpec.isEmpty()) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAppendOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAppendOp.java index 80203f19f4e..55d5567feca 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAppendOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAppendOp.java @@ -34,6 +34,7 @@ import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo; import org.apache.hadoop.hdfs.server.blockmanagement.BlockManager; import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.BlockUCState; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.FSNamesystem.RecoverLeaseOp; import org.apache.hadoop.hdfs.server.namenode.NameNodeLayoutVersion.Feature; import org.apache.hadoop.ipc.RetriableException; @@ -87,7 +88,7 @@ static LastBlockWithStatus appendFile(final FSNamesystem fsn, final INodesInPath iip; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, srcArg); + iip = fsd.resolvePath(pc, srcArg, DirOp.WRITE); // Verify that the destination does not exist as a directory already final INode inode = iip.getLastINode(); final String path = iip.getPath(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java index 864186c1b13..6c2506b16e2 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java @@ -34,6 +34,7 @@ import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo; import org.apache.hadoop.hdfs.server.blockmanagement.BlockManager; import org.apache.hadoop.hdfs.server.blockmanagement.BlockStoragePolicySuite; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.util.EnumCounters; import org.apache.hadoop.security.AccessControlException; @@ -59,7 +60,7 @@ static HdfsFileStatus setPermission( INodesInPath iip; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, src); + iip = fsd.resolvePath(pc, src, DirOp.WRITE); fsd.checkOwner(pc, iip); unprotectedSetPermission(fsd, iip, permission); } finally { @@ -79,7 +80,7 @@ static HdfsFileStatus setOwner( INodesInPath iip; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, src); + iip = fsd.resolvePath(pc, src, DirOp.WRITE); fsd.checkOwner(pc, iip); if (!pc.isSuperUser()) { if (username != null && !pc.getUser().equals(username)) { @@ -107,7 +108,7 @@ static HdfsFileStatus setTimes( INodesInPath iip; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, src); + iip = fsd.resolvePath(pc, src, DirOp.WRITE); // Write access is required to set access and modification times if (fsd.isPermissionEnabled()) { fsd.checkPathAccess(pc, iip, FsAction.WRITE); @@ -135,7 +136,7 @@ static boolean setReplication( FSPermissionChecker pc = fsd.getPermissionChecker(); fsd.writeLock(); try { - final INodesInPath iip = fsd.resolvePathForWrite(pc, src); + final INodesInPath iip = fsd.resolvePath(pc, src, DirOp.WRITE); if (fsd.isPermissionEnabled()) { fsd.checkPathAccess(pc, iip, FsAction.WRITE); } @@ -182,7 +183,7 @@ static HdfsFileStatus setStoragePolicy(FSDirectory fsd, BlockManager bm, INodesInPath iip; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, src); + iip = fsd.resolvePath(pc, src, DirOp.WRITE); if (fsd.isPermissionEnabled()) { fsd.checkPathAccess(pc, iip, FsAction.WRITE); @@ -206,7 +207,7 @@ static BlockStoragePolicy getStoragePolicy(FSDirectory fsd, BlockManager bm, FSPermissionChecker pc = fsd.getPermissionChecker(); fsd.readLock(); try { - final INodesInPath iip = fsd.resolvePath(pc, path, false); + final INodesInPath iip = fsd.resolvePath(pc, path, DirOp.READ_LINK); if (fsd.isPermissionEnabled()) { fsd.checkPathAccess(pc, iip, FsAction.READ); } @@ -226,10 +227,7 @@ static long getPreferredBlockSize(FSDirectory fsd, String src) FSPermissionChecker pc = fsd.getPermissionChecker(); fsd.readLock(); try { - final INodesInPath iip = fsd.resolvePath(pc, src, false); - if (fsd.isPermissionEnabled()) { - fsd.checkTraverse(pc, iip); - } + final INodesInPath iip = fsd.resolvePath(pc, src, DirOp.READ_LINK); return INodeFile.valueOf(iip.getLastINode(), iip.getPath()) .getPreferredBlockSize(); } finally { @@ -251,7 +249,7 @@ static void setQuota(FSDirectory fsd, String src, long nsQuota, long ssQuota, fsd.writeLock(); try { - INodesInPath iip = fsd.resolvePathForWrite(pc, src); + INodesInPath iip = fsd.resolvePath(pc, src, DirOp.WRITE); INodeDirectory changed = unprotectedSetQuota(fsd, iip, nsQuota, ssQuota, type); if (changed != null) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirConcatOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirConcatOp.java index 188575621d6..3749e8406d7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirConcatOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirConcatOp.java @@ -26,6 +26,7 @@ import org.apache.hadoop.hdfs.protocol.HdfsFileStatus; import org.apache.hadoop.hdfs.protocol.QuotaExceededException; import org.apache.hadoop.hdfs.protocol.SnapshotException; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import java.io.IOException; import java.util.Arrays; @@ -54,11 +55,10 @@ static HdfsFileStatus concat(FSDirectory fsd, String target, String[] srcs, if (FSDirectory.LOG.isDebugEnabled()) { FSDirectory.LOG.debug("concat {} to {}", Arrays.toString(srcs), target); } - final INodesInPath targetIIP = fsd.getINodesInPath4Write(target); + FSPermissionChecker pc = fsd.getPermissionChecker(); + final INodesInPath targetIIP = fsd.resolvePath(pc, target, DirOp.WRITE); // write permission for the target - FSPermissionChecker pc = null; if (fsd.isPermissionEnabled()) { - pc = fsd.getPermissionChecker(); fsd.checkPathAccess(pc, targetIIP, FsAction.WRITE); } @@ -125,7 +125,7 @@ private static INodeFile[] verifySrcFiles(FSDirectory fsd, String[] srcs, final INodeDirectory targetParent = targetINode.getParent(); // now check the srcs for(String src : srcs) { - final INodesInPath iip = fsd.getINodesInPath4Write(src); + final INodesInPath iip = fsd.resolvePath(pc, src, DirOp.WRITE); // permission check for srcs if (pc != null) { fsd.checkPathAccess(pc, iip, FsAction.READ); // read the file diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirDeleteOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirDeleteOp.java index b0995fc3091..32a7b796b67 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirDeleteOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirDeleteOp.java @@ -18,15 +18,18 @@ package org.apache.hadoop.hdfs.server.namenode; import org.apache.hadoop.fs.InvalidPathException; +import org.apache.hadoop.fs.ParentNotDirectoryException; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.PathIsNotEmptyDirectoryException; import org.apache.hadoop.fs.UnresolvedLinkException; import org.apache.hadoop.fs.permission.FsAction; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.INode.BlocksMapUpdateInfo; import org.apache.hadoop.hdfs.server.namenode.INode.ReclaimContext; import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.util.ChunkedArrayList; +import java.io.FileNotFoundException; import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -102,7 +105,7 @@ static BlocksMapUpdateInfo delete( throw new InvalidPathException(src); } - final INodesInPath iip = fsd.resolvePathForWrite(pc, src, false); + final INodesInPath iip = fsd.resolvePath(pc, src, DirOp.WRITE_LINK); if (fsd.isPermissionEnabled()) { fsd.checkPermission(pc, iip, false, null, FsAction.WRITE, null, FsAction.ALL, true); @@ -276,10 +279,14 @@ private static boolean unprotectedDelete(FSDirectory fsd, INodesInPath iip, * @param iip directory whose descendants are to be checked. * @throws AccessControlException if a non-empty protected descendant * was found. + * @throws ParentNotDirectoryException + * @throws UnresolvedLinkException + * @throws FileNotFoundException */ private static void checkProtectedDescendants( FSDirectory fsd, INodesInPath iip) - throws AccessControlException, UnresolvedLinkException { + throws AccessControlException, UnresolvedLinkException, + ParentNotDirectoryException { final SortedSet protectedDirs = fsd.getProtectedDirectories(); if (protectedDirs.isEmpty()) { return; @@ -298,8 +305,8 @@ private static void checkProtectedDescendants( // character after '/'. for (String descendant : protectedDirs.subSet(src + Path.SEPARATOR, src + "0")) { - if (fsd.isNonEmptyDirectory(fsd.getINodesInPath4Write( - descendant, false))) { + INodesInPath subdirIIP = fsd.getINodesInPath(descendant, DirOp.WRITE); + if (fsd.isNonEmptyDirectory(subdirIIP)) { throw new AccessControlException( "Cannot delete non-empty protected subdirectory " + descendant); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java index d7a36112781..d5f6be01b27 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java @@ -45,6 +45,7 @@ import org.apache.hadoop.hdfs.protocol.SnapshotAccessControlException; import org.apache.hadoop.hdfs.protocol.proto.HdfsProtos; import org.apache.hadoop.hdfs.protocolPB.PBHelperClient; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.security.SecurityUtil; import com.google.common.base.Preconditions; @@ -157,7 +158,7 @@ static HdfsFileStatus createEncryptionZone(final FSDirectory fsd, final INodesInPath iip; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, srcArg); + iip = fsd.resolvePath(pc, srcArg, DirOp.WRITE); final XAttr ezXAttr = fsd.ezManager.createEncryptionZone(iip, suite, version, keyName); xAttrs.add(ezXAttr); @@ -183,7 +184,7 @@ static Map.Entry getEZForPath( final EncryptionZone ret; fsd.readLock(); try { - iip = fsd.resolvePath(pc, srcArg); + iip = fsd.resolvePath(pc, srcArg, DirOp.READ); if (fsd.isPermissionEnabled()) { fsd.checkPathAccess(pc, iip, FsAction.READ); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirMkdirOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirMkdirOp.java index d0d050cff25..a7aa293a129 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirMkdirOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirMkdirOp.java @@ -19,7 +19,7 @@ import com.google.common.base.Preconditions; import org.apache.hadoop.fs.FileAlreadyExistsException; -import org.apache.hadoop.fs.InvalidPathException; +import org.apache.hadoop.fs.ParentNotDirectoryException; import org.apache.hadoop.fs.UnresolvedLinkException; import org.apache.hadoop.fs.permission.AclEntry; import org.apache.hadoop.fs.permission.FsAction; @@ -29,7 +29,9 @@ import org.apache.hadoop.hdfs.protocol.AclException; import org.apache.hadoop.hdfs.protocol.HdfsFileStatus; import org.apache.hadoop.hdfs.protocol.QuotaExceededException; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot; +import org.apache.hadoop.security.AccessControlException; import java.io.IOException; import java.util.List; @@ -43,17 +45,10 @@ static HdfsFileStatus mkdirs(FSNamesystem fsn, String src, if(NameNode.stateChangeLog.isDebugEnabled()) { NameNode.stateChangeLog.debug("DIR* NameSystem.mkdirs: " + src); } - if (!DFSUtil.isValidName(src)) { - throw new InvalidPathException(src); - } FSPermissionChecker pc = fsd.getPermissionChecker(); fsd.writeLock(); try { - INodesInPath iip = fsd.resolvePathForWrite(pc, src); - src = iip.getPath(); - if (fsd.isPermissionEnabled()) { - fsd.checkTraverse(pc, iip); - } + INodesInPath iip = fsd.resolvePath(pc, src, DirOp.CREATE); final INode lastINode = iip.getLastINode(); if (lastINode != null && lastINode.isFile()) { @@ -159,9 +154,10 @@ private static INodesInPath createParentDirectories(FSDirectory fsd, static void mkdirForEditLog(FSDirectory fsd, long inodeId, String src, PermissionStatus permissions, List aclEntries, long timestamp) throws QuotaExceededException, UnresolvedLinkException, AclException, - FileAlreadyExistsException { + FileAlreadyExistsException, ParentNotDirectoryException, + AccessControlException { assert fsd.hasWriteLock(); - INodesInPath iip = fsd.getINodesInPath(src, false); + INodesInPath iip = fsd.getINodesInPath(src, DirOp.WRITE_LINK); final byte[] localName = iip.getLastLocalName(); final INodesInPath existing = iip.getParentINodesInPath(); Preconditions.checkState(existing.getLastINode() != null); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java index e91e9e59401..a08e8b839a2 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java @@ -24,12 +24,12 @@ import org.apache.hadoop.fs.ParentNotDirectoryException; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.permission.FsAction; -import org.apache.hadoop.hdfs.DFSUtil; import org.apache.hadoop.hdfs.DistributedFileSystem; import org.apache.hadoop.hdfs.protocol.HdfsFileStatus; import org.apache.hadoop.hdfs.protocol.QuotaExceededException; import org.apache.hadoop.hdfs.protocol.SnapshotException; import org.apache.hadoop.hdfs.server.blockmanagement.BlockStoragePolicySuite; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.INode.BlocksMapUpdateInfo; import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot; import org.apache.hadoop.hdfs.util.ReadOnlyList; @@ -54,15 +54,12 @@ static RenameResult renameToInt( NameNode.stateChangeLog.debug("DIR* NameSystem.renameTo: " + src + " to " + dst); } - if (!DFSUtil.isValidName(dst)) { - throw new IOException("Invalid name: " + dst); - } FSPermissionChecker pc = fsd.getPermissionChecker(); // Rename does not operate on link targets // Do not resolveLink when checking permissions of src and dst - INodesInPath srcIIP = fsd.resolvePathForWrite(pc, src, false); - INodesInPath dstIIP = fsd.resolvePathForWrite(pc, dst, false); + INodesInPath srcIIP = fsd.resolvePath(pc, src, DirOp.WRITE_LINK); + INodesInPath dstIIP = fsd.resolvePath(pc, dst, DirOp.CREATE_LINK); dstIIP = dstForRenameTo(srcIIP, dstIIP); return renameTo(fsd, pc, srcIIP, dstIIP, logRetryCache); } @@ -115,8 +112,8 @@ static void verifyFsLimitsForRename(FSDirectory fsd, INodesInPath srcIIP, @Deprecated static INodesInPath renameForEditLog(FSDirectory fsd, String src, String dst, long timestamp) throws IOException { - final INodesInPath srcIIP = fsd.getINodesInPath4Write(src, false); - INodesInPath dstIIP = fsd.getINodesInPath4Write(dst, false); + final INodesInPath srcIIP = fsd.getINodesInPath(src, DirOp.WRITE_LINK); + INodesInPath dstIIP = fsd.getINodesInPath(dst, DirOp.WRITE_LINK); // this is wrong but accidentally works. the edit contains the full path // so the following will do nothing, but shouldn't change due to backward // compatibility when maybe full path wasn't logged. @@ -242,9 +239,6 @@ static RenameResult renameToInt( NameNode.stateChangeLog.debug("DIR* NameSystem.renameTo: with options -" + " " + src + " to " + dst); } - if (!DFSUtil.isValidName(dst)) { - throw new InvalidPathException("Invalid name: " + dst); - } final FSPermissionChecker pc = fsd.getPermissionChecker(); BlocksMapUpdateInfo collectedBlocks = new BlocksMapUpdateInfo(); @@ -260,8 +254,8 @@ static RenameResult renameTo(FSDirectory fsd, FSPermissionChecker pc, String src, String dst, BlocksMapUpdateInfo collectedBlocks, boolean logRetryCache,Options.Rename... options) throws IOException { - final INodesInPath srcIIP = fsd.resolvePathForWrite(pc, src, false); - final INodesInPath dstIIP = fsd.resolvePathForWrite(pc, dst, false); + final INodesInPath srcIIP = fsd.resolvePath(pc, src, DirOp.WRITE_LINK); + final INodesInPath dstIIP = fsd.resolvePath(pc, dst, DirOp.CREATE_LINK); if (fsd.isPermissionEnabled()) { // Rename does not operate on link targets // Do not resolveLink when checking permissions of src and dst @@ -312,8 +306,8 @@ static void renameForEditLog( Options.Rename... options) throws IOException { BlocksMapUpdateInfo collectedBlocks = new BlocksMapUpdateInfo(); - final INodesInPath srcIIP = fsd.getINodesInPath4Write(src, false); - final INodesInPath dstIIP = fsd.getINodesInPath4Write(dst, false); + final INodesInPath srcIIP = fsd.getINodesInPath(src, DirOp.WRITE_LINK); + final INodesInPath dstIIP = fsd.getINodesInPath(dst, DirOp.WRITE_LINK); unprotectedRenameTo(fsd, srcIIP, dstIIP, timestamp, collectedBlocks, options); if (!collectedBlocks.getToDeleteList().isEmpty()) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSnapshotOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSnapshotOp.java index ad282d164d9..ff076e48761 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSnapshotOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSnapshotOp.java @@ -26,6 +26,7 @@ import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport; import org.apache.hadoop.hdfs.protocol.SnapshotException; import org.apache.hadoop.hdfs.protocol.SnapshottableDirectoryStatus; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.snapshot.DirectorySnapshottableFeature; import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot; import org.apache.hadoop.hdfs.server.namenode.snapshot.SnapshotManager; @@ -84,9 +85,9 @@ static String createSnapshot( FSDirectory fsd, SnapshotManager snapshotManager, String snapshotRoot, String snapshotName, boolean logRetryCache) throws IOException { - final INodesInPath iip = fsd.getINodesInPath4Write(snapshotRoot); + FSPermissionChecker pc = fsd.getPermissionChecker(); + final INodesInPath iip = fsd.resolvePath(pc, snapshotRoot, DirOp.WRITE); if (fsd.isPermissionEnabled()) { - FSPermissionChecker pc = fsd.getPermissionChecker(); fsd.checkOwner(pc, iip); } @@ -114,9 +115,9 @@ static String createSnapshot( static void renameSnapshot(FSDirectory fsd, SnapshotManager snapshotManager, String path, String snapshotOldName, String snapshotNewName, boolean logRetryCache) throws IOException { - final INodesInPath iip = fsd.getINodesInPath4Write(path); + FSPermissionChecker pc = fsd.getPermissionChecker(); + final INodesInPath iip = fsd.resolvePath(pc, path, DirOp.WRITE); if (fsd.isPermissionEnabled()) { - FSPermissionChecker pc = fsd.getPermissionChecker(); fsd.checkOwner(pc, iip); } verifySnapshotName(fsd, snapshotNewName, path); @@ -150,11 +151,11 @@ static SnapshotDiffReport getSnapshotDiffReport(FSDirectory fsd, final FSPermissionChecker pc = fsd.getPermissionChecker(); fsd.readLock(); try { + INodesInPath iip = fsd.resolvePath(pc, path, DirOp.READ); if (fsd.isPermissionEnabled()) { checkSubtreeReadPermission(fsd, pc, path, fromSnapshot); checkSubtreeReadPermission(fsd, pc, path, toSnapshot); } - INodesInPath iip = fsd.getINodesInPath(path, true); diffs = snapshotManager.diff(iip, path, fromSnapshot, toSnapshot); } finally { fsd.readUnlock(); @@ -205,9 +206,9 @@ static INode.BlocksMapUpdateInfo deleteSnapshot( FSDirectory fsd, SnapshotManager snapshotManager, String snapshotRoot, String snapshotName, boolean logRetryCache) throws IOException { - final INodesInPath iip = fsd.getINodesInPath4Write(snapshotRoot); + FSPermissionChecker pc = fsd.getPermissionChecker(); + final INodesInPath iip = fsd.resolvePath(pc, snapshotRoot, DirOp.WRITE); if (fsd.isPermissionEnabled()) { - FSPermissionChecker pc = fsd.getPermissionChecker(); fsd.checkOwner(pc, iip); } @@ -238,7 +239,7 @@ private static void checkSubtreeReadPermission( final String fromPath = snapshot == null ? snapshottablePath : Snapshot.getSnapshotPath(snapshottablePath, snapshot); - INodesInPath iip = fsd.getINodesInPath(fromPath, true); + INodesInPath iip = fsd.resolvePath(pc, fromPath, DirOp.READ); fsd.checkPermission(pc, iip, false, null, null, FsAction.READ, FsAction.READ); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java index 63301309f01..2eb11150351 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirStatAndListingOp.java @@ -22,7 +22,6 @@ import org.apache.hadoop.fs.ContentSummary; import org.apache.hadoop.fs.DirectoryListingStartAfterNotFoundException; import org.apache.hadoop.fs.FileEncryptionInfo; -import org.apache.hadoop.fs.InvalidPathException; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.permission.FsAction; import org.apache.hadoop.fs.permission.FsPermission; @@ -37,9 +36,11 @@ import org.apache.hadoop.hdfs.protocol.LocatedBlocks; import org.apache.hadoop.hdfs.protocol.SnapshotException; import org.apache.hadoop.hdfs.server.blockmanagement.BlockManager; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.snapshot.DirectorySnapshottableFeature; import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot; import org.apache.hadoop.hdfs.util.ReadOnlyList; +import org.apache.hadoop.security.AccessControlException; import java.io.FileNotFoundException; import java.io.IOException; @@ -50,14 +51,8 @@ class FSDirStatAndListingOp { static DirectoryListing getListingInt(FSDirectory fsd, final String srcArg, byte[] startAfter, boolean needLocation) throws IOException { - final INodesInPath iip; - if (fsd.isPermissionEnabled()) { - FSPermissionChecker pc = fsd.getPermissionChecker(); - iip = fsd.resolvePath(pc, srcArg); - } else { - String src = FSDirectory.resolvePath(srcArg, fsd); - iip = fsd.getINodesInPath(src, true); - } + final FSPermissionChecker pc = fsd.getPermissionChecker(); + final INodesInPath iip = fsd.resolvePath(pc, srcArg, DirOp.READ); // Get file name when startAfter is an INodePath. This is not the // common case so avoid any unnecessary processing unless required. @@ -78,11 +73,8 @@ static DirectoryListing getListingInt(FSDirectory fsd, final String srcArg, boolean isSuperUser = true; if (fsd.isPermissionEnabled()) { - FSPermissionChecker pc = fsd.getPermissionChecker(); if (iip.getLastINode() != null && iip.getLastINode().isDirectory()) { fsd.checkPathAccess(pc, iip, FsAction.READ_EXECUTE); - } else { - fsd.checkTraverse(pc, iip); } isSuperUser = pc.isSuperUser(); } @@ -102,18 +94,20 @@ static DirectoryListing getListingInt(FSDirectory fsd, final String srcArg, static HdfsFileStatus getFileInfo( FSDirectory fsd, String srcArg, boolean resolveLink) throws IOException { - String src = srcArg; - if (!DFSUtil.isValidName(src)) { - throw new InvalidPathException("Invalid file name: " + src); - } + DirOp dirOp = resolveLink ? DirOp.READ : DirOp.READ_LINK; + FSPermissionChecker pc = fsd.getPermissionChecker(); final INodesInPath iip; - if (fsd.isPermissionEnabled()) { - FSPermissionChecker pc = fsd.getPermissionChecker(); - iip = fsd.resolvePath(pc, srcArg, resolveLink); - fsd.checkPermission(pc, iip, false, null, null, null, null, false); + if (pc.isSuperUser()) { + // superuser can only get an ACE if an existing ancestor is a file. + // right or (almost certainly) wrong, current fs contracts expect + // superuser to receive null instead. + try { + iip = fsd.resolvePath(pc, srcArg, dirOp); + } catch (AccessControlException ace) { + return null; + } } else { - src = FSDirectory.resolvePath(srcArg, fsd); - iip = fsd.getINodesInPath(src, resolveLink); + iip = fsd.resolvePath(pc, srcArg, dirOp); } return getFileInfo(fsd, iip); } @@ -123,17 +117,14 @@ static HdfsFileStatus getFileInfo( */ static boolean isFileClosed(FSDirectory fsd, String src) throws IOException { FSPermissionChecker pc = fsd.getPermissionChecker(); - final INodesInPath iip = fsd.resolvePath(pc, src); - if (fsd.isPermissionEnabled()) { - fsd.checkTraverse(pc, iip); - } + final INodesInPath iip = fsd.resolvePath(pc, src, DirOp.READ); return !INodeFile.valueOf(iip.getLastINode(), src).isUnderConstruction(); } static ContentSummary getContentSummary( FSDirectory fsd, String src) throws IOException { FSPermissionChecker pc = fsd.getPermissionChecker(); - final INodesInPath iip = fsd.resolvePath(pc, src, false); + final INodesInPath iip = fsd.resolvePath(pc, src, DirOp.READ_LINK); if (fsd.isPermissionEnabled()) { fsd.checkPermission(pc, iip, false, null, null, null, FsAction.READ_EXECUTE); @@ -156,7 +147,7 @@ static GetBlockLocationsResult getBlockLocations( BlockManager bm = fsd.getBlockManager(); fsd.readLock(); try { - final INodesInPath iip = fsd.resolvePath(pc, src); + final INodesInPath iip = fsd.resolvePath(pc, src, DirOp.READ); src = iip.getPath(); final INodeFile inode = INodeFile.valueOf(iip.getLastINode(), src); if (fsd.isPermissionEnabled()) { @@ -529,7 +520,7 @@ static QuotaUsage getQuotaUsage( final INodesInPath iip; fsd.readLock(); try { - iip = fsd.resolvePath(pc, src, false); + iip = fsd.resolvePath(pc, src, DirOp.READ_LINK); if (fsd.isPermissionEnabled()) { fsd.checkPermission(pc, iip, false, null, null, null, FsAction.READ_EXECUTE); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSymlinkOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSymlinkOp.java index 083a4c22a19..c5a738259a4 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSymlinkOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSymlinkOp.java @@ -25,6 +25,7 @@ import org.apache.hadoop.hdfs.DFSUtil; import org.apache.hadoop.hdfs.protocol.HdfsFileStatus; import org.apache.hadoop.hdfs.protocol.QuotaExceededException; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import java.io.IOException; @@ -55,7 +56,7 @@ static HdfsFileStatus createSymlinkInt( INodesInPath iip; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, link, false); + iip = fsd.resolvePath(pc, link, DirOp.WRITE_LINK); link = iip.getPath(); if (!createParent) { fsd.verifyParentDir(iip); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirTruncateOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirTruncateOp.java index 55a71192122..4184200a3d8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirTruncateOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirTruncateOp.java @@ -32,6 +32,7 @@ import org.apache.hadoop.hdfs.server.blockmanagement.BlockManager; import org.apache.hadoop.hdfs.server.blockmanagement.BlockUnderConstructionFeature; import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.BlockUCState; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.FSNamesystem.RecoverLeaseOp; import org.apache.hadoop.hdfs.server.namenode.INode.BlocksMapUpdateInfo; @@ -76,7 +77,7 @@ static TruncateResult truncate(final FSNamesystem fsn, final String srcArg, Block truncateBlock = null; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, srcArg); + iip = fsd.resolvePath(pc, srcArg, DirOp.WRITE); src = iip.getPath(); if (fsd.isPermissionEnabled()) { fsd.checkPathAccess(pc, iip, FsAction.WRITE); @@ -146,7 +147,7 @@ static TruncateResult truncate(final FSNamesystem fsn, final String srcArg, * {@link FSDirTruncateOp#truncate}, this will not schedule block recovery. * * @param fsn namespace - * @param src path name + * @param iip path name * @param clientName client name * @param clientMachine client machine info * @param newLength the target file size @@ -154,7 +155,8 @@ static TruncateResult truncate(final FSNamesystem fsn, final String srcArg, * @param truncateBlock truncate block * @throws IOException */ - static void unprotectedTruncate(final FSNamesystem fsn, final String src, + static void unprotectedTruncate(final FSNamesystem fsn, + final INodesInPath iip, final String clientName, final String clientMachine, final long newLength, final long mtime, final Block truncateBlock) throws UnresolvedLinkException, QuotaExceededException, @@ -162,7 +164,6 @@ static void unprotectedTruncate(final FSNamesystem fsn, final String src, assert fsn.hasWriteLock(); FSDirectory fsd = fsn.getFSDirectory(); - INodesInPath iip = fsd.getINodesInPath(src, true); INodeFile file = iip.getLastINode().asFile(); BlocksMapUpdateInfo collectedBlocks = new BlocksMapUpdateInfo(); boolean onBlockBoundary = unprotectedTruncate(fsn, iip, newLength, diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirWriteFileOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirWriteFileOp.java index 1305cf1f333..b0dfdce6a5c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirWriteFileOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirWriteFileOp.java @@ -45,6 +45,7 @@ import org.apache.hadoop.hdfs.server.blockmanagement.BlockUnderConstructionFeature; import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeStorageInfo; import org.apache.hadoop.hdfs.server.common.HdfsServerConstants; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot; import org.apache.hadoop.net.Node; import org.apache.hadoop.net.NodeBase; @@ -287,7 +288,7 @@ static Node getClientNode(BlockManager bm, String clientMachine) { static INodesInPath resolvePathForStartFile(FSDirectory dir, FSPermissionChecker pc, String src, EnumSet flag, boolean createParent) throws IOException { - INodesInPath iip = dir.resolvePathForWrite(pc, src); + INodesInPath iip = dir.resolvePath(pc, src, DirOp.CREATE); if (dir.isPermissionEnabled()) { dir.checkAncestorAccess(pc, iip, FsAction.WRITE); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirXAttrOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirXAttrOp.java index 6badf2472fc..f676f366c48 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirXAttrOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirXAttrOp.java @@ -30,6 +30,7 @@ import org.apache.hadoop.hdfs.protocol.HdfsFileStatus; import org.apache.hadoop.hdfs.protocol.proto.HdfsProtos; import org.apache.hadoop.hdfs.protocolPB.PBHelperClient; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.security.AccessControlException; import java.io.FileNotFoundException; @@ -72,7 +73,7 @@ static HdfsFileStatus setXAttr( INodesInPath iip; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, src); + iip = fsd.resolvePath(pc, src, DirOp.WRITE); src = iip.getPath(); checkXAttrChangeAccess(fsd, iip, xAttr, pc); unprotectedSetXAttrs(fsd, iip, xAttrs, flag); @@ -94,7 +95,7 @@ static List getXAttrs(FSDirectory fsd, final String srcArg, if (!getAll) { XAttrPermissionFilter.checkPermissionForApi(pc, xAttrs, isRawPath); } - final INodesInPath iip = fsd.resolvePath(pc, src); + final INodesInPath iip = fsd.resolvePath(pc, src, DirOp.READ); if (fsd.isPermissionEnabled()) { fsd.checkPathAccess(pc, iip, FsAction.READ); } @@ -133,7 +134,7 @@ static List listXAttrs( FSDirXAttrOp.checkXAttrsConfigFlag(fsd); final FSPermissionChecker pc = fsd.getPermissionChecker(); final boolean isRawPath = FSDirectory.isReservedRawName(src); - final INodesInPath iip = fsd.resolvePath(pc, src); + final INodesInPath iip = fsd.resolvePath(pc, src, DirOp.READ); if (fsd.isPermissionEnabled()) { /* To access xattr names, you need EXECUTE in the owning directory. */ fsd.checkParentAccess(pc, iip, FsAction.EXECUTE); @@ -165,7 +166,7 @@ static HdfsFileStatus removeXAttr( INodesInPath iip; fsd.writeLock(); try { - iip = fsd.resolvePathForWrite(pc, src); + iip = fsd.resolvePath(pc, src, DirOp.WRITE); src = iip.getPath(); checkXAttrChangeAccess(fsd, iip, xAttr, pc); @@ -186,8 +187,7 @@ static List unprotectedRemoveXAttrs( FSDirectory fsd, final String src, final List toRemove) throws IOException { assert fsd.hasWriteLock(); - INodesInPath iip = fsd.getINodesInPath4Write( - FSDirectory.normalizePath(src), true); + INodesInPath iip = fsd.getINodesInPath(src, DirOp.WRITE); INode inode = FSDirectory.resolveLastINode(iip); int snapshotId = iip.getLatestSnapshotId(); List existingXAttrs = XAttrStorage.readINodeXAttrs(inode); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java index c7a8d684cd0..3b191c7cb1f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java @@ -25,6 +25,7 @@ import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension; +import org.apache.hadoop.fs.InvalidPathException; import org.apache.hadoop.fs.ParentNotDirectoryException; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.StorageType; @@ -41,6 +42,7 @@ import org.apache.hadoop.hdfs.protocol.HdfsFileStatus; import org.apache.hadoop.hdfs.protocol.QuotaExceededException; import org.apache.hadoop.hdfs.protocol.SnapshotAccessControlException; +import org.apache.hadoop.hdfs.protocol.UnresolvedPathException; import org.apache.hadoop.hdfs.protocol.proto.HdfsProtos; import org.apache.hadoop.hdfs.protocolPB.PBHelperClient; import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo; @@ -231,6 +233,17 @@ public int getWriteHoldCount() { */ private final NameCache nameCache; + // used to specify path resolution type. *_LINK will return symlinks instead + // of throwing an unresolved exception + public enum DirOp { + READ, + READ_LINK, + WRITE, // disallows snapshot paths. + WRITE_LINK, + CREATE, // like write, but also blocks invalid path names. + CREATE_LINK; + }; + FSDirectory(FSNamesystem ns, Configuration conf) throws IOException { this.dirLock = new ReentrantReadWriteLock(true); // fair this.inodeId = new INodeId(); @@ -478,65 +491,73 @@ void disableQuotaChecks() { } /** - * This is a wrapper for resolvePath(). If the path passed - * is prefixed with /.reserved/raw, then it checks to ensure that the caller - * has super user privileges. + * Resolves a given path into an INodesInPath. All ancestor inodes that + * exist are validated as traversable directories. Symlinks in the ancestry + * will generate an UnresolvedLinkException. The returned IIP will be an + * accessible path that also passed additional sanity checks based on how + * the path will be used as specified by the DirOp. + * READ: Expands reserved paths and performs permission checks + * during traversal. Raw paths are only accessible by a superuser. + * WRITE: In addition to READ checks, ensures the path is not a + * snapshot path. + * CREATE: In addition to WRITE checks, ensures path does not contain + * illegal character sequences. * - * @param pc The permission checker used when resolving path. - * @param path The path to resolve. + * @param pc A permission checker for traversal checks. Pass null for + * no permission checks. + * @param src The path to resolve. + * @param dirOp The {@link DirOp} that controls additional checks. + * @param resolveLink If false, only ancestor symlinks will be checked. If + * true, the last inode will also be checked. * @return if the path indicates an inode, return path after replacing up to * with the corresponding path of the inode, else the path * in {@code src} as is. If the path refers to a path in the "raw" * directory, return the non-raw pathname. * @throws FileNotFoundException * @throws AccessControlException + * @throws ParentNotDirectoryException + * @throws UnresolvedLinkException */ - @VisibleForTesting - public INodesInPath resolvePath(FSPermissionChecker pc, String src) - throws UnresolvedLinkException, FileNotFoundException, - AccessControlException { - return resolvePath(pc, src, true); - } - @VisibleForTesting public INodesInPath resolvePath(FSPermissionChecker pc, String src, - boolean resolveLink) throws UnresolvedLinkException, - FileNotFoundException, AccessControlException { + DirOp dirOp) throws UnresolvedLinkException, FileNotFoundException, + AccessControlException, ParentNotDirectoryException { + boolean isCreate = (dirOp == DirOp.CREATE || dirOp == DirOp.CREATE_LINK); + // prevent creation of new invalid paths + if (isCreate && !DFSUtil.isValidName(src)) { + throw new InvalidPathException("Invalid file name: " + src); + } + byte[][] components = INode.getPathComponents(src); boolean isRaw = isReservedRawName(components); if (isPermissionEnabled && pc != null && isRaw) { pc.checkSuperuserPrivilege(); } components = resolveComponents(components, this); - return INodesInPath.resolve(rootDir, components, isRaw, resolveLink); - } - - INodesInPath resolvePathForWrite(FSPermissionChecker pc, String src) - throws UnresolvedLinkException, FileNotFoundException, - AccessControlException { - return resolvePathForWrite(pc, src, true); - } - - INodesInPath resolvePathForWrite(FSPermissionChecker pc, String src, - boolean resolveLink) throws UnresolvedLinkException, - FileNotFoundException, AccessControlException { - INodesInPath iip = resolvePath(pc, src, resolveLink); - if (iip.isSnapshot()) { - throw new SnapshotAccessControlException( - "Modification on a read-only snapshot is disallowed"); + INodesInPath iip = INodesInPath.resolve(rootDir, components, isRaw); + // verify all ancestors are dirs and traversable. note that only + // methods that create new namespace items have the signature to throw + // PNDE + try { + checkTraverse(pc, iip, dirOp); + } catch (ParentNotDirectoryException pnde) { + if (!isCreate) { + throw new AccessControlException(pnde.getMessage()); + } + throw pnde; } return iip; } INodesInPath resolvePath(FSPermissionChecker pc, String src, long fileId) throws UnresolvedLinkException, FileNotFoundException, - AccessControlException { + AccessControlException, ParentNotDirectoryException { // Older clients may not have given us an inode ID to work with. // In this case, we have to try to resolve the path and hope it // hasn't changed or been deleted since the file was opened for write. INodesInPath iip; if (fileId == HdfsConstants.GRANDFATHER_INODE_ID) { - iip = resolvePath(pc, src); + iip = resolvePath(pc, src, DirOp.WRITE); } else { INode inode = getInode(fileId); if (inode == null) { @@ -1483,63 +1504,57 @@ INode getINode4DotSnapshot(INodesInPath iip) throws UnresolvedLinkException { return null; } - INodesInPath getExistingPathINodes(byte[][] components) - throws UnresolvedLinkException { - return INodesInPath.resolve(rootDir, components, false); + /** + * Resolves the given path into inodes. Reserved paths are not handled and + * permissions are not verified. Client supplied paths should be + * resolved via {@link #resolvePath(FSPermissionChecker, String, DirOp)}. + * This method should only be used by internal methods. + * @return the {@link INodesInPath} containing all inodes in the path. + * @throws UnresolvedLinkException + * @throws ParentNotDirectoryException + * @throws AccessControlException + */ + public INodesInPath getINodesInPath(String src, DirOp dirOp) + throws UnresolvedLinkException, AccessControlException, + ParentNotDirectoryException { + return getINodesInPath(INode.getPathComponents(src), dirOp); + } + + public INodesInPath getINodesInPath(byte[][] components, DirOp dirOp) + throws UnresolvedLinkException, AccessControlException, + ParentNotDirectoryException { + INodesInPath iip = INodesInPath.resolve(rootDir, components); + checkTraverse(null, iip, dirOp); + return iip; } /** * Get {@link INode} associated with the file / directory. + * See {@link #getINode(String, DirOp)} */ - public INodesInPath getINodesInPath4Write(String src) - throws UnresolvedLinkException, SnapshotAccessControlException { - return getINodesInPath4Write(src, true); + @VisibleForTesting // should be removed after a lot of tests are updated + public INode getINode(String src) throws UnresolvedLinkException, + AccessControlException, ParentNotDirectoryException { + return getINode(src, DirOp.READ); } /** * Get {@link INode} associated with the file / directory. - * @throws SnapshotAccessControlException if path is in RO snapshot + * See {@link #getINode(String, DirOp)} */ + @VisibleForTesting // should be removed after a lot of tests are updated public INode getINode4Write(String src) throws UnresolvedLinkException, - SnapshotAccessControlException { - return getINodesInPath4Write(src, true).getLastINode(); - } - - /** @return the {@link INodesInPath} containing all inodes in the path. */ - public INodesInPath getINodesInPath(String path, boolean resolveLink) - throws UnresolvedLinkException { - final byte[][] components = INode.getPathComponents(path); - return INodesInPath.resolve(rootDir, components, resolveLink); - } - - /** @return the last inode in the path. */ - INode getINode(String path, boolean resolveLink) - throws UnresolvedLinkException { - return getINodesInPath(path, resolveLink).getLastINode(); + AccessControlException, FileNotFoundException, + ParentNotDirectoryException { + return getINode(src, DirOp.WRITE); } /** * Get {@link INode} associated with the file / directory. */ - public INode getINode(String src) throws UnresolvedLinkException { - return getINode(src, true); - } - - /** - * @return the INodesInPath of the components in src - * @throws UnresolvedLinkException if symlink can't be resolved - * @throws SnapshotAccessControlException if path is in RO snapshot - */ - INodesInPath getINodesInPath4Write(String src, boolean resolveLink) - throws UnresolvedLinkException, SnapshotAccessControlException { - final byte[][] components = INode.getPathComponents(src); - INodesInPath inodesInPath = INodesInPath.resolve(rootDir, components, - resolveLink); - if (inodesInPath.isSnapshot()) { - throw new SnapshotAccessControlException( - "Modification on a read-only snapshot is disallowed"); - } - return inodesInPath; + public INode getINode(String src, DirOp dirOp) throws UnresolvedLinkException, + AccessControlException, ParentNotDirectoryException { + return getINodesInPath(src, dirOp).getLastINode(); } FSPermissionChecker getPermissionChecker() @@ -1582,9 +1597,33 @@ void checkAncestorAccess(FSPermissionChecker pc, INodesInPath iip, checkPermission(pc, iip, false, access, null, null, null); } - void checkTraverse(FSPermissionChecker pc, INodesInPath iip) - throws AccessControlException { - checkPermission(pc, iip, false, null, null, null, null); + void checkTraverse(FSPermissionChecker pc, INodesInPath iip, + boolean resolveLink) throws AccessControlException, + UnresolvedPathException, ParentNotDirectoryException { + FSPermissionChecker.checkTraverse( + isPermissionEnabled ? pc : null, iip, resolveLink); + } + + void checkTraverse(FSPermissionChecker pc, INodesInPath iip, + DirOp dirOp) throws AccessControlException, UnresolvedPathException, + ParentNotDirectoryException { + final boolean resolveLink; + switch (dirOp) { + case READ_LINK: + case WRITE_LINK: + case CREATE_LINK: + resolveLink = false; + break; + default: + resolveLink = true; + break; + } + checkTraverse(pc, iip, resolveLink); + boolean allowSnapshot = (dirOp == DirOp.READ || dirOp == DirOp.READ_LINK); + if (!allowSnapshot && iip.isSnapshot()) { + throw new SnapshotAccessControlException( + "Modification on a read-only snapshot is disallowed"); + } } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java index 5b825930d3a..7dc43458b93 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java @@ -47,6 +47,7 @@ import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.RollingUpgradeStartupOption; import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.StartupOption; import org.apache.hadoop.hdfs.server.common.Storage; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.AddBlockOp; import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.AddCacheDirectiveInfoOp; import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.AddCachePoolOp; @@ -345,7 +346,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, // 3. OP_ADD to open file for append (old append) // See if the file already exists (persistBlocks call) - INodesInPath iip = fsDir.getINodesInPath(path, true); + INodesInPath iip = fsDir.getINodesInPath(path, DirOp.WRITE); INodeFile oldFile = INodeFile.valueOf(iip.getLastINode(), path, true); if (oldFile != null && addCloseOp.overwrite) { // This is OP_ADD with overwrite @@ -421,7 +422,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, " clientMachine " + addCloseOp.clientMachine); } - final INodesInPath iip = fsDir.getINodesInPath(path, true); + final INodesInPath iip = fsDir.getINodesInPath(path, DirOp.READ); final INodeFile file = INodeFile.valueOf(iip.getLastINode(), path); // Update the salient file attributes. @@ -457,7 +458,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, " clientMachine " + appendOp.clientMachine + " newBlock " + appendOp.newBlock); } - INodesInPath iip = fsDir.getINodesInPath4Write(path); + INodesInPath iip = fsDir.getINodesInPath(path, DirOp.WRITE); INodeFile file = INodeFile.valueOf(iip.getLastINode(), path); if (!file.isUnderConstruction()) { LocatedBlock lb = FSDirAppendOp.prepareFileForAppend(fsNamesys, iip, @@ -481,7 +482,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, FSNamesystem.LOG.debug(op.opCode + ": " + path + " numblocks : " + updateOp.blocks.length); } - INodesInPath iip = fsDir.getINodesInPath(path, true); + INodesInPath iip = fsDir.getINodesInPath(path, DirOp.READ); INodeFile oldFile = INodeFile.valueOf(iip.getLastINode(), path); // Update in-memory data structures updateBlocks(fsDir, updateOp, iip, oldFile); @@ -507,7 +508,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, SetReplicationOp setReplicationOp = (SetReplicationOp)op; String src = renameReservedPathsOnUpgrade( setReplicationOp.path, logVersion); - INodesInPath iip = fsDir.getINodesInPath4Write(src); + INodesInPath iip = fsDir.getINodesInPath(src, DirOp.WRITE); short replication = fsNamesys.getBlockManager().adjustReplication( setReplicationOp.replication); FSDirAttrOp.unprotectedSetReplication(fsDir, iip, replication); @@ -521,10 +522,10 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, srcs[i] = renameReservedPathsOnUpgrade(concatDeleteOp.srcs[i], logVersion); } - INodesInPath targetIIP = fsDir.getINodesInPath4Write(trg); + INodesInPath targetIIP = fsDir.getINodesInPath(trg, DirOp.WRITE); INodeFile[] srcFiles = new INodeFile[srcs.length]; for (int i = 0; i < srcs.length; i++) { - INodesInPath srcIIP = fsDir.getINodesInPath4Write(srcs[i]); + INodesInPath srcIIP = fsDir.getINodesInPath(srcs[i], DirOp.WRITE); srcFiles[i] = srcIIP.getLastINode().asFile(); } FSDirConcatOp.unprotectedConcat(fsDir, targetIIP, srcFiles, @@ -551,7 +552,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, DeleteOp deleteOp = (DeleteOp)op; final String src = renameReservedPathsOnUpgrade( deleteOp.path, logVersion); - final INodesInPath iip = fsDir.getINodesInPath4Write(src, false); + final INodesInPath iip = fsDir.getINodesInPath(src, DirOp.WRITE_LINK); FSDirDeleteOp.deleteForEditLog(fsDir, iip, deleteOp.timestamp); if (toAddRetryCache) { @@ -578,7 +579,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, SetPermissionsOp setPermissionsOp = (SetPermissionsOp)op; final String src = renameReservedPathsOnUpgrade(setPermissionsOp.src, logVersion); - final INodesInPath iip = fsDir.getINodesInPath4Write(src); + final INodesInPath iip = fsDir.getINodesInPath(src, DirOp.WRITE); FSDirAttrOp.unprotectedSetPermission(fsDir, iip, setPermissionsOp.permissions); break; @@ -587,7 +588,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, SetOwnerOp setOwnerOp = (SetOwnerOp)op; final String src = renameReservedPathsOnUpgrade( setOwnerOp.src, logVersion); - final INodesInPath iip = fsDir.getINodesInPath4Write(src); + final INodesInPath iip = fsDir.getINodesInPath(src, DirOp.WRITE); FSDirAttrOp.unprotectedSetOwner(fsDir, iip, setOwnerOp.username, setOwnerOp.groupname); break; @@ -596,7 +597,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, SetNSQuotaOp setNSQuotaOp = (SetNSQuotaOp)op; final String src = renameReservedPathsOnUpgrade( setNSQuotaOp.src, logVersion); - final INodesInPath iip = fsDir.getINodesInPath4Write(src); + final INodesInPath iip = fsDir.getINodesInPath(src, DirOp.WRITE); FSDirAttrOp.unprotectedSetQuota(fsDir, iip, setNSQuotaOp.nsQuota, HdfsConstants.QUOTA_DONT_SET, null); break; @@ -605,7 +606,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, ClearNSQuotaOp clearNSQuotaOp = (ClearNSQuotaOp)op; final String src = renameReservedPathsOnUpgrade( clearNSQuotaOp.src, logVersion); - final INodesInPath iip = fsDir.getINodesInPath4Write(src); + final INodesInPath iip = fsDir.getINodesInPath(src, DirOp.WRITE); FSDirAttrOp.unprotectedSetQuota(fsDir, iip, HdfsConstants.QUOTA_RESET, HdfsConstants.QUOTA_DONT_SET, null); break; @@ -614,7 +615,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, SetQuotaOp setQuotaOp = (SetQuotaOp) op; final String src = renameReservedPathsOnUpgrade( setQuotaOp.src, logVersion); - final INodesInPath iip = fsDir.getINodesInPath4Write(src); + final INodesInPath iip = fsDir.getINodesInPath(src, DirOp.WRITE); FSDirAttrOp.unprotectedSetQuota(fsDir, iip, setQuotaOp.nsQuota, setQuotaOp.dsQuota, null); break; @@ -624,7 +625,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, (FSEditLogOp.SetQuotaByStorageTypeOp) op; final String src = renameReservedPathsOnUpgrade( setQuotaByStorageTypeOp.src, logVersion); - final INodesInPath iip = fsDir.getINodesInPath4Write(src); + final INodesInPath iip = fsDir.getINodesInPath(src, DirOp.WRITE); FSDirAttrOp.unprotectedSetQuota(fsDir, iip, HdfsConstants.QUOTA_DONT_SET, setQuotaByStorageTypeOp.dsQuota, setQuotaByStorageTypeOp.type); @@ -634,7 +635,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, TimesOp timesOp = (TimesOp)op; final String src = renameReservedPathsOnUpgrade( timesOp.path, logVersion); - final INodesInPath iip = fsDir.getINodesInPath4Write(src); + final INodesInPath iip = fsDir.getINodesInPath(src, DirOp.WRITE); FSDirAttrOp.unprotectedSetTimes(fsDir, iip, timesOp.mtime, timesOp.atime, true); break; @@ -648,7 +649,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, lastInodeId); final String path = renameReservedPathsOnUpgrade(symlinkOp.path, logVersion); - final INodesInPath iip = fsDir.getINodesInPath(path, false); + final INodesInPath iip = fsDir.getINodesInPath(path, DirOp.WRITE_LINK); FSDirSymlinkOp.unprotectedAddSymlink(fsDir, iip.getExistingINodes(), iip.getLastLocalName(), inodeId, symlinkOp.value, symlinkOp.mtime, symlinkOp.atime, symlinkOp.permissionStatus); @@ -708,7 +709,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, reassignLeaseOp.leaseHolder); final String path = renameReservedPathsOnUpgrade(reassignLeaseOp.path, logVersion); - INodeFile pendingFile = fsDir.getINode(path).asFile(); + INodeFile pendingFile = fsDir.getINode(path, DirOp.READ).asFile(); Preconditions.checkState(pendingFile.isUnderConstruction()); fsNamesys.reassignLeaseInternal(lease, reassignLeaseOp.newHolder, pendingFile); @@ -724,7 +725,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, final String snapshotRoot = renameReservedPathsOnUpgrade(createSnapshotOp.snapshotRoot, logVersion); - INodesInPath iip = fsDir.getINodesInPath4Write(snapshotRoot); + INodesInPath iip = fsDir.getINodesInPath(snapshotRoot, DirOp.WRITE); String path = fsNamesys.getSnapshotManager().createSnapshot(iip, snapshotRoot, createSnapshotOp.snapshotName); if (toAddRetryCache) { @@ -740,7 +741,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, final String snapshotRoot = renameReservedPathsOnUpgrade(deleteSnapshotOp.snapshotRoot, logVersion); - INodesInPath iip = fsDir.getINodesInPath4Write(snapshotRoot); + INodesInPath iip = fsDir.getINodesInPath(snapshotRoot, DirOp.WRITE); fsNamesys.getSnapshotManager().deleteSnapshot(iip, deleteSnapshotOp.snapshotName, new INode.ReclaimContext(fsNamesys.dir.getBlockStoragePolicySuite(), @@ -761,7 +762,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, final String snapshotRoot = renameReservedPathsOnUpgrade(renameSnapshotOp.snapshotRoot, logVersion); - INodesInPath iip = fsDir.getINodesInPath4Write(snapshotRoot); + INodesInPath iip = fsDir.getINodesInPath(snapshotRoot, DirOp.WRITE); fsNamesys.getSnapshotManager().renameSnapshot(iip, snapshotRoot, renameSnapshotOp.snapshotOldName, renameSnapshotOp.snapshotNewName); @@ -886,13 +887,13 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, } case OP_SET_ACL: { SetAclOp setAclOp = (SetAclOp) op; - FSDirAclOp.unprotectedSetAcl(fsDir, setAclOp.src, setAclOp.aclEntries, - true); + INodesInPath iip = fsDir.getINodesInPath(setAclOp.src, DirOp.WRITE); + FSDirAclOp.unprotectedSetAcl(fsDir, iip, setAclOp.aclEntries, true); break; } case OP_SET_XATTR: { SetXAttrOp setXAttrOp = (SetXAttrOp) op; - INodesInPath iip = fsDir.getINodesInPath4Write(setXAttrOp.src); + INodesInPath iip = fsDir.getINodesInPath(setXAttrOp.src, DirOp.WRITE); FSDirXAttrOp.unprotectedSetXAttrs(fsDir, iip, setXAttrOp.xAttrs, EnumSet.of(XAttrSetFlag.CREATE, @@ -914,7 +915,8 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, } case OP_TRUNCATE: { TruncateOp truncateOp = (TruncateOp) op; - FSDirTruncateOp.unprotectedTruncate(fsNamesys, truncateOp.src, + INodesInPath iip = fsDir.getINodesInPath(truncateOp.src, DirOp.WRITE); + FSDirTruncateOp.unprotectedTruncate(fsNamesys, iip, truncateOp.clientName, truncateOp.clientMachine, truncateOp.newLength, truncateOp.timestamp, truncateOp.truncateBlock); break; @@ -923,7 +925,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, SetStoragePolicyOp setStoragePolicyOp = (SetStoragePolicyOp) op; final String path = renameReservedPathsOnUpgrade(setStoragePolicyOp.path, logVersion); - final INodesInPath iip = fsDir.getINodesInPath4Write(path); + final INodesInPath iip = fsDir.getINodesInPath(path, DirOp.WRITE); FSDirAttrOp.unprotectedSetStoragePolicy( fsDir, fsNamesys.getBlockManager(), iip, setStoragePolicyOp.policyId); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java index 49b641e51fd..7bbb1f286a3 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java @@ -24,7 +24,6 @@ import java.io.DataOutputStream; import java.io.File; import java.io.FileInputStream; -import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.security.DigestInputStream; @@ -44,8 +43,6 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.fs.PathIsNotDirectoryException; -import org.apache.hadoop.fs.UnresolvedLinkException; import org.apache.hadoop.fs.permission.PermissionStatus; import org.apache.hadoop.hdfs.DFSUtil; import org.apache.hadoop.hdfs.protocol.HdfsConstants; @@ -58,6 +55,7 @@ import org.apache.hadoop.hdfs.server.common.HdfsServerConstants; import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.StartupOption; import org.apache.hadoop.hdfs.server.common.InconsistentFSStateException; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.snapshot.DirectoryWithSnapshotFeature; import org.apache.hadoop.hdfs.server.namenode.snapshot.FileDiffList; import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot; @@ -599,7 +597,7 @@ private int loadDirectory(DataInput in, Counter counter) throws IOException { // Rename .snapshot paths if we're doing an upgrade parentPath = renameReservedPathsOnUpgrade(parentPath, getLayoutVersion()); final INodeDirectory parent = INodeDirectory.valueOf( - namesystem.dir.getINode(parentPath, true), parentPath); + namesystem.dir.getINode(parentPath, DirOp.READ), parentPath); return loadChildren(parent, in, counter); } @@ -650,15 +648,14 @@ private void loadFullNameINodes(long numFiles, DataInput in, Counter counter) } } - private INodeDirectory getParentINodeDirectory(byte[][] pathComponents - ) throws FileNotFoundException, PathIsNotDirectoryException, - UnresolvedLinkException { + private INodeDirectory getParentINodeDirectory(byte[][] pathComponents) + throws IOException { if (pathComponents.length < 2) { // root return null; } // Gets the parent INode - final INodesInPath inodes = namesystem.dir.getExistingPathINodes( - pathComponents); + final INodesInPath inodes = + namesystem.dir.getINodesInPath(pathComponents, DirOp.WRITE); return INodeDirectory.valueOf(inodes.getINode(-2), pathComponents); } @@ -952,7 +949,7 @@ LayoutVersion.Feature.ADD_INODE_ID, getLayoutVersion())) { inSnapshot = true; } else { path = renameReservedPathsOnUpgrade(path, getLayoutVersion()); - final INodesInPath iip = fsDir.getINodesInPath(path, true); + final INodesInPath iip = fsDir.getINodesInPath(path, DirOp.WRITE); oldnode = INodeFile.valueOf(iip.getLastINode(), path); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 33187393bf6..2e283bc4f5c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -231,6 +231,7 @@ import org.apache.hadoop.hdfs.server.common.Storage.StorageDirectory; import org.apache.hadoop.hdfs.server.common.Util; import org.apache.hadoop.hdfs.server.namenode.FSDirEncryptionZoneOp.EncryptionKeyInfo; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.FsImageProto.SecretManagerSection; import org.apache.hadoop.hdfs.server.namenode.INode.BlocksMapUpdateInfo; import org.apache.hadoop.hdfs.server.namenode.JournalSet.JournalAndStream; @@ -1782,7 +1783,7 @@ LocatedBlocks getBlockLocations(String clientMachine, String srcArg, * HDFS-7463. A better fix is to change the edit log of SetTime to * use inode id instead of a path. */ - final INodesInPath iip = dir.resolvePath(pc, srcArg); + final INodesInPath iip = dir.resolvePath(pc, srcArg, DirOp.READ); src = iip.getPath(); INode inode = iip.getLastINode(); @@ -2248,10 +2249,6 @@ private HdfsFileStatus startFileInt(String src, */ boolean recoverLease(String src, String holder, String clientMachine) throws IOException { - if (!DFSUtil.isValidName(src)) { - throw new IOException("Invalid file name: " + src); - } - boolean skipSync = false; FSPermissionChecker pc = getPermissionChecker(); checkOperation(OperationCategory.WRITE); @@ -2259,7 +2256,7 @@ boolean recoverLease(String src, String holder, String clientMachine) try { checkOperation(OperationCategory.WRITE); checkNameNodeSafeMode("Cannot recover the lease of " + src); - final INodesInPath iip = dir.resolvePathForWrite(pc, src); + final INodesInPath iip = dir.resolvePath(pc, src, DirOp.WRITE); src = iip.getPath(); final INodeFile inode = INodeFile.valueOf(iip.getLastINode(), src); if (!inode.isUnderConstruction()) { @@ -3308,12 +3305,14 @@ public boolean isInSnapshot(long blockCollectionID) { String fullName = bc.getName(); try { if (fullName != null && fullName.startsWith(Path.SEPARATOR) - && dir.getINode(fullName) == bc) { + && dir.getINode(fullName, DirOp.READ) == bc) { // If file exists in normal path then no need to look in snapshot return false; } - } catch (UnresolvedLinkException e) { - LOG.error("Error while resolving the link : " + fullName, e); + } catch (IOException e) { + // the snapshot path and current path may contain symlinks, ancestor + // dirs replaced by files, etc. + LOG.error("Error while resolving the path : " + fullName, e); return false; } /* @@ -6387,7 +6386,7 @@ List listCorruptFileBlocksWithSnapshot(String path, List lsf = new ArrayList<>(); if (snapshottableDirs != null) { for (String snap : snapshottableDirs) { - final INode isnap = getFSDirectory().getINode(snap, false); + final INode isnap = getFSDirectory().getINode(snap, DirOp.READ_LINK); final DirectorySnapshottableFeature sf = isnap.asDirectory().getDirectorySnapshottableFeature(); if (sf == null) { @@ -7371,7 +7370,7 @@ void checkAccess(String src, FsAction mode) throws IOException { readLock(); try { checkOperation(OperationCategory.READ); - final INodesInPath iip = dir.resolvePath(pc, src); + final INodesInPath iip = dir.resolvePath(pc, src, DirOp.READ); src = iip.getPath(); INode inode = iip.getLastINode(); if (inode == null) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java index c9b1c76b360..107d5630532 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java @@ -17,16 +17,19 @@ */ package org.apache.hadoop.hdfs.server.namenode; +import java.io.IOException; import java.util.Collection; import java.util.Stack; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.fs.ParentNotDirectoryException; import org.apache.hadoop.fs.permission.AclEntryScope; import org.apache.hadoop.fs.permission.AclEntryType; import org.apache.hadoop.fs.permission.FsAction; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hdfs.DFSUtil; +import org.apache.hadoop.hdfs.protocol.UnresolvedPathException; import org.apache.hadoop.hdfs.server.namenode.INodeAttributeProvider.AccessControlEnforcer; import org.apache.hadoop.hdfs.util.ReadOnlyList; import org.apache.hadoop.security.AccessControlException; @@ -42,12 +45,8 @@ class FSPermissionChecker implements AccessControlEnforcer { static final Log LOG = LogFactory.getLog(UserGroupInformation.class); - private static String constructPath(INodeAttributes[] inodes, int end) { - byte[][] components = new byte[end+1][]; - for (int i=0; i <= end; i++) { - components[i] = inodes[i].getLocalNameBytes(); - } - return DFSUtil.byteArray2PathString(components); + private static String getPath(byte[][] components, int start, int end) { + return DFSUtil.byteArray2PathString(components, start, end - start + 1); } /** @return a string for throwing {@link AccessControlException} */ @@ -203,21 +202,27 @@ public void checkPermission(String fsOwner, String supergroup, for(; ancestorIndex >= 0 && inodes[ancestorIndex] == null; ancestorIndex--); - checkTraverse(inodeAttrs, ancestorIndex); + try { + checkTraverse(inodeAttrs, inodes, components, ancestorIndex); + } catch (UnresolvedPathException | ParentNotDirectoryException ex) { + // must tunnel these exceptions out to avoid breaking interface for + // external enforcer + throw new TraverseAccessControlException(ex); + } final INodeAttributes last = inodeAttrs[inodeAttrs.length - 1]; if (parentAccess != null && parentAccess.implies(FsAction.WRITE) && inodeAttrs.length > 1 && last != null) { - checkStickyBit(inodeAttrs, inodeAttrs.length - 2); + checkStickyBit(inodeAttrs, components, inodeAttrs.length - 2); } if (ancestorAccess != null && inodeAttrs.length > 1) { - check(inodeAttrs, ancestorIndex, ancestorAccess); + check(inodeAttrs, components, ancestorIndex, ancestorAccess); } if (parentAccess != null && inodeAttrs.length > 1) { - check(inodeAttrs, inodeAttrs.length - 2, parentAccess); + check(inodeAttrs, components, inodeAttrs.length - 2, parentAccess); } if (access != null) { - check(inodeAttrs, inodeAttrs.length - 1, access); + check(inodeAttrs, components, inodeAttrs.length - 1, access); } if (subAccess != null) { INode rawLast = inodes[inodeAttrs.length - 1]; @@ -225,7 +230,7 @@ public void checkPermission(String fsOwner, String supergroup, snapshotId, subAccess, ignoreEmptyDir); } if (doCheckOwner) { - checkOwner(inodeAttrs, inodeAttrs.length - 1); + checkOwner(inodeAttrs, components, inodeAttrs.length - 1); } } @@ -243,29 +248,27 @@ private INodeAttributes getINodeAttrs(byte[][] pathByNameArr, int pathIdx, } /** Guarded by {@link FSNamesystem#readLock()} */ - private void checkOwner(INodeAttributes[] inodes, int i) + private void checkOwner(INodeAttributes[] inodes, byte[][] components, int i) throws AccessControlException { if (getUser().equals(inodes[i].getUserName())) { return; } throw new AccessControlException( "Permission denied. user=" + getUser() + - " is not the owner of inode=" + constructPath(inodes, i)); + " is not the owner of inode=" + getPath(components, 0, i)); } - /** Guarded by {@link FSNamesystem#readLock()} */ - private void checkTraverse(INodeAttributes[] inodeAttrs, int last) - throws AccessControlException { + /** Guarded by {@link FSNamesystem#readLock()} + * @throws AccessControlException + * @throws ParentNotDirectoryException + * @throws UnresolvedPathException + */ + private void checkTraverse(INodeAttributes[] inodeAttrs, INode[] inodes, + byte[][] components, int last) throws AccessControlException, + UnresolvedPathException, ParentNotDirectoryException { for (int i=0; i <= last; i++) { - INodeAttributes inode = inodeAttrs[i]; - if (!inode.isDirectory()) { - throw new AccessControlException( - constructPath(inodeAttrs, i) + " (is not a directory)"); - } - if (!hasPermission(inode, FsAction.EXECUTE)) { - throw new AccessControlException(toAccessControlString( - inode, constructPath(inodeAttrs, i), FsAction.EXECUTE)); - } + checkIsDirectory(inodes[i], components, i); + check(inodeAttrs, components, i, FsAction.EXECUTE); } } @@ -300,12 +303,12 @@ private void checkSubAccess(byte[][] components, int pathIdx, } /** Guarded by {@link FSNamesystem#readLock()} */ - private void check(INodeAttributes[] inodes, int i, FsAction access) - throws AccessControlException { + private void check(INodeAttributes[] inodes, byte[][] components, int i, + FsAction access) throws AccessControlException { INodeAttributes inode = (i >= 0) ? inodes[i] : null; if (inode != null && !hasPermission(inode, access)) { throw new AccessControlException( - toAccessControlString(inode, constructPath(inodes, i), access)); + toAccessControlString(inode, getPath(components, 0, i), access)); } } @@ -415,8 +418,8 @@ private boolean hasAclPermission(INodeAttributes inode, } /** Guarded by {@link FSNamesystem#readLock()} */ - private void checkStickyBit(INodeAttributes[] inodes, int index) - throws AccessControlException { + private void checkStickyBit(INodeAttributes[] inodes, byte[][] components, + int index) throws AccessControlException { INodeAttributes parent = inodes[index]; if (!parent.getFsPermission().getStickyBit()) { return; @@ -436,10 +439,10 @@ private void checkStickyBit(INodeAttributes[] inodes, int index) throw new AccessControlException(String.format( "Permission denied by sticky bit: user=%s, path=\"%s\":%s:%s:%s%s, " + "parent=\"%s\":%s:%s:%s%s", user, - constructPath(inodes, index + 1), + getPath(components, 0, index + 1), inode.getUserName(), inode.getGroupName(), inode.isDirectory() ? "d" : "-", inode.getFsPermission().toString(), - constructPath(inodes, index), + getPath(components, 0, index), parent.getUserName(), parent.getGroupName(), parent.isDirectory() ? "d" : "-", parent.getFsPermission().toString())); } @@ -472,4 +475,100 @@ public void checkPermission(CachePool pool, FsAction access) + pool.getPoolName() + ": user " + getUser() + " does not have " + access.toString() + " permissions."); } + + /** + * Verifies that all existing ancestors are directories. If a permission + * checker is provided then the user must have exec access. Ancestor + * symlinks will throw an unresolved exception, and resolveLink determines + * if the last inode will throw an unresolved exception. This method + * should always be called after a path is resolved into an IIP. + * @param pc for permission checker, null for no checking + * @param iip path to verify + * @param resolveLink whether last inode may be a symlink + * @throws AccessControlException + * @throws UnresolvedPathException + * @throws ParentNotDirectoryException + */ + static void checkTraverse(FSPermissionChecker pc, INodesInPath iip, + boolean resolveLink) throws AccessControlException, + UnresolvedPathException, ParentNotDirectoryException { + try { + if (pc == null || pc.isSuperUser()) { + checkSimpleTraverse(iip); + } else { + pc.checkPermission(iip, false, null, null, null, null, false); + } + } catch (TraverseAccessControlException tace) { + // unwrap the non-ACE (unresolved, parent not dir) exception + // tunneled out of checker. + tace.throwCause(); + } + // maybe check that the last inode is a symlink + if (resolveLink) { + int last = iip.length() - 1; + checkNotSymlink(iip.getINode(last), iip.getPathComponents(), last); + } + } + + // rudimentary permission-less directory check + private static void checkSimpleTraverse(INodesInPath iip) + throws UnresolvedPathException, ParentNotDirectoryException { + byte[][] components = iip.getPathComponents(); + for (int i=0; i < iip.length() - 1; i++) { + INode inode = iip.getINode(i); + if (inode == null) { + break; + } + checkIsDirectory(inode, components, i); + } + } + + private static void checkIsDirectory(INode inode, byte[][] components, int i) + throws UnresolvedPathException, ParentNotDirectoryException { + if (inode != null && !inode.isDirectory()) { + checkNotSymlink(inode, components, i); + throw new ParentNotDirectoryException( + getPath(components, 0, i) + " (is not a directory)"); + } + } + + private static void checkNotSymlink(INode inode, byte[][] components, int i) + throws UnresolvedPathException { + if (inode != null && inode.isSymlink()) { + final int last = components.length - 1; + final String path = getPath(components, 0, last); + final String preceding = getPath(components, 0, i - 1); + final String remainder = getPath(components, i + 1, last); + final String target = inode.asSymlink().getSymlinkString(); + if (LOG.isDebugEnabled()) { + final String link = inode.getLocalName(); + LOG.debug("UnresolvedPathException " + + " path: " + path + " preceding: " + preceding + + " count: " + i + " link: " + link + " target: " + target + + " remainder: " + remainder); + } + throw new UnresolvedPathException(path, preceding, remainder, target); + } + } + + //used to tunnel non-ACE exceptions encountered during path traversal. + //ops that create inodes are expected to throw ParentNotDirectoryExceptions. + //the signature of other methods requires the PNDE to be thrown as an ACE. + @SuppressWarnings("serial") + static class TraverseAccessControlException extends AccessControlException { + TraverseAccessControlException(IOException ioe) { + super(ioe); + } + public void throwCause() throws UnresolvedPathException, + ParentNotDirectoryException, AccessControlException { + Throwable ioe = getCause(); + if (ioe instanceof UnresolvedPathException) { + throw (UnresolvedPathException)ioe; + } + if (ioe instanceof ParentNotDirectoryException) { + throw (ParentNotDirectoryException)ioe; + } + throw this; + } + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodesInPath.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodesInPath.java index f05fa377ef8..b37321d6243 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodesInPath.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodesInPath.java @@ -24,11 +24,8 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.apache.hadoop.fs.Path; -import org.apache.hadoop.fs.UnresolvedLinkException; import org.apache.hadoop.hdfs.DFSUtil; import org.apache.hadoop.hdfs.protocol.HdfsConstants; -import org.apache.hadoop.hdfs.protocol.UnresolvedPathException; import org.apache.hadoop.hdfs.server.common.HdfsServerConstants; import org.apache.hadoop.hdfs.server.namenode.snapshot.DirectoryWithSnapshotFeature; import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot; @@ -77,34 +74,12 @@ static INodesInPath fromComponents(byte[][] components) { } /** - * Given some components, create a path name. - * @param components The path components - * @param start index - * @param end index - * @return concatenated path - */ - private static String constructPath(byte[][] components, int start, int end) { - StringBuilder buf = new StringBuilder(); - for (int i = start; i < end; i++) { - buf.append(DFSUtil.bytes2String(components[i])); - if (i < end - 1) { - buf.append(Path.SEPARATOR); - } - } - return buf.toString(); - } - - /** - * Retrieve existing INodes from a path. For non-snapshot path, - * the number of INodes is equal to the number of path components. For - * snapshot path (e.g., /foo/.snapshot/s1/bar), the number of INodes is - * (number_of_path_components - 1). - * - * An UnresolvedPathException is always thrown when an intermediate path - * component refers to a symbolic link. If the final path component refers - * to a symbolic link then an UnresolvedPathException is only thrown if - * resolveLink is true. - * + * Retrieve existing INodes from a path. The number of INodes is equal + * to the number of path components. For a snapshot path + * (e.g. /foo/.snapshot/s1/bar), the ".snapshot/s1" will be represented in + * one path component corresponding to its Snapshot.Root inode. This 1-1 + * mapping ensures the path can always be properly reconstructed. + * *

* Example:
* Given the path /c1/c2/c3 where only /c1/c2 exists, resulting in the @@ -118,19 +93,15 @@ private static String constructPath(byte[][] components, int start, int end) { * * @param startingDir the starting directory * @param components array of path component name - * @param resolveLink indicates whether UnresolvedLinkException should - * be thrown when the path refers to a symbolic link. * @return the specified number of existing INodes in the path */ static INodesInPath resolve(final INodeDirectory startingDir, - final byte[][] components, final boolean resolveLink) - throws UnresolvedLinkException { - return resolve(startingDir, components, false, resolveLink); + final byte[][] components) { + return resolve(startingDir, components, false); } static INodesInPath resolve(final INodeDirectory startingDir, - final byte[][] components, final boolean isRaw, - final boolean resolveLink) throws UnresolvedLinkException { + byte[][] components, final boolean isRaw) { Preconditions.checkArgument(startingDir.compareTo(components[0]) == 0); INode curNode = startingDir; @@ -179,30 +150,13 @@ static INodesInPath resolve(final INodeDirectory startingDir, } } } - if (curNode.isSymlink() && (!lastComp || resolveLink)) { - final String path = constructPath(components, 0, components.length); - final String preceding = constructPath(components, 0, count); - final String remainder = - constructPath(components, count + 1, components.length); - final String link = DFSUtil.bytes2String(components[count]); - final String target = curNode.asSymlink().getSymlinkString(); - if (LOG.isDebugEnabled()) { - LOG.debug("UnresolvedPathException " + - " path: " + path + " preceding: " + preceding + - " count: " + count + " link: " + link + " target: " + target + - " remainder: " + remainder); - } - throw new UnresolvedPathException(path, preceding, remainder, target); - } if (lastComp || !isDir) { break; } - final byte[] childName = components[count + 1]; - + + final byte[] childName = components[++count]; // check if the next byte[] in components is for ".snapshot" if (isDotSnapshotDir(childName) && dir.isSnapshottable()) { - // skip the ".snapshot" in components - count++; isSnapshot = true; // check if ".snapshot" is the last element of components if (count == components.length - 1) { @@ -216,19 +170,25 @@ static INodesInPath resolve(final INodeDirectory startingDir, curNode = s.getRoot(); snapshotId = s.getId(); } + // combine .snapshot & name into 1 component element to ensure + // 1-to-1 correspondence between components and inodes arrays is + // preserved so a path can be reconstructed. + byte[][] componentsCopy = + Arrays.copyOf(components, components.length - 1); + componentsCopy[count] = DFSUtil.string2Bytes( + DFSUtil.byteArray2PathString(components, count, 2)); + // shift the remaining components after snapshot name + int start = count + 2; + System.arraycopy(components, start, componentsCopy, count + 1, + components.length - start); + components = componentsCopy; + // reduce the inodes array to compensate for reduction in components + inodes = Arrays.copyOf(inodes, components.length); } else { // normal case, and also for resolving file/dir under snapshot root curNode = dir.getChild(childName, isSnapshot ? snapshotId : CURRENT_STATE_ID); } - count++; - } - if (isSnapshot && !isDotSnapshotDir(components[components.length - 1])) { - // for snapshot path shrink the inode array. however, for path ending with - // .snapshot, still keep last the null inode in the array - INode[] newNodes = new INode[components.length - 1]; - System.arraycopy(inodes, 0, newNodes, 0, newNodes.length); - inodes = newNodes; } return new INodesInPath(inodes, components, isRaw, isSnapshot, snapshotId); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java index c738d64381b..8ad78249a89 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java @@ -37,6 +37,7 @@ import org.apache.hadoop.hdfs.protocol.SnapshottableDirectoryStatus; import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport.DiffReportEntry; import org.apache.hadoop.hdfs.server.namenode.FSDirectory; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.FSImageFormat; import org.apache.hadoop.hdfs.server.namenode.FSNamesystem; import org.apache.hadoop.hdfs.server.namenode.INode; @@ -108,7 +109,7 @@ private void checkNestedSnapshottable(INodeDirectory dir, String path) */ public void setSnapshottable(final String path, boolean checkNestedSnapshottable) throws IOException { - final INodesInPath iip = fsdir.getINodesInPath4Write(path); + final INodesInPath iip = fsdir.getINodesInPath(path, DirOp.WRITE); final INodeDirectory d = INodeDirectory.valueOf(iip.getLastINode(), path); if (checkNestedSnapshottable) { checkNestedSnapshottable(d, path); @@ -149,7 +150,7 @@ public void removeSnapshottable(List toRemove) { * @throws SnapshotException if there are snapshots in the directory. */ public void resetSnapshottable(final String path) throws IOException { - final INodesInPath iip = fsdir.getINodesInPath4Write(path); + final INodesInPath iip = fsdir.getINodesInPath(path, DirOp.WRITE); final INodeDirectory d = INodeDirectory.valueOf(iip.getLastINode(), path); DirectorySnapshottableFeature sf = d.getDirectorySnapshottableFeature(); if (sf == null) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileStatus.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileStatus.java index ad13bf21d66..a2e6959146a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileStatus.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileStatus.java @@ -134,8 +134,8 @@ public void testGetFileInfo() throws IOException { dfsClient.getFileInfo("non-absolute"); fail("getFileInfo for a non-absolute path did not throw IOException"); } catch (RemoteException re) { - assertTrue("Wrong exception for invalid file name", - re.toString().contains("Invalid file name")); + assertTrue("Wrong exception for invalid file name: "+re, + re.toString().contains("Absolute path required")); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReservedRawPaths.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReservedRawPaths.java index 5416739304d..3f57dcf663d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReservedRawPaths.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReservedRawPaths.java @@ -37,6 +37,7 @@ import org.apache.hadoop.hdfs.client.HdfsAdmin; import org.apache.hadoop.hdfs.server.namenode.EncryptionZoneManager; import org.apache.hadoop.hdfs.server.namenode.FSDirectory; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.INodesInPath; import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.security.UserGroupInformation; @@ -112,11 +113,11 @@ public void testINodesInPath() throws IOException { FSDirectory fsd = cluster.getNamesystem().getFSDirectory(); final String path = "/path"; - INodesInPath iip = fsd.resolvePath(null, path); + INodesInPath iip = fsd.resolvePath(null, path, DirOp.READ); assertFalse(iip.isRaw()); assertEquals(path, iip.getPath()); - iip = fsd.resolvePath(null, "/.reserved/raw" + path); + iip = fsd.resolvePath(null, "/.reserved/raw" + path, DirOp.READ); assertTrue(iip.isRaw()); assertEquals(path, iip.getPath()); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSAclBaseTest.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSAclBaseTest.java index 52e638ecbf5..836fc0cc39d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSAclBaseTest.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSAclBaseTest.java @@ -43,6 +43,7 @@ import org.apache.hadoop.hdfs.protocol.AclException; import org.apache.hadoop.hdfs.protocol.FsPermissionExtension; import org.apache.hadoop.hdfs.protocol.HdfsConstants.SafeModeAction; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.io.IOUtils; import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.security.UserGroupInformation; @@ -861,8 +862,8 @@ public void testSetPermissionCannotSetAclBit() throws IOException { fs.setPermission(path, new FsPermissionExtension(FsPermission. createImmutable((short)0755), true, true)); - INode inode = cluster.getNamesystem().getFSDirectory().getINode( - path.toUri().getPath(), false); + INode inode = cluster.getNamesystem().getFSDirectory() + .getINode(path.toUri().getPath(), DirOp.READ_LINK); assertNotNull(inode); FsPermission perm = inode.getFsPermission(); assertNotNull(perm); @@ -1673,7 +1674,7 @@ public static void assertAclFeature(final MiniDFSCluster miniCluster, public static AclFeature getAclFeature(Path pathToCheck, MiniDFSCluster cluster) throws IOException { INode inode = cluster.getNamesystem().getFSDirectory() - .getINode(pathToCheck.toUri().getPath(), false); + .getINode(pathToCheck.toUri().getPath(), DirOp.READ_LINK); assertNotNull(inode); AclFeature aclFeature = inode.getAclFeature(); return aclFeature; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java index 5028b9dbea2..8f3616081d3 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java @@ -34,6 +34,7 @@ import org.apache.hadoop.hdfs.server.blockmanagement.BlockManagerTestUtil; import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor; import org.apache.hadoop.hdfs.server.common.Storage.StorageDirectory; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.MkdirOp; import org.apache.hadoop.hdfs.server.namenode.FSNamesystem.SafeModeInfo; import org.apache.hadoop.hdfs.server.namenode.LeaseManager.Lease; @@ -139,9 +140,11 @@ public static Lease getLeaseForPath(NameNode nn, String path) { final FSNamesystem fsn = nn.getNamesystem(); INode inode; try { - inode = fsn.getFSDirectory().getINode(path, false); + inode = fsn.getFSDirectory().getINode(path, DirOp.READ); } catch (UnresolvedLinkException e) { throw new RuntimeException("Lease manager should not support symlinks"); + } catch (IOException ioe) { + return null; // unresolvable path, ex. parent dir is a file } return inode == null ? null : fsn.leaseManager.getLease((INodeFile) inode); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirectory.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirectory.java index 14f5d2b9cab..093595312a8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirectory.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirectory.java @@ -36,6 +36,8 @@ import org.apache.hadoop.fs.XAttr; import org.apache.hadoop.fs.XAttrSetFlag; import org.apache.hadoop.hdfs.protocol.NSQuotaExceededException; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; +import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.hdfs.DFSConfigKeys; import org.apache.hadoop.hdfs.DFSTestUtil; import org.apache.hadoop.hdfs.DistributedFileSystem; @@ -396,16 +398,16 @@ public void testVerifyParentDir() throws Exception { hdfs.createNewFile(new Path("/dir1/file")); hdfs.createNewFile(new Path("/dir1/dir2/file")); - INodesInPath iip = fsdir.resolvePath(null, "/"); + INodesInPath iip = fsdir.resolvePath(null, "/", DirOp.READ); fsdir.verifyParentDir(iip); - iip = fsdir.resolvePath(null, "/dir1"); + iip = fsdir.resolvePath(null, "/dir1", DirOp.READ); fsdir.verifyParentDir(iip); - iip = fsdir.resolvePath(null, "/dir1/file"); + iip = fsdir.resolvePath(null, "/dir1/file", DirOp.READ); fsdir.verifyParentDir(iip); - iip = fsdir.resolvePath(null, "/dir-nonexist/file"); + iip = fsdir.resolvePath(null, "/dir-nonexist/file", DirOp.READ); try { fsdir.verifyParentDir(iip); fail("expected FNF"); @@ -413,13 +415,13 @@ public void testVerifyParentDir() throws Exception { // expected. } - iip = fsdir.resolvePath(null, "/dir1/dir2"); + iip = fsdir.resolvePath(null, "/dir1/dir2", DirOp.READ); fsdir.verifyParentDir(iip); - iip = fsdir.resolvePath(null, "/dir1/dir2/file"); + iip = fsdir.resolvePath(null, "/dir1/dir2/file", DirOp.READ); fsdir.verifyParentDir(iip); - iip = fsdir.resolvePath(null, "/dir1/dir-nonexist/file"); + iip = fsdir.resolvePath(null, "/dir1/dir-nonexist/file", DirOp.READ); try { fsdir.verifyParentDir(iip); fail("expected FNF"); @@ -427,12 +429,23 @@ public void testVerifyParentDir() throws Exception { // expected. } - iip = fsdir.resolvePath(null, "/dir1/file/fail"); try { - fsdir.verifyParentDir(iip); - fail("expected FNF"); - } catch (ParentNotDirectoryException pnd) { - // expected. + iip = fsdir.resolvePath(null, "/dir1/file/fail", DirOp.READ); + fail("expected ACE"); + } catch (AccessControlException ace) { + assertTrue(ace.getMessage().contains("is not a directory")); + } + try { + iip = fsdir.resolvePath(null, "/dir1/file/fail", DirOp.WRITE); + fail("expected ACE"); + } catch (AccessControlException ace) { + assertTrue(ace.getMessage().contains("is not a directory")); + } + try { + iip = fsdir.resolvePath(null, "/dir1/file/fail", DirOp.CREATE); + fail("expected PNDE"); + } catch (ParentNotDirectoryException pnde) { + assertTrue(pnde.getMessage().contains("is not a directory")); } } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java index 6a9e5440071..f9fe97c7caf 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java @@ -48,6 +48,7 @@ import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.fs.permission.PermissionStatus; import org.apache.hadoop.hdfs.protocol.HdfsConstants; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot; import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.security.UserGroupInformation; @@ -403,7 +404,7 @@ private void addAcl(INodeWithAdditionalFields inode, AclEntry... acl) private void assertPermissionGranted(UserGroupInformation user, String path, FsAction access) throws IOException { - INodesInPath iip = dir.getINodesInPath(path, true); + INodesInPath iip = dir.getINodesInPath(path, DirOp.READ); dir.getPermissionChecker(SUPERUSER, SUPERGROUP, user).checkPermission(iip, false, null, null, access, null, false); } @@ -411,7 +412,7 @@ private void assertPermissionGranted(UserGroupInformation user, String path, private void assertPermissionDenied(UserGroupInformation user, String path, FsAction access) throws IOException { try { - INodesInPath iip = dir.getINodesInPath(path, true); + INodesInPath iip = dir.getINodesInPath(path, DirOp.READ); dir.getPermissionChecker(SUPERUSER, SUPERGROUP, user).checkPermission(iip, false, null, null, access, null, false); fail("expected AccessControlException for user + " + user + ", path = " + diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java index bc81987a12f..c34b4de846d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFileTruncate.java @@ -58,6 +58,7 @@ import org.apache.hadoop.hdfs.server.common.HdfsServerConstants; import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.StartupOption; import org.apache.hadoop.hdfs.server.datanode.FsDatasetTestUtils; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.test.GenericTestUtils; import org.apache.hadoop.util.Time; @@ -1007,7 +1008,7 @@ public void testTruncateRecovery() throws IOException { byte[] contents = AppendTestUtil.initBuffer(BLOCK_SIZE); writeContents(contents, BLOCK_SIZE, srcPath); - INodesInPath iip = fsn.getFSDirectory().getINodesInPath4Write(src, true); + INodesInPath iip = fsn.getFSDirectory().getINodesInPath(src, DirOp.WRITE); INodeFile file = iip.getLastINode().asFile(); long initialGenStamp = file.getLastBlock().getGenerationStamp(); // Test that prepareFileForTruncate sets up in-place truncate. @@ -1038,7 +1039,7 @@ public void testTruncateRecovery() throws IOException { writeContents(contents, BLOCK_SIZE, srcPath); fs.allowSnapshot(parent); fs.createSnapshot(parent, "ss0"); - iip = fsn.getFSDirectory().getINodesInPath(src, true); + iip = fsn.getFSDirectory().getINodesInPath(src, DirOp.WRITE); file = iip.getLastINode().asFile(); file.recordModification(iip.getLatestSnapshotId(), true); assertThat(file.isBlockInLatestSnapshot(file.getLastBlock()), is(true)); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsck.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsck.java index efe39056405..e2593cd0501 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsck.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsck.java @@ -92,6 +92,7 @@ import org.apache.hadoop.hdfs.server.blockmanagement.BlockManager; import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor; import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeManager; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.NamenodeFsck.Result; import org.apache.hadoop.hdfs.server.protocol.NamenodeProtocols; import org.apache.hadoop.hdfs.tools.DFSck; @@ -875,7 +876,7 @@ public void testFsckError() throws Exception { // intentionally corrupt NN data structure INodeFile node = (INodeFile) cluster.getNamesystem().dir.getINode( - fileName, true); + fileName, DirOp.READ); final BlockInfo[] blocks = node.getBlocks(); assertEquals(blocks.length, 1); blocks[0].setNumBytes(-1L); // set the block length to be negative @@ -1126,8 +1127,8 @@ public void testFsckFileNotFound() throws Exception { when(fsName.getBlockManager()).thenReturn(blockManager); when(fsName.getFSDirectory()).thenReturn(fsd); when(fsd.getFSNamesystem()).thenReturn(fsName); - when(fsd.resolvePath(any(FSPermissionChecker.class), anyString())) - .thenReturn(iip); + when(fsd.resolvePath(any(FSPermissionChecker.class), + anyString(), any(DirOp.class))).thenReturn(iip); when(blockManager.getDatanodeManager()).thenReturn(dnManager); NamenodeFsck fsck = new NamenodeFsck(conf, namenode, nettop, pmap, out, diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestGetBlockLocations.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestGetBlockLocations.java index eec5c98b9e9..c66a7879331 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestGetBlockLocations.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestGetBlockLocations.java @@ -21,6 +21,7 @@ import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.fs.permission.PermissionStatus; import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.junit.Test; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; @@ -68,7 +69,7 @@ public void testGetBlockLocationsRacingWithDelete() throws IOException { @Override public Void answer(InvocationOnMock invocation) throws Throwable { - INodesInPath iip = fsd.getINodesInPath(FILE_PATH, true); + INodesInPath iip = fsd.getINodesInPath(FILE_PATH, DirOp.READ); FSDirDeleteOp.delete(fsd, iip, new INode.BlocksMapUpdateInfo(), new ArrayList(), new ArrayList(), now()); @@ -119,7 +120,7 @@ private static FSNamesystem setupFileSystem() throws IOException { final FSNamesystem fsn = new FSNamesystem(conf, image, true); final FSDirectory fsd = fsn.getFSDirectory(); - INodesInPath iip = fsd.getINodesInPath("/", true); + INodesInPath iip = fsd.getINodesInPath("/", DirOp.READ); PermissionStatus perm = new PermissionStatus( "hdfs", "supergroup", FsPermission.createImmutable((short) 0x1ff)); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java index 07f01d00ad6..d1d915e2ea4 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestSnapshotPathINodes.java @@ -200,6 +200,11 @@ public void testSnapshotPathINodes() throws Exception { // SnapshotRootIndex should be 3: {root, Testsnapshot, sub1, s1, file1} final Snapshot snapshot = getSnapshot(nodesInPath, "s1", 3); assertSnapshot(nodesInPath, true, snapshot, 3); + assertEquals(".snapshot/s1", + DFSUtil.bytes2String(nodesInPath.getPathComponent(3))); + assertTrue(nodesInPath.getINode(3) instanceof Snapshot.Root); + assertEquals("s1", nodesInPath.getINode(3).getLocalName()); + // Check the INode for file1 (snapshot file) INode snapshotFileNode = nodesInPath.getLastINode(); assertINodeFile(snapshotFileNode, file1); @@ -219,6 +224,9 @@ public void testSnapshotPathINodes() throws Exception { // The number of INodes returned should still be components.length // since we put a null in the inode array for ".snapshot" assertEquals(nodesInPath.length(), components.length); + assertEquals(".snapshot", + DFSUtil.bytes2String(nodesInPath.getLastLocalName())); + assertNull(nodesInPath.getLastINode()); // ensure parent inodes can strip the .snapshot assertEquals(sub1.toString(), nodesInPath.getParentINodesInPath().getPath()); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotTestHelper.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotTestHelper.java index 29d227294e5..2ea2a9853f2 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotTestHelper.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotTestHelper.java @@ -469,7 +469,13 @@ public int hashCode() { public static void dumpTree(String message, MiniDFSCluster cluster ) throws UnresolvedLinkException { System.out.println("XXX " + message); - cluster.getNameNode().getNamesystem().getFSDirectory().getINode("/" - ).dumpTreeRecursively(System.out); + try { + cluster.getNameNode().getNamesystem().getFSDirectory().getINode("/" + ).dumpTreeRecursively(System.out); + } catch (UnresolvedLinkException ule) { + throw ule; + } catch (IOException ioe) { + throw new RuntimeException(ioe); + } } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotReplication.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotReplication.java index d073228a700..20cb270e37c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotReplication.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotReplication.java @@ -30,6 +30,7 @@ import org.apache.hadoop.hdfs.MiniDFSCluster; import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo; import org.apache.hadoop.hdfs.server.namenode.FSDirectory; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; import org.apache.hadoop.hdfs.server.namenode.FSNamesystem; import org.apache.hadoop.hdfs.server.namenode.INode; import org.apache.hadoop.hdfs.server.namenode.INodeFile; @@ -146,7 +147,7 @@ private void checkSnapshotFileReplication(Path currentFile, } // Then check replication for every snapshot for (Path ss : snapshotRepMap.keySet()) { - final INodesInPath iip = fsdir.getINodesInPath(ss.toString(), true); + final INodesInPath iip = fsdir.getINodesInPath(ss.toString(), DirOp.READ); final INodeFile ssInode = iip.getLastINode().asFile(); // The replication number derived from the // INodeFileWithLink#getPreferredBlockReplication should diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermissionSymlinks.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermissionSymlinks.java index bc41edc1107..7bd29d21ddf 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermissionSymlinks.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermissionSymlinks.java @@ -27,7 +27,6 @@ import static org.junit.Assert.fail; import java.io.IOException; -import java.io.FileNotFoundException; import java.security.PrivilegedExceptionAction; import java.util.Arrays; @@ -424,8 +423,12 @@ public FileContext run() throws IOException { try { myfc.access(badPath, FsAction.READ); fail("The access call should have failed"); - } catch (FileNotFoundException e) { + } catch (AccessControlException ace) { // expected + String message = ace.getMessage(); + assertTrue(message, message.contains("is not a directory")); + assertTrue(message.contains(target.toString())); + assertFalse(message.contains(badPath.toString())); } } }