Reduce number of callbacks when checking access flags. (#89)

* Reduce number of callbacks when checking access flags. Signed-off-by: David Kocher <[email protected]> * Fix test. Signed-off-by: David Kocher <[email protected]>
dCache · May 21, 2020 · 15d38f2 · 15d38f2
1 parent 7d14d29
commit 15d38f2
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 13 deletions.
diff --git a/core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java b/core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java
@@ -85,9 +85,9 @@ protected VirtualFileSystem delegate() {
         return _inner;
     }
 
-    private boolean canAccess(Inode inode, int mode) {
+    private boolean canAccess(Inode inode, Stat stat, int mode) {
         try {
-            checkAccess(inode, mode, false);
+            checkAccess(inode, stat, mode, false);
             return true;
         } catch (IOException e) {
         }
@@ -102,38 +102,39 @@ public int access(Inode inode, int mode) throws IOException {
             throw new InvalException("invalid access mask");
         }
 
+        Stat stat = _inner.getattr(inode);
         if ((mode & ACCESS4_READ) != 0) {
-            if (canAccess(inode, ACE4_READ_DATA)) {
+            if (canAccess(inode, stat, ACE4_READ_DATA)) {
                 accessmask |= ACCESS4_READ;
             }
         }
 
         if ((mode & ACCESS4_LOOKUP) != 0) {
-            if (canAccess(inode, ACE4_EXECUTE)) {
+            if (canAccess(inode, stat, ACE4_EXECUTE)) {
                 accessmask |= ACCESS4_LOOKUP;
             }
         }
 
         if ((mode & ACCESS4_MODIFY) != 0) {
-            if (canAccess(inode, ACE4_WRITE_DATA)) {
+            if (canAccess(inode, stat, ACE4_WRITE_DATA)) {
                 accessmask |= ACCESS4_MODIFY;
             }
         }
 
         if ((mode & ACCESS4_EXECUTE) != 0) {
-            if (canAccess(inode, ACE4_EXECUTE)) {
+            if (canAccess(inode, stat, ACE4_EXECUTE)) {
                 accessmask |= ACCESS4_EXECUTE;
             }
         }
 
         if ((mode & ACCESS4_EXTEND) != 0) {
-            if (canAccess(inode, ACE4_APPEND_DATA)) {
+            if (canAccess(inode, stat, ACE4_APPEND_DATA)) {
                 accessmask |= ACCESS4_EXTEND;
             }
         }
 
         if ((mode & ACCESS4_DELETE) != 0) {
-            if (canAccess(inode, ACE4_DELETE_CHILD)) {
+            if (canAccess(inode, stat, ACE4_DELETE_CHILD)) {
                 accessmask |= ACCESS4_DELETE;
             }
         }
@@ -146,19 +147,19 @@ public int access(Inode inode, int mode) throws IOException {
          */
 
         if ((mode & ACCESS4_XAREAD) != 0) {
-            if (canAccess(inode, ACE4_READ_DATA)) {
+            if (canAccess(inode, stat, ACE4_READ_DATA)) {
                 accessmask |= ACCESS4_XAREAD;
             }
         }
 
         if ((mode & ACCESS4_XALIST) != 0) {
-            if (canAccess(inode, ACE4_READ_DATA)) {
+            if (canAccess(inode, stat, ACE4_READ_DATA)) {
                 accessmask |= ACCESS4_XALIST;
             }
         }
 
         if ((mode & ACCESS4_XAWRITE) != 0) {
-            if (canAccess(inode, ACE4_WRITE_DATA)) {
+            if (canAccess(inode, stat, ACE4_WRITE_DATA)) {
                 accessmask |= ACCESS4_XAWRITE;
             }
         }
@@ -393,6 +394,10 @@ private Subject checkAccess(Inode inode, int requestedMask) throws IOException {
     }
 
     private Subject checkAccess(Inode inode, int requestedMask, boolean shouldLog) throws IOException {
+        return checkAccess(inode, _inner.getattr(inode), requestedMask, shouldLog);
+    }
+
+    private Subject checkAccess(Inode inode, Stat stat, int requestedMask, boolean shouldLog) throws IOException {
 
         Subject effectiveSubject = _subject;
         Access aclMatched = Access.UNDEFINED;
@@ -453,7 +458,6 @@ private Subject checkAccess(Inode inode, int requestedMask, boolean shouldLog) t
          * always allows it.
          */
         if ((aclMatched == Access.UNDEFINED) && (requestedMask != ACE4_READ_ATTRIBUTES)) {
-            Stat stat = _inner.getattr(inode);
             int unixAccessmask = unixToAccessmask(effectiveSubject, stat);
             if ((unixAccessmask & requestedMask) != requestedMask) {
                 if (shouldLog) {

diff --git a/core/src/test/java/org/dcache/nfs/vfs/PseudoFsTest.java b/core/src/test/java/org/dcache/nfs/vfs/PseudoFsTest.java
@@ -24,6 +24,8 @@
 import java.nio.charset.StandardCharsets;
 import java.util.stream.Stream;
 import javax.security.auth.Subject;
+
+import com.google.common.primitives.Longs;
 import org.dcache.auth.Subjects;
 import org.dcache.nfs.ExportFile;
 import org.dcache.nfs.FsExport;
@@ -383,7 +385,7 @@ public void testAccessDenyOnHighjackedInode() throws IOException {
         Inode inode = new Inode(
                 new FileHandle.FileHandleBuilder()
                     .setExportIdx(1)
-                    .build(new byte[] {0x1})
+                    .build(Longs.toByteArray(1L))
             );
 
         given(mockedExportFile.getExport(1, localAddress.getAddress())).willReturn(null);