6876541: (file) Files.walkFileTree(...): no SecurityException if read access to the starting file is denied

Reviewed-by: chegar

jdk/src/share/classes/java/nio/file/FileTreeWalker.java

@@ -41,8 +41,12 @@ class FileTreeWalker {
     private final boolean detectCycles;
     private final LinkOption[] linkOptions;
     private final FileVisitor<? super Path> visitor;
+    private final int maxDepth;
 
-    FileTreeWalker(Set<FileVisitOption> options, FileVisitor<? super Path> visitor) {
+    FileTreeWalker(Set<FileVisitOption> options,
+                   FileVisitor<? super Path> visitor,
+                   int maxDepth)
+    {
         boolean fl = false;
         boolean dc = false;
         for (FileVisitOption option: options) {
@@ -58,18 +62,15 @@ class FileTreeWalker {
         this.linkOptions = (fl) ? new LinkOption[0] :
             new LinkOption[] { LinkOption.NOFOLLOW_LINKS };
         this.visitor = visitor;
+        this.maxDepth = maxDepth;
     }
 
     /**
      * Walk file tree starting at the given file
      */
-    void walk(Path start, int maxDepth) {
-        // don't use attributes of starting file as they may be stale
-        if (start instanceof BasicFileAttributesHolder) {
-            ((BasicFileAttributesHolder)start).invalidate();
-        }
+    void walk(Path start) {
         FileVisitResult result = walk(start,
-                                      maxDepth,
+                                      0,
                                       new ArrayList<AncestorDirectory>());
         if (result == null) {
             throw new NullPointerException("Visitor returned 'null'");
@@ -89,12 +90,15 @@ class FileTreeWalker {
                                  List<AncestorDirectory> ancestors)
     {
         // depth check
-        if (depth-- < 0)
+        if (depth > maxDepth)
             return FileVisitResult.CONTINUE;
 
         // if attributes are cached then use them if possible
         BasicFileAttributes attrs = null;
-        if (file instanceof BasicFileAttributesHolder) {
+        if ((depth > 0) &&
+            (file instanceof BasicFileAttributesHolder) &&
+            (System.getSecurityManager() == null))
+        {
             BasicFileAttributes cached = ((BasicFileAttributesHolder)file).get();
             if (!followLinks || !cached.isSymbolicLink())
                 attrs = cached;
@@ -120,6 +124,10 @@ class FileTreeWalker {
                     }
                 }
             } catch (SecurityException x) {
+                // If access to starting file is denied then SecurityException
+                // is thrown, otherwise the file is ignored.
+                if (depth == 0)
+                    throw x;
                 return FileVisitResult.CONTINUE;
             }
         }
@@ -196,7 +204,7 @@ class FileTreeWalker {
                 try {
                     for (Path entry: stream) {
                         inAction = true;
-                        result = walk(entry, depth, ancestors);
+                        result = walk(entry, depth+1, ancestors);
                         inAction = false;
 
                         // returning null will cause NPE to be thrown

jdk/src/share/classes/java/nio/file/Files.java

@@ -223,7 +223,7 @@ public final class Files {
     {
         if (maxDepth < 0)
             throw new IllegalArgumentException("'maxDepth' is negative");
-        new FileTreeWalker(options, visitor).walk(start, maxDepth);
+        new FileTreeWalker(options, visitor, maxDepth).walk(start);
     }
 
     /**

jdk/test/java/nio/file/Files/WalkWithSecurity.java

@@ -0,0 +1,132 @@
+/*
+ * Copyright 2009 Sun Microsystems, Inc.  All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/* @test
+ * @bug 6876541
+ * @summary Test Files.walkFileTree in the presence of a security manager
+ * @build WalkWithSecurity
+ * @run main/othervm WalkWithSecurity grantAll.policy pass
+ * @run main/othervm WalkWithSecurity denyAll.policy fail
+ * @run main/othervm WalkWithSecurity grantTopOnly.policy top_only
+ */
+
+import java.nio.file.*;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.io.IOException;
+
+public class WalkWithSecurity {
+
+    public static void main(String[] args) throws IOException {
+        String policyFile = args[0];
+        ExpectedResult expectedResult = ExpectedResult.valueOf(args[1].toUpperCase());
+
+        String here = System.getProperty("user.dir");
+        String testSrc = System.getProperty("test.src");
+        if (testSrc == null)
+            throw new RuntimeException("This test must be run by jtreg");
+        Path dir = Paths.get(testSrc);
+
+        // Sanity check the environment
+        if (Paths.get(here).isSameFile(dir))
+            throw new RuntimeException("Working directory cannot be " + dir);
+        DirectoryStream<Path> stream = dir.newDirectoryStream();
+        try {
+            if (!stream.iterator().hasNext())
+                throw new RuntimeException(testSrc + " is empty");
+        } finally {
+            stream.close();
+        }
+
+        // Install security manager with the given policy file
+        System.setProperty("java.security.policy",
+            dir.resolve(policyFile).toString());
+        System.setSecurityManager(new SecurityManager());
+
+        // Walk the source tree
+        CountingVisitor visitor = new CountingVisitor();
+        SecurityException exception = null;
+        try {
+            Files.walkFileTree(dir, visitor);
+        } catch (SecurityException se) {
+            exception = se;
+        }
+
+        // Check result
+        switch (expectedResult) {
+            case PASS:
+                if (exception != null) {
+                    exception.printStackTrace();
+                    throw new RuntimeException("SecurityException not expected");
+                }
+                if (visitor.count() == 0)
+                    throw new RuntimeException("No files visited");
+                break;
+            case FAIL:
+                if (exception == null)
+                    throw new RuntimeException("SecurityException expected");
+                if (visitor.count() > 0)
+                    throw new RuntimeException("Files were visited");
+                break;
+            case TOP_ONLY:
+                if (exception != null) {
+                    exception.printStackTrace();
+                    throw new RuntimeException("SecurityException not expected");
+                }
+                if (visitor.count() == 0)
+                    throw new RuntimeException("Starting file not visited");
+                if (visitor.count() > 1)
+                    throw new RuntimeException("More than starting file visited");
+                break;
+            default:
+                throw new RuntimeException("Should not get here");
+        }
+    }
+
+    static enum ExpectedResult {
+        PASS,
+        FAIL,
+        TOP_ONLY;
+    }
+
+    static class CountingVisitor extends SimpleFileVisitor<Path> {
+        private int count;
+
+        int count() {
+            return count;
+        }
+
+        @Override
+        public FileVisitResult preVisitDirectory(Path dir) {
+            System.out.println(dir);
+            count++;
+            return FileVisitResult.CONTINUE;
+        }
+
+        @Override
+        public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
+            System.out.println(file);
+            count++;
+            return FileVisitResult.CONTINUE;
+        }
+    }
+}

jdk/test/java/nio/file/Files/denyAll.policy

@@ -0,0 +1,3 @@
+// policy file that does not grant any permissions
+grant {
+};

jdk/test/java/nio/file/Files/grantAll.policy

@@ -0,0 +1,5 @@
+// policy file that grants read access to source directory and all descendants
+grant {
+    permission java.io.FilePermission "${test.src}", "read";
+    permission java.io.FilePermission "${test.src}${file.separator}-", "read";
+};

jdk/test/java/nio/file/Files/grantTopOnly.policy

@@ -0,0 +1,4 @@
+// policy file that grants read access to source directory
+grant {
+    permission java.io.FilePermission "${test.src}", "read";
+};