6244047: impossible to specify directories to logging FileHandler unless they exist
Reviewed-by: alanb
--- a/jdk/src/share/classes/java/util/logging/FileHandler.java Wed Nov 14 16:41:12 2012 -0800
+++ b/jdk/src/share/classes/java/util/logging/FileHandler.java Thu Nov 15 13:46:45 2012 +0000
@@ -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 <tt>Handler</tt>.
@@ -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<String, String> 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 <tt>LoggingPermission("control")</tt>.
* @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 Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/util/logging/CheckLockLocationTest.java Thu Nov 15 13:46:45 2012 +0000
@@ -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);
+ }
+}