--- old/src/share/classes/java/util/logging/FileHandler.java 2012-11-14 16:53:58.330576797 -0500 +++ new/src/share/classes/java/util/logging/FileHandler.java 2012-11-14 16:53:58.186579715 -0500 @@ -25,10 +25,19 @@ package java.util.logging; -import java.io.*; +import static java.nio.file.StandardOpenOption.CREATE_NEW; +import static java.nio.file.StandardOpenOption.WRITE; + +import java.io.BufferedOutputStream; +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.OutputStream; import java.nio.channels.FileChannel; -import java.nio.channels.FileLock; -import java.security.*; +import java.nio.file.FileAlreadyExistsException; +import java.nio.file.Paths; +import java.security.AccessController; +import java.security.PrivilegedAction; /** * Simple file logging Handler. @@ -137,14 +146,16 @@ private int count; private String pattern; private String lockFileName; - private FileOutputStream lockStream; + private FileChannel lockFileChannel; private File files[]; private static final int MAX_LOCKS = 100; private static java.util.HashMap locks = new java.util.HashMap<>(); - // A metered stream is a subclass of OutputStream that - // (a) forwards all its output to a target stream - // (b) keeps track of how many bytes have been written + /** + * A metered stream is a subclass of OutputStream that + * (a) forwards all its output to a target stream + * (b) keeps track of how many bytes have been written + */ private class MeteredStream extends OutputStream { OutputStream out; int written; @@ -189,9 +200,10 @@ setOutputStream(meter); } - // Private method to configure a FileHandler from LogManager - // properties and/or default values as specified in the class - // javadoc. + /** + * Configure a FileHandler from LogManager properties and/or default values + * as specified in the class javadoc. + */ private void configure() { LogManager manager = LogManager.getLogManager(); @@ -287,7 +299,8 @@ * the caller does not have LoggingPermission("control"). * @exception IllegalArgumentException if pattern is an empty string */ - public FileHandler(String pattern, boolean append) throws IOException, SecurityException { + public FileHandler(String pattern, boolean append) throws IOException, + SecurityException { if (pattern.length() < 1 ) { throw new IllegalArgumentException(); } @@ -376,8 +389,10 @@ openFiles(); } - // Private method to open the set of output files, based on the - // configured instance variables. + /** + * Open the set of output files, based on the configured + * instance variables. + */ private void openFiles() throws IOException { LogManager manager = LogManager.getLogManager(); manager.checkPermission(); @@ -413,18 +428,18 @@ // object. Try again. continue; } - FileChannel fc; + try { - lockStream = new FileOutputStream(lockFileName); - fc = lockStream.getChannel(); - } catch (IOException ix) { - // We got an IOException while trying to open the file. - // Try the next file. + lockFileChannel = FileChannel.open(Paths.get(lockFileName), + CREATE_NEW, WRITE); + } catch (FileAlreadyExistsException ix) { + // try the next lock file name in the sequence continue; } + boolean available; try { - available = fc.tryLock() != null; + available = lockFileChannel.tryLock() != null; // We got the lock OK. } catch (IOException ix) { // We got an IOException while trying to get the lock. @@ -440,7 +455,7 @@ } // We failed to get the lock. Try next file. - fc.close(); + lockFileChannel.close(); } } @@ -472,8 +487,17 @@ setErrorManager(new ErrorManager()); } - // Generate a filename from a pattern. - private File generate(String pattern, int generation, int unique) throws IOException { + /** + * Generate a file based on a user-supplied pattern, generation number, + * and an integer uniqueness suffix + * @param pattern the pattern for naming the output file + * @param generation the generation number to distinguish rotated logs + * @param unique a unique number to resolve conflicts + * @return the generated File + * @throws IOException + */ + private File generate(String pattern, int generation, int unique) + throws IOException { File file = null; String word = ""; int ix = 0; @@ -548,7 +572,9 @@ return file; } - // Rotate the set of output files + /** + * Rotate the set of output files + */ private synchronized void rotate() { Level oldLevel = getLevel(); setLevel(Level.OFF); @@ -615,9 +641,8 @@ return; } try { - // Closing the lock file's FileOutputStream will close - // the underlying channel and free any locks. - lockStream.close(); + // Close the lock file channel (which also will free any locks) + lockFileChannel.close(); } catch (Exception ex) { // Problems closing the stream. Punt. } @@ -626,7 +651,7 @@ } new File(lockFileName).delete(); lockFileName = null; - lockStream = null; + lockFileChannel = null; } private static class InitializationErrorManager extends ErrorManager { @@ -636,6 +661,8 @@ } } - // Private native method to check if we are in a set UID program. + /** + * check if we are in a set UID program. + */ private static native boolean isSetUID(); } --- /dev/null 2012-11-14 11:52:22.256709639 -0500 +++ new/test/java/util/logging/CheckLockLocationTest.java 2012-11-14 16:53:58.646570393 -0500 @@ -0,0 +1,210 @@ +/* + * Copyright (c) 2012, 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 6244047 + * @author Jim Gish + * @summary throw more precise IOException when pattern specifies invalid directory + * + * @run main/othervm CheckLockLocationTest + */ +import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.nio.file.AccessDeniedException; +import java.nio.file.FileSystemException; +import java.nio.file.NoSuchFileException; +import java.util.logging.FileHandler; +public class CheckLockLocationTest { + + private static final String NON_WRITABLE_DIR = "non-writable-dir"; + private static final String NOT_A_DIR = "not-a-dir"; + private static final String WRITABLE_DIR = "writable-dir"; + private static final String NON_EXISTENT_DIR = "non-existent-dir"; + + public static void main(String... args) throws IOException { + // we'll base all file creation attempts on the system temp directory, + // %t and also try specifying non-existent directories and plain files + // that should be directories, and non-writable directories, + // to exercise all code paths of checking the lock location + File writableDir = setup(); + // we now have three files/directories to work with: + // writableDir + // notAdir + // nonWritableDir + // nonExistentDir (which doesn't exist) + runTests(writableDir); + } + + /** + * @param writableDir in which log and lock file are created + * @throws SecurityException + * @throws RuntimeException + * @throws IOException + */ + private static void runTests(File writableDir) throws SecurityException, + RuntimeException, IOException { + // Test 1: make sure we can create FileHandler in writable directory + try { + new FileHandler("%t/" + WRITABLE_DIR + "/log.log"); + } catch (IOException ex) { + throw new RuntimeException("Test failed: should have been able" + + " to create FileHandler for " + "%t/" + WRITABLE_DIR + + "/log.log in writable directory.", ex); + } finally { + // the above test leaves files in the directory. Get rid of the + // files created and the directory + delete(writableDir); + } + + // Test 2: creating FileHandler in non-writable directory should fail + try { + new FileHandler("%t/" + NON_WRITABLE_DIR + "/log.log"); + throw new RuntimeException("Test failed: should not have been able" + + " to create FileHandler for " + "%t/" + NON_WRITABLE_DIR + + "/log.log in non-writable directory."); + } catch (IOException ex) { + // check for the right exception + if (!(ex instanceof AccessDeniedException)) { + throw new RuntimeException("Test failed: Expected exception was not an AccessDeniedException", ex); + } + } + + // Test 3: creating FileHandler in non-directory should fail + try { + new FileHandler("%t/" + NOT_A_DIR + "/log.log"); + throw new RuntimeException("Test failed: should not have been able" + + " to create FileHandler for " + "%t/" + NOT_A_DIR + + "/log.log in non-directory."); + } catch (IOException ex) { + // check for the right exception + if (!(ex instanceof FileSystemException && ex.getMessage().contains("Not a directory"))) { + throw new RuntimeException("Test failed: Expected exception was not a FileSystemException", ex); + } + } + + // Test 4: make sure we can't create a FileHandler in a non-existent dir + try { + new FileHandler("%t/" + NON_EXISTENT_DIR + "/log.log"); + throw new RuntimeException("Test failed: should not have been able" + + " to create FileHandler for " + "%t/" + NON_EXISTENT_DIR + + "/log.log in a non-existent directory."); + } catch (IOException ex) { + // check for the right exception + if (!(ex instanceof NoSuchFileException)) { + throw new RuntimeException("Test failed: Expected exception was not a NoSuchFileException", ex); + } + } + } + + /** + * Setup all the files and directories needed for the tests + * + * @return writable directory created that needs to be deleted when done + * @throws RuntimeException + */ + private static File setup() throws RuntimeException { + // First do some setup in the temporary directory (using same logic as + // FileHandler for %t pattern) + String tmpDir = System.getProperty("java.io.tmpdir"); // i.e. %t + if (tmpDir == null) { + tmpDir = System.getProperty("user.home"); + } + File tmpOrHomeDir = new File(tmpDir); + // Create a writable directory here (%t/writable-dir) + File writableDir = new File(tmpOrHomeDir, WRITABLE_DIR); + if (!createFile(writableDir, true)) { + throw new RuntimeException("Test setup failed: unable to create" + + " writable working directory " + + writableDir.getAbsolutePath() ); + } + // writableDirectory and its contents will be deleted after the test + // that uses it + + // Create a plain file which we will attempt to use as a directory + // (%t/not-a-dir) + File notAdir = new File(tmpOrHomeDir, NOT_A_DIR); + if (!createFile(notAdir, false)) { + throw new RuntimeException("Test setup failed: unable to a plain" + + " working file " + notAdir.getAbsolutePath() ); + } + notAdir.deleteOnExit(); + + // Create a non-writable directory (%t/non-writable-dir) + File nonWritableDir = new File(tmpOrHomeDir, NON_WRITABLE_DIR); + if (!createFile(nonWritableDir, true)) { + throw new RuntimeException("Test setup failed: unable to create" + + " a non-" + + "writable working directory " + + nonWritableDir.getAbsolutePath() ); + } + nonWritableDir.deleteOnExit(); + + // make it non-writable + if (!nonWritableDir.setWritable(false)) { + throw new RuntimeException("Test setup failed: unable to make" + + " working directory " + nonWritableDir.getAbsolutePath() + + " non-writable."); + } + + // make sure non-existent directory really doesn't exist + File nonExistentDir = new File(tmpOrHomeDir, NON_EXISTENT_DIR); + if (nonExistentDir.exists()) { + nonExistentDir.delete(); + } + return writableDir; + } + + /** + * @param newFile + * @return true if file already exists or creation succeeded + */ + private static boolean createFile(File newFile, boolean makeDirectory) { + if (newFile.exists()) { + return true; + } + if (makeDirectory) { + return newFile.mkdir(); + } else { + try { + return newFile.createNewFile(); + } catch (IOException ioex) { + ioex.printStackTrace(); + return false; + } + } + } + + /* + * Recursively delete all files starting at specified file + */ + private static void delete(File f) throws IOException { + if (f != null && f.isDirectory()) { + for (File c : f.listFiles()) + delete(c); + } + if (!f.delete()) + throw new FileNotFoundException("Failed to delete file: " + f); + } +}