changeset 8034:2df9079cfbd9

8017212: File.createTempFile requires unnecessary "read" permission Reviewed-by: alanb
author igerasim
date Thu, 28 Aug 2014 18:39:18 +0400
parents 85eb739ea5a6
children 353c0eb4aace
files src/share/classes/java/io/File.java test/java/io/File/CheckPermission.java test/java/io/File/NulFile.java test/java/io/File/createTempFile/SpecialTempFile.java
diffstat 4 files changed, 422 insertions(+), 22 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/classes/java/io/File.java	Mon Dec 17 12:18:46 2012 +0800
+++ b/src/share/classes/java/io/File.java	Thu Aug 28 18:39:18 2014 +0400
@@ -1891,7 +1891,7 @@
 
             String name = prefix + Long.toString(n) + suffix;
             File f = new File(dir, name);
-            if (!name.equals(f.getName())) {
+            if (!name.equals(f.getName()) || f.isInvalid()) {
                 if (System.getSecurityManager() != null)
                     throw new IOException("Unable to create temporary file");
                 else
@@ -1981,19 +1981,26 @@
 
         File tmpdir = (directory != null) ? directory
                                           : TempDirectory.location();
+        SecurityManager sm = System.getSecurityManager();
         File f;
-        try {
-            do {
-                f = TempDirectory.generateFile(prefix, suffix, tmpdir);
-            } while (f.exists());
-            if (!f.createNewFile())
-                throw new IOException("Unable to create temporary file");
-        } catch (SecurityException se) {
-            // don't reveal temporary directory location
-            if (directory == null)
-                throw new SecurityException("Unable to create temporary file");
-            throw se;
-        }
+        do {
+            f = TempDirectory.generateFile(prefix, suffix, tmpdir);
+
+            if (sm != null) {
+                try {
+                    sm.checkWrite(f.getPath());
+                } catch (SecurityException se) {
+                    // don't reveal temporary directory location
+                    if (directory == null)
+                        throw new SecurityException("Unable to create temporary file");
+                    throw se;
+                }
+            }
+        } while ((fs.getBooleanAttributes(f) & FileSystem.BA_EXISTS) != 0);
+
+        if (!fs.createFileExclusively(f.getPath()))
+            throw new IOException("Unable to create temporary file");
+
         return f;
     }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/java/io/File/CheckPermission.java	Thu Aug 28 18:39:18 2014 +0400
@@ -0,0 +1,379 @@
+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. 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 Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/* @test
+ * @bug 8017212
+ * @summary Examine methods in File.java that access the file system do the
+ *          right permission check when a security manager exists.
+ * @author Dan Xu
+ */
+
+import java.io.File;
+import java.io.FilenameFilter;
+import java.io.FileFilter;
+import java.io.IOException;
+import java.security.Permission;
+import java.util.ArrayList;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+public class CheckPermission {
+
+    private static final String CHECK_PERMISSION_TEST = "CheckPermissionTest";
+
+    public enum FileOperation {
+        READ, WRITE, DELETE, EXEC
+    }
+
+    static class Checks {
+        private List<Permission> permissionsChecked = new ArrayList<>();
+        private Set<String> propertiesChecked = new HashSet<>();
+
+        private Map<FileOperation, List<String>> fileOperationChecked =
+            new EnumMap<>(FileOperation.class);
+
+        List<Permission> permissionsChecked() {
+            return permissionsChecked;
+        }
+
+        Set<String> propertiesChecked() {
+            return propertiesChecked;
+        }
+
+        List<String> fileOperationChecked(FileOperation op) {
+            return fileOperationChecked.get(op);
+        }
+
+        void addFileOperation(FileOperation op, String file) {
+            List<String> opList = fileOperationChecked.get(op);
+            if (opList == null) {
+                opList = new ArrayList<>();
+                fileOperationChecked.put(op, opList);
+            }
+            opList.add(file);
+        }
+    }
+
+    static ThreadLocal<Checks> myChecks = new ThreadLocal<>();
+
+    static void prepare() {
+        myChecks.set(new Checks());
+    }
+
+    static class LoggingSecurityManager extends SecurityManager {
+        static void install() {
+            System.setSecurityManager(new LoggingSecurityManager());
+        }
+
+        private void checkFileOperation(FileOperation op, String file) {
+            Checks checks = myChecks.get();
+            if (checks != null)
+                checks.addFileOperation(op, file);
+        }
+
+        @Override
+        public void checkRead(String file) {
+            checkFileOperation(FileOperation.READ, file);
+        }
+
+        @Override
+        public void checkWrite(String file) {
+            checkFileOperation(FileOperation.WRITE, file);
+        }
+
+        @Override
+        public void checkDelete(String file) {
+            checkFileOperation(FileOperation.DELETE, file);
+        }
+
+        @Override
+        public void checkExec(String file) {
+            checkFileOperation(FileOperation.EXEC, file);
+        }
+
+        @Override
+        public void checkPermission(Permission perm) {
+            Checks checks = myChecks.get();
+            if (checks != null)
+                checks.permissionsChecked().add(perm);
+        }
+
+        @Override
+        public void checkPropertyAccess(String key) {
+            Checks checks = myChecks.get();
+            if (checks != null)
+                checks.propertiesChecked().add(key);
+        }
+    }
+
+    static void assertCheckPermission(Class<? extends Permission> type,
+            String name)
+    {
+        for (Permission perm : myChecks.get().permissionsChecked()) {
+            if (type.isInstance(perm) && perm.getName().equals(name))
+                return;
+        }
+        throw new RuntimeException(type.getName() + "(\"" + name
+            + "\") not checked");
+    }
+
+    static void assertCheckPropertyAccess(String key) {
+        if (!myChecks.get().propertiesChecked().contains(key))
+            throw new RuntimeException("Property " + key + " not checked");
+    }
+
+    static void assertChecked(File file, List<String> list) {
+        if (list != null && !list.isEmpty()) {
+            for (String path : list) {
+                if (path.equals(file.getPath()))
+                    return;
+            }
+        }
+        throw new RuntimeException("Access not checked");
+    }
+
+    static void assertNotChecked(File file, List<String> list) {
+        if (list != null && !list.isEmpty()) {
+            for (String path : list) {
+                if (path.equals(file.getPath()))
+                    throw new RuntimeException("Access checked");
+            }
+        }
+    }
+
+    static void assertCheckOperation(File file, Set<FileOperation> ops) {
+        for (FileOperation op : ops)
+            assertChecked(file, myChecks.get().fileOperationChecked(op));
+    }
+
+    static void assertNotCheckOperation(File file, Set<FileOperation> ops) {
+        for (FileOperation op : ops)
+            assertNotChecked(file, myChecks.get().fileOperationChecked(op));
+    }
+
+    static void assertOnlyCheckOperation(File file,
+            EnumSet<FileOperation> ops)
+    {
+        assertCheckOperation(file, ops);
+        assertNotCheckOperation(file, EnumSet.complementOf(ops));
+    }
+
+    static File testFile, another;
+
+    static void setup() {
+        testFile = new File(CHECK_PERMISSION_TEST + System.currentTimeMillis());
+        if (testFile.exists()) {
+            testFile.delete();
+        }
+
+        another = new File(CHECK_PERMISSION_TEST + "Another"
+                           + System.currentTimeMillis());
+        if (another.exists()) {
+            another.delete();
+        }
+
+        LoggingSecurityManager.install();
+    }
+
+    public static void main(String[] args) throws IOException {
+        setup();
+
+        prepare();
+        testFile.canRead();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+
+        prepare();
+        testFile.canWrite();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.WRITE));
+
+        prepare();
+        testFile.exists();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+
+        prepare();
+        testFile.isDirectory();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+
+        prepare();
+        testFile.isFile();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+
+        prepare();
+        testFile.isHidden();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+
+        prepare();
+        testFile.lastModified();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+
+        prepare();
+        testFile.length();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+
+        prepare();
+        testFile.createNewFile();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.WRITE));
+
+        prepare();
+        testFile.list();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+
+        prepare();
+        testFile.list(new FilenameFilter() {
+            @Override
+            public boolean accept(File dir, String name) {
+                return false;
+            }
+        });
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+
+        prepare();
+        testFile.listFiles();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+
+        prepare();
+        testFile.listFiles(new FilenameFilter() {
+            @Override
+            public boolean accept(File dir, String name) {
+                return false;
+            }
+        });
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+
+        prepare();
+        testFile.listFiles(new FileFilter() {
+            @Override
+            public boolean accept(File file) {
+                return false;
+            }
+        });
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+
+        prepare();
+        testFile.mkdir();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.WRITE));
+
+        if (testFile.exists()) {
+            prepare();
+            testFile.mkdirs();
+            assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+        }
+
+        if (!another.exists()) {
+            prepare();
+            another.mkdirs();
+            assertOnlyCheckOperation(another,
+                    EnumSet.of(FileOperation.READ, FileOperation.WRITE));
+        }
+
+        prepare();
+        another.delete();
+        assertOnlyCheckOperation(another, EnumSet.of(FileOperation.DELETE));
+
+        prepare();
+        boolean renRst = testFile.renameTo(another);
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.WRITE));
+        assertOnlyCheckOperation(another, EnumSet.of(FileOperation.WRITE));
+
+        if (renRst) {
+            if (testFile.exists())
+                throw new RuntimeException(testFile + " is already renamed to "
+                    + another);
+            testFile = another;
+        }
+
+        prepare();
+        testFile.setLastModified(0);
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.WRITE));
+
+        prepare();
+        testFile.setReadOnly();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.WRITE));
+
+        prepare();
+        testFile.setWritable(true, true);
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.WRITE));
+
+        prepare();
+        testFile.setWritable(true);
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.WRITE));
+
+        prepare();
+        testFile.setReadable(true, true);
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.WRITE));
+
+        prepare();
+        testFile.setReadable(true);
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.WRITE));
+
+        prepare();
+        testFile.setExecutable(true, true);
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.WRITE));
+
+        prepare();
+        testFile.setExecutable(true);
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.WRITE));
+
+        prepare();
+        testFile.canExecute();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.EXEC));
+
+        prepare();
+        testFile.getTotalSpace();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+        assertCheckPermission(RuntimePermission.class,
+                "getFileSystemAttributes");
+
+        prepare();
+        testFile.getFreeSpace();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+        assertCheckPermission(RuntimePermission.class,
+                "getFileSystemAttributes");
+
+        prepare();
+        testFile.getUsableSpace();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.READ));
+        assertCheckPermission(RuntimePermission.class,
+                "getFileSystemAttributes");
+
+        prepare();
+        File tmpFile = File.createTempFile(CHECK_PERMISSION_TEST, null);
+        assertOnlyCheckOperation(tmpFile, EnumSet.of(FileOperation.WRITE));
+        tmpFile.delete();
+        assertCheckOperation(tmpFile, EnumSet.of(FileOperation.DELETE));
+
+        prepare();
+        tmpFile = File.createTempFile(CHECK_PERMISSION_TEST, null, null);
+        assertOnlyCheckOperation(tmpFile, EnumSet.of(FileOperation.WRITE));
+        tmpFile.delete();
+        assertCheckOperation(tmpFile, EnumSet.of(FileOperation.DELETE));
+
+        prepare();
+        testFile.deleteOnExit();
+        assertOnlyCheckOperation(testFile, EnumSet.of(FileOperation.DELETE));
+    }
+}
--- a/test/java/io/File/NulFile.java	Mon Dec 17 12:18:46 2012 +0800
+++ b/test/java/io/File/NulFile.java	Thu Aug 28 18:39:18 2014 +0400
@@ -612,8 +612,13 @@
             try {
                 File.createTempFile(prefix, suffix, directory);
             } catch (IOException ex) {
-                if (ExceptionMsg.equals(ex.getMessage()))
+                String err = "Unable to create temporary file";
+                if (err.equals(ex.getMessage()))
                     exceptionThrown = true;
+                else {
+                    throw new RuntimeException("Get IOException with message, "
+                            + ex.getMessage() + ", expect message, "+ err);
+                }
             }
         }
         if (!exceptionThrown) {
--- a/test/java/io/File/createTempFile/SpecialTempFile.java	Mon Dec 17 12:18:46 2012 +0800
+++ b/test/java/io/File/createTempFile/SpecialTempFile.java	Thu Aug 28 18:39:18 2014 +0400
@@ -23,9 +23,8 @@
 
 /*
  * @test
- * @bug 8013827 8011950 8025128
+ * @bug 8013827 8011950 8017212 8025128
  * @summary Check whether File.createTempFile can handle special parameters
- *          on Windows platforms
  * @author Dan Xu
  */
 
@@ -35,7 +34,7 @@
 public class SpecialTempFile {
 
     private static void test(String name, String[] prefix, String[] suffix,
-                             boolean expectedException) throws IOException
+                             boolean exceptionExpected) throws IOException
     {
         if (prefix == null || suffix == null
             || prefix.length != suffix.length)
@@ -44,14 +43,13 @@
         }
 
         final String exceptionMsg = "Unable to create temporary file";
+        String[] dirs = { null, "." };
 
         for (int i = 0; i < prefix.length; i++) {
             boolean exceptionThrown = false;
             File f = null;
 
-            String[] dirs = { null, "." };
-
-            for (String dir : dirs ) {
+            for (String dir: dirs) {
                 System.out.println("In test " + name +
                                    ", creating temp file with prefix, " +
                                    prefix[i] + ", suffix, " + suffix[i] +
@@ -63,7 +61,7 @@
                     else
                         f = File.createTempFile(prefix[i], suffix[i], new File(dir));
                 } catch (IOException e) {
-                    if (expectedException) {
+                    if (exceptionExpected) {
                         if (e.getMessage().startsWith(exceptionMsg))
                             exceptionThrown = true;
                         else
@@ -74,13 +72,23 @@
                     }
                 }
 
-                if (expectedException && (!exceptionThrown || f != null))
+                if (exceptionExpected && (!exceptionThrown || f != null))
                     throw new RuntimeException("IOException is expected");
             }
         }
     }
 
     public static void main(String[] args) throws Exception {
+        // Common test
+        final String name = "SpecialTempFile";
+        File f = new File(System.getProperty("java.io.tmpdir"), name);
+        if (!f.exists()) {
+            f.createNewFile();
+        }
+        String[] nulPre = { name + "\u0000" };
+        String[] nulSuf = { ".test" };
+        test("NulName", nulPre, nulSuf, true);
+
         // Test JDK-8025128
         String[] goodPre = { "///..///", "/foo" };
         String[] goodSuf = { ".temp", ".tmp" };
@@ -91,6 +99,7 @@
         String[] slashSuf = { "///..///..", "///..///..", "///..///.." };
         test("SlashedName", slashPre, slashSuf, true);
 
+        // Windows tests
         if (!System.getProperty("os.name").startsWith("Windows"))
             return;