dcache,pnfs: skip symlink resolution when checking restrictions on di…

…rectory children during listing

Motivation:

Resolution of symlinks on restriction source and target paths
was introduced at https://rb.dcache.org/r/13970/
`commons,dcache: alternative symlinks resolution on restrictions`
master@3657a1f4e6681aebec236ab5086dbaece1864643

Since then, we have noticed a somewhat appreciable slowdown
on directory listing.  The reason is that the listing must
check permissions on READ_METADATA on all children, since
WebDAV specifies that paths for which the user does not
have this permission should not be visible.

Modification:

The proposed solution is to check the restrictions but
without resolving the symlinks.  We partially sacrifice
correctness in WebDAV when symlinks are involved in favor of
speed.

Result:

A modest speed-up of directory listing.

Target: master
Request: 9.1
Request: 9.0
Request: 8.2
Patch: https://rb.dcache.org/r/14006/
Requires-notes: yes
Acked-by: Dmitry

modules/common/src/main/java/org/dcache/auth/attributes/DenyActivityRestriction.java

@@ -60,7 +60,7 @@ public boolean isRestricted(Activity activity, FsPath path) {
     }
 
     @Override
-    public boolean isRestricted(Activity activity, FsPath directory, String name) {
+    public boolean isRestricted(Activity activity, FsPath directory, String name, boolean skipSymlinkResolution) {
         return denied.contains(activity);
     }
 

modules/common/src/main/java/org/dcache/auth/attributes/MultiTargetedRestriction.java

@@ -138,28 +138,14 @@ public boolean hasUnrestrictedChild(Activity activity, FsPath parent) {
 
     @Override
     public boolean isRestricted(Activity activity, FsPath path) {
-        for (Authorisation authorisation : authorisations) {
-            Function<FsPath, FsPath> resolver = getPathResolver();
-            FsPath allowedPath = resolver.apply(authorisation.getPath());
-            EnumSet<Activity> allowedActivity = authorisation.getActivity();
-            path = resolver.apply(path);
-            if (allowedActivity.contains(activity) && path.hasPrefix(allowedPath)) {
-                return false;
-            }
-
-            // As a special case, certain activities are always allowed for
-            // parents of an AllowedPath.
-            if (ALLOWED_PARENT_ACTIVITIES.contains(activity) && allowedPath.hasPrefix(path)) {
-                return false;
-            }
-        }
-
-        return true;
+        return isRestricted(activity, path, getPathResolver());
     }
 
     @Override
-    public boolean isRestricted(Activity activity, FsPath directory, String child) {
-        return isRestricted(activity, directory.child(child));
+    public boolean isRestricted(Activity activity, FsPath directory, String child,
+          boolean skipSymlinkResolution) {
+        return isRestricted(activity, directory.child(child),
+              skipSymlinkResolution ? getIdentityResolver() : getPathResolver());
     }
 
     @Override
@@ -227,4 +213,23 @@ public String toString() {
               .map(Object::toString)
               .collect(Collectors.joining(", ", "MultiTargetedRestriction[", "]"));
     }
+
+    private boolean isRestricted(Activity activity, FsPath path, Function<FsPath, FsPath> resolver) {
+        for (Authorisation authorisation : authorisations) {
+            FsPath allowedPath = resolver.apply(authorisation.getPath());
+            EnumSet<Activity> allowedActivity = authorisation.getActivity();
+            path = resolver.apply(path);
+            if (allowedActivity.contains(activity) && path.hasPrefix(allowedPath)) {
+                return false;
+            }
+
+            // As a special case, certain activities are always allowed for
+            // parents of an AllowedPath.
+            if (ALLOWED_PARENT_ACTIVITIES.contains(activity) && allowedPath.hasPrefix(path)) {
+                return false;
+            }
+        }
+
+        return true;
+    }
 }

modules/common/src/main/java/org/dcache/auth/attributes/PrefixRestriction.java

@@ -46,24 +46,14 @@ public ImmutableSet<FsPath> getPrefixes() {
 
     @Override
     public boolean isRestricted(Activity activity, FsPath path) {
-        for (FsPath prefix : prefixes) {
-            Function<FsPath, FsPath> resolver = getPathResolver();
-            prefix = resolver.apply(prefix);
-            path = resolver.apply(path);
-            if (path.hasPrefix(prefix)) {
-                return false;
-            }
-            if (prefix.hasPrefix(path) && (activity == Activity.READ_METADATA
-                  || activity == Activity.LIST)) {
-                return false;
-            }
-        }
-        return true;
+        return isRestricted(activity, path, getPathResolver());
     }
 
     @Override
-    public boolean isRestricted(Activity activity, FsPath directory, String child) {
-        return isRestricted(activity, directory.child(child));
+    public boolean isRestricted(Activity activity, FsPath directory, String child,
+          boolean skipSymlinkResolution) {
+        return isRestricted(activity, directory.child(child),
+              skipSymlinkResolution ? getIdentityResolver() : getPathResolver());
     }
 
     @Override
@@ -154,4 +144,19 @@ public String toString() {
         }
         return sb.append(']').toString();
     }
+
+    private boolean isRestricted(Activity activity, FsPath path, Function<FsPath, FsPath> resolver) {
+        for (FsPath prefix : prefixes) {
+            prefix = resolver.apply(prefix);
+            path = resolver.apply(path);
+            if (path.hasPrefix(prefix)) {
+                return false;
+            }
+            if (prefix.hasPrefix(path) && (activity == Activity.READ_METADATA
+                  || activity == Activity.LIST)) {
+                return false;
+            }
+        }
+        return true;
+    }
 }

modules/common/src/main/java/org/dcache/auth/attributes/Restriction.java

@@ -116,7 +116,22 @@ public interface Restriction extends LoginAttribute, Serializable {
      * @param child     The name of the target object within directory.
      * @return true if the user is not allowed this activity.
      */
-    boolean isRestricted(Activity activity, FsPath directory, String child);
+    default boolean isRestricted(Activity activity, FsPath directory, String child) {
+        return isRestricted(activity, directory, child, false);
+    }
+
+    /**
+     * An optimised version of isRestricted.  A restriction must respond as if {@literal
+     * isRestricted(activity, new FsPath(directory).add(child));} were called, but the method may be
+     * able to avoid creating a new FsPath object.
+     *
+     * @param activity  What the user is attempting.
+     * @param directory The directory containing the target
+     * @param child     The name of the target object within directory.
+     * @param skipSymlinkResolution If true, do not resolve symlinks.
+     * @return true if the user is not allowed this activity.
+     */
+    boolean isRestricted(Activity activity, FsPath directory, String child, boolean skipSymlinkResolution);
 
     /**
      * Return true iff there is a child of the supplied path whether the activity is not
@@ -157,6 +172,10 @@ public interface Restriction extends LoginAttribute, Serializable {
      * @return returns NOP resolver.  Should be overridden by implementations.
      */
     default Function<FsPath, FsPath> getPathResolver() {
+        return getIdentityResolver();
+    }
+
+    default Function<FsPath, FsPath> getIdentityResolver() {
         return Function.identity();
     }
 

modules/common/src/main/java/org/dcache/auth/attributes/Restrictions.java

@@ -165,9 +165,9 @@ public boolean isRestricted(Activity activity, FsPath path) {
         }
 
         @Override
-        public boolean isRestricted(Activity activity, FsPath directory, String name) {
+        public boolean isRestricted(Activity activity, FsPath directory, String name, boolean skipSymlink) {
             for (Restriction r : restrictions) {
-                r.setPathResolver(getPathResolver());
+                r.setPathResolver(skipSymlink? getIdentityResolver() : getPathResolver());
                 if (r.isRestricted(activity, directory, name)) {
                     return true;
                 }

modules/dcache/src/main/java/diskCacheV111/namespace/PnfsManagerV3.java

@@ -2223,7 +2223,7 @@ private void sendPartialReply() {
         @Override
         public void addEntry(String name, FileAttributes attrs) {
             if (Subjects.isRoot(_subject)
-                  || !_restriction.isRestricted(READ_METADATA, _directory, name)) {
+                  || !_restriction.isRestricted(READ_METADATA, _directory, name, true)) {
                 long now = System.currentTimeMillis();
                 _msg.addEntry(name, attrs);
                 if (_msg.getEntries().size() >= _directoryListLimit ||