From 8543d84e30cdeff7f7b11e971318a79170669a8d Mon Sep 17 00:00:00 2001 From: Alan Bateman Date: Mon, 14 Sep 2009 17:47:26 +0100 Subject: [PATCH] 6876541: (file) Files.walkFileTree(...): no SecurityException if read access to the starting file is denied Reviewed-by: chegar --- .../classes/java/nio/file/FileTreeWalker.java | 28 ++-- .../share/classes/java/nio/file/Files.java | 2 +- .../java/nio/file/Files/WalkWithSecurity.java | 132 ++++++++++++++++++ jdk/test/java/nio/file/Files/denyAll.policy | 3 + jdk/test/java/nio/file/Files/grantAll.policy | 5 + .../java/nio/file/Files/grantTopOnly.policy | 4 + 6 files changed, 163 insertions(+), 11 deletions(-) create mode 100644 jdk/test/java/nio/file/Files/WalkWithSecurity.java create mode 100644 jdk/test/java/nio/file/Files/denyAll.policy create mode 100644 jdk/test/java/nio/file/Files/grantAll.policy create mode 100644 jdk/test/java/nio/file/Files/grantTopOnly.policy diff --git a/jdk/src/share/classes/java/nio/file/FileTreeWalker.java b/jdk/src/share/classes/java/nio/file/FileTreeWalker.java index 71cb86eb88a..1452bd66b2a 100644 --- a/jdk/src/share/classes/java/nio/file/FileTreeWalker.java +++ b/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 visitor; + private final int maxDepth; - FileTreeWalker(Set options, FileVisitor visitor) { + FileTreeWalker(Set options, + FileVisitor 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()); if (result == null) { throw new NullPointerException("Visitor returned 'null'"); @@ -89,12 +90,15 @@ class FileTreeWalker { List 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 diff --git a/jdk/src/share/classes/java/nio/file/Files.java b/jdk/src/share/classes/java/nio/file/Files.java index bf596409e5a..ca5bc5698e1 100644 --- a/jdk/src/share/classes/java/nio/file/Files.java +++ b/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); } /** diff --git a/jdk/test/java/nio/file/Files/WalkWithSecurity.java b/jdk/test/java/nio/file/Files/WalkWithSecurity.java new file mode 100644 index 00000000000..5b2ab564474 --- /dev/null +++ b/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 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 { + 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; + } + } +} diff --git a/jdk/test/java/nio/file/Files/denyAll.policy b/jdk/test/java/nio/file/Files/denyAll.policy new file mode 100644 index 00000000000..32500947791 --- /dev/null +++ b/jdk/test/java/nio/file/Files/denyAll.policy @@ -0,0 +1,3 @@ +// policy file that does not grant any permissions +grant { +}; diff --git a/jdk/test/java/nio/file/Files/grantAll.policy b/jdk/test/java/nio/file/Files/grantAll.policy new file mode 100644 index 00000000000..85bc0d0fb51 --- /dev/null +++ b/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"; +}; diff --git a/jdk/test/java/nio/file/Files/grantTopOnly.policy b/jdk/test/java/nio/file/Files/grantTopOnly.policy new file mode 100644 index 00000000000..fca1539416f --- /dev/null +++ b/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"; +};