# HG changeset patch # User dxu # Date 1373575225 25200 # Node ID 7d65f90d73483bee83dc1b0c6525fececf79c85c # Parent c094f5a236ba22ecadb55092c86e3d14ae1af277 8017212: File.createTempFile requires unnecessary "read" permission Summary: Directly call FileSystem method to check a file existence. Also reviewed by tom.hawtin@oracle.com Reviewed-by: alanb diff -r c094f5a236ba -r 7d65f90d7348 jdk/src/share/classes/java/io/File.java --- a/jdk/src/share/classes/java/io/File.java Thu Jul 11 11:14:06 2013 -0700 +++ b/jdk/src/share/classes/java/io/File.java Thu Jul 11 13:40:25 2013 -0700 @@ -1910,7 +1910,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()) throw new IOException("Unable to create temporary file"); return f; } @@ -1996,19 +1996,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; } diff -r c094f5a236ba -r 7d65f90d7348 jdk/test/java/io/File/CheckPermission.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/jdk/test/java/io/File/CheckPermission.java Thu Jul 11 13:40:25 2013 -0700 @@ -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 permissionsChecked = new ArrayList<>(); + private Set propertiesChecked = new HashSet<>(); + + private Map> fileOperationChecked = + new EnumMap<>(FileOperation.class); + + List permissionsChecked() { + return permissionsChecked; + } + + Set propertiesChecked() { + return propertiesChecked; + } + + List fileOperationChecked(FileOperation op) { + return fileOperationChecked.get(op); + } + + void addFileOperation(FileOperation op, String file) { + List opList = fileOperationChecked.get(op); + if (opList == null) { + opList = new ArrayList<>(); + fileOperationChecked.put(op, opList); + } + opList.add(file); + } + } + + static ThreadLocal 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 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 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 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 ops) { + for (FileOperation op : ops) + assertChecked(file, myChecks.get().fileOperationChecked(op)); + } + + static void assertNotCheckOperation(File file, Set ops) { + for (FileOperation op : ops) + assertNotChecked(file, myChecks.get().fileOperationChecked(op)); + } + + static void assertOnlyCheckOperation(File file, + EnumSet 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)); + } +} diff -r c094f5a236ba -r 7d65f90d7348 jdk/test/java/io/File/NulFile.java --- a/jdk/test/java/io/File/NulFile.java Thu Jul 11 11:14:06 2013 -0700 +++ b/jdk/test/java/io/File/NulFile.java Thu Jul 11 13:40:25 2013 -0700 @@ -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) { diff -r c094f5a236ba -r 7d65f90d7348 jdk/test/java/io/File/createTempFile/SpecialTempFile.java --- a/jdk/test/java/io/File/createTempFile/SpecialTempFile.java Thu Jul 11 11:14:06 2013 -0700 +++ b/jdk/test/java/io/File/createTempFile/SpecialTempFile.java Thu Jul 11 13:40:25 2013 -0700 @@ -23,9 +23,8 @@ /* * @test - * @bug 8013827 8011950 + * @bug 8013827 8011950 8017212 * @summary Check whether File.createTempFile can handle special parameters - * on Windows platforms * @author Dan Xu */ @@ -64,6 +63,17 @@ } 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); + + // Windows tests if (!System.getProperty("os.name").startsWith("Windows")) return;