On Wed, 16 Nov 2022 14:02:01 GMT, Weibing Xiao <d...@openjdk.org> wrote:
>> print warning message for java.io.tmpdir when it is set through the command >> line and the value is not good for creating file folder. > > 1) Removed the link for CSR. > 2) Add a new method in SystemProps.java to check if the warning message is > required > 3) Print the warning message directly in System::initPhase3 Hello @weibxiao, I had a look at the linked JBS issue https://bugs.openjdk.org/browse/JDK-8290313. It appears to me that the original motivation of this change is to improve the situation where a call to `File.createTempFile(...)` can fail with the following excepiton: java.io.IOException: No such file or directory at java.io.UnixFileSystem.createFileExclusively(Native Method) at java.io.File.createTempFile(File.java:2001) at java.io.File.createTempFile(File.java:2047) if either the `java.io.tmpdir` doesn't exist or the user passed value for the `directory` parameter to `File.createTempDirectory(prefix, suffix, directory)` doesn't exist. I feel that the change being proposed here to print a warning if the user specified `java.io.tmpdir` won't improve this situation because: - This warning will get printed potentially (far) away and early in the logs as compared to where this exception might get logged. - The user may not even have access to the log file, and it just might be the case that the application code caught this exception and printed its stacktrace - This will continue to fail with the above exception (and without the proposed warning) if the user passed `directory` to the `File.createTempDirectory(prefix, suffix, directory)` is non-existent. For example (`/tmp/blah` directory doesn't exist in the example below): jshell> File.createTempFile("foo", null, new File("/tmp/blah")); | Exception java.io.IOException: No such file or directory | at UnixFileSystem.createFileExclusively0 (Native Method) | at UnixFileSystem.createFileExclusively (UnixFileSystem.java:356) | at File.createTempFile (File.java:2179) | at (#2:1) I think if we want to improve this error message then maybe attempt the solution proposed by Alan in the JBS issue: > Extend jdk.includeInExceptions to include "file", meaning opt-in in secure > deployments to include the tmp directory in the exception. or maybe change the code in `File.createTempFile(...)` to something like: diff --git a/src/java.base/share/classes/java/io/File.java b/src/java.base/share/classes/java/io/File.java index 8f1e88219fe..4eb9db40e9d 100644 --- a/src/java.base/share/classes/java/io/File.java +++ b/src/java.base/share/classes/java/io/File.java @@ -2157,7 +2157,9 @@ public class File File tmpdir = (directory != null) ? directory : TempDirectory.location(); - + if (!tmpdir.isDirectory()) { + throw new IOException("Parent directory doesn't exist, cannot create a temporary file"); + } @SuppressWarnings("removal") SecurityManager sm = System.getSecurityManager(); File f; I haven't given much thought to the diff I proposed here, so this obviously will need more experimentation and inputs from others whether it's worth pursuing. Additionally, if we do decide to improve the exception being thrown, perhaps we should also review the code in `java.nio.file.Files.createTemp....` methods and the exceptions they throw. ------------- PR: https://git.openjdk.org/jdk/pull/11174
