diff --git a/src/java.base/share/classes/java/lang/ProcessBuilder.java b/src/java.base/share/classes/java/lang/ProcessBuilder.java index 0b86dd507ac..e870682a38e 100644 --- a/src/java.base/share/classes/java/lang/ProcessBuilder.java +++ b/src/java.base/share/classes/java/lang/ProcessBuilder.java @@ -190,6 +190,9 @@ import jdk.internal.event.ProcessStartEvent; public final class ProcessBuilder { + // Lazily and racy initialize when needed, racy is ok, any logger is ok + private static System.Logger LOGGER; + private List command; private File directory; private Map environment; @@ -1067,6 +1070,19 @@ public final class ProcessBuilder * * @throws IOException if an I/O error occurs * + * @implNote + * In the reference implementation, logging of the command, arguments, directory, + * stack trace, and process id can be enabled. + * The logged information may contain sensitive security information and the potential exposure + * of the information should be carefully reviewed. + * Logging of the information is enabled when the logging level of the + * {@linkplain System#getLogger(String) system logger} named {@code java.lang.ProcessBuilder} + * is {@link System.Logger.Level#DEBUG Level.DEBUG} or {@link System.Logger.Level#TRACE Level.TRACE}. + * When enabled for {@code Level.DEBUG} only the process id, directory, command, and stack trace + * are logged. + * When enabled for {@code Level.TRACE} the arguments are included with the process id, + * directory, command, and stack trace. + * * @see Runtime#exec(String[], String[], java.io.File) */ public Process start() throws IOException { @@ -1119,6 +1135,21 @@ public final class ProcessBuilder event.pid = process.pid(); event.commit(); } + // Racy initialization for logging; errors in configuration may throw exceptions + System.Logger logger = LOGGER; + if (logger == null) { + LOGGER = logger = System.getLogger("java.lang.ProcessBuilder"); + } + if (logger.isLoggable(System.Logger.Level.DEBUG)) { + boolean detail = logger.isLoggable(System.Logger.Level.TRACE); + var level = (detail) ? System.Logger.Level.TRACE : System.Logger.Level.DEBUG; + var cmdargs = (detail) ? String.join("\" \"", cmdarray) : cmdarray[0]; + RuntimeException stackTraceEx = new RuntimeException("ProcessBuilder.start() debug"); + LOGGER.log(level, "ProcessBuilder.start(): pid: " + process.pid() + + ", dir: " + dir + + ", cmd: \"" + cmdargs + "\"", + stackTraceEx); + } return process; } catch (IOException | IllegalArgumentException e) { String exceptionInfo = ": " + e.getMessage(); @@ -1263,6 +1294,10 @@ public final class ProcessBuilder * @throws UnsupportedOperationException * If the operating system does not support the creation of processes * + * @implNote + * In the reference implementation, logging of each process created can be enabled, + * see {@link ProcessBuilder#start()} for details. + * * @throws IOException if an I/O error occurs * @since 9 */ diff --git a/src/java.base/share/classes/java/lang/Runtime.java b/src/java.base/share/classes/java/lang/Runtime.java index 2a5c37b2ae9..f77c7168457 100644 --- a/src/java.base/share/classes/java/lang/Runtime.java +++ b/src/java.base/share/classes/java/lang/Runtime.java @@ -349,6 +349,10 @@ public class Runtime { * @throws IllegalArgumentException * If {@code command} is empty * + * @implNote + * In the reference implementation, logging of the created process can be enabled, + * see {@link ProcessBuilder#start()} for details. + * * @see #exec(String[], String[], File) * @see ProcessBuilder */ @@ -397,6 +401,10 @@ public class Runtime { * @throws IllegalArgumentException * If {@code command} is empty * + * @implNote + * In the reference implementation, logging of the created process can be enabled, + * see {@link ProcessBuilder#start()} for details. + * * @see #exec(String[], String[], File) * @see ProcessBuilder */ @@ -458,6 +466,10 @@ public class Runtime { * @throws IllegalArgumentException * If {@code command} is empty * + * @implNote + * In the reference implementation, logging of the created process can be enabled, + * see {@link ProcessBuilder#start()} for details. + * * @see ProcessBuilder * @since 1.3 */ @@ -503,6 +515,10 @@ public class Runtime { * If {@code cmdarray} is an empty array * (has length {@code 0}) * + * @implNote + * In the reference implementation, logging of the created process can be enabled, + * see {@link ProcessBuilder#start()} for details. + * * @see ProcessBuilder */ public Process exec(String[] cmdarray) throws IOException { @@ -546,6 +562,10 @@ public class Runtime { * If {@code cmdarray} is an empty array * (has length {@code 0}) * + * @implNote + * In the reference implementation, logging of the created process can be enabled, + * see {@link ProcessBuilder#start()} for details. + * * @see ProcessBuilder */ public Process exec(String[] cmdarray, String[] envp) throws IOException { @@ -641,6 +661,10 @@ public class Runtime { * If {@code cmdarray} is an empty array * (has length {@code 0}) * + * @implNote + * In the reference implementation, logging of the created process can be enabled, + * see {@link ProcessBuilder#start()} for details. + * * @see ProcessBuilder * @since 1.3 */ diff --git a/test/jdk/java/lang/ProcessBuilder/ProcessLogging-FINE.properties b/test/jdk/java/lang/ProcessBuilder/ProcessLogging-FINE.properties new file mode 100644 index 00000000000..45f4b3c342d --- /dev/null +++ b/test/jdk/java/lang/ProcessBuilder/ProcessLogging-FINE.properties @@ -0,0 +1,8 @@ +############################################################ +# Enable logging java.lang.ProcessBuilder to the console +############################################################ + +handlers= java.util.logging.ConsoleHandler + +java.util.logging.ConsoleHandler.level = ALL +java.lang.ProcessBuilder.level = FINE diff --git a/test/jdk/java/lang/ProcessBuilder/ProcessLogging-FINER.properties b/test/jdk/java/lang/ProcessBuilder/ProcessLogging-FINER.properties new file mode 100644 index 00000000000..3986d99f46b --- /dev/null +++ b/test/jdk/java/lang/ProcessBuilder/ProcessLogging-FINER.properties @@ -0,0 +1,8 @@ +############################################################ +# Enable logging java.lang.ProcessBuilder to the console +############################################################ + +handlers= java.util.logging.ConsoleHandler + +java.util.logging.ConsoleHandler.level = ALL +java.lang.ProcessBuilder.level = FINER diff --git a/test/jdk/java/lang/ProcessBuilder/ProcessLogging-INFO.properties b/test/jdk/java/lang/ProcessBuilder/ProcessLogging-INFO.properties new file mode 100644 index 00000000000..f7b50e8aad2 --- /dev/null +++ b/test/jdk/java/lang/ProcessBuilder/ProcessLogging-INFO.properties @@ -0,0 +1,8 @@ +############################################################ +# Enable logging java.lang.ProcessBuilder to the console +############################################################ + +handlers= java.util.logging.ConsoleHandler + +java.util.logging.ConsoleHandler.level = ALL +java.lang.ProcessBuilder.level = INFO diff --git a/test/jdk/java/lang/ProcessBuilder/ProcessStartLoggingTest.java b/test/jdk/java/lang/ProcessBuilder/ProcessStartLoggingTest.java new file mode 100644 index 00000000000..e33ec0bc69a --- /dev/null +++ b/test/jdk/java/lang/ProcessBuilder/ProcessStartLoggingTest.java @@ -0,0 +1,189 @@ +/* + * Copyright (c) 2023, 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. + */ + +import java.io.BufferedReader; +import java.io.File; +import java.io.IOException; +import java.nio.file.Path; +import java.util.List; +import java.util.logging.LogRecord; +import java.util.logging.Logger; +import java.util.logging.StreamHandler; +import java.util.stream.Stream; + +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.ParameterizedTest; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +/* + * @test + * @summary verify logging of ProcessBuilder.start() + * @run junit/othervm ProcessStartLoggingTest + */ +public class ProcessStartLoggingTest { + + private final static int NORMAL_STATUS = 0; + private final static int ERROR_STATUS = 1; + + private static final String TEST_JDK = System.getProperty("test.jdk"); + private static final String TEST_SRC = System.getProperty("test.src"); + + private static Object HOLD_LOGGER; + + /** + * Launch a process with the arguments. + * @param args 1 or strings passed directly to ProcessBuilder as command and arguments. + */ + public static void main(String[] args) throws InterruptedException { + if (System.getProperty("ThrowingHandler") != null) { + HOLD_LOGGER = ProcessStartLoggingTest.ThrowingHandler.installHandler(); + } + String directory = System.getProperty("directory"); + try { + ProcessBuilder pb = new ProcessBuilder(args); + pb.directory((directory == null) ? null : new File(directory)); + Process p = pb.start(); + int status = p.waitFor(); + if (status != 0) { + System.out.println("exitValue: " + status); + } + } catch (IOException ioe) { + System.out.println("ProcessBuilder.start() threw IOException: " + ioe); + } + } + + /** + * Test various log level settings, and none. + * @return a stream of arguments for parameterized test + */ + private static Stream logParamProvider() { + + return Stream.of( + // Logging enabled with level TRACE + Arguments.of(List.of("-Djava.util.logging.config.file=" + + Path.of(TEST_SRC, "ProcessLogging-FINER.properties").toString(), + "-Ddirectory=."), + List.of("echo", "echo0"), + NORMAL_STATUS, + "dir: ., cmd: \"echo\" \"echo0\""), + // Logging enabled with level DEBUG + Arguments.of(List.of("-Djava.util.logging.config.file=" + + Path.of(TEST_SRC, "ProcessLogging-FINE.properties").toString(), + "-Ddirectory=."), + List.of("echo", "echo1"), + NORMAL_STATUS, + "dir: ., cmd: \"echo\""), + // Logging disabled due to level INFO + Arguments.of(List.of("-Djava.util.logging.config.file=" + + Path.of(TEST_SRC, "ProcessLogging-INFO.properties").toString()), + List.of("echo", "echo2"), + NORMAL_STATUS, + ""), + // Console logger DEBUG + Arguments.of(List.of("--limit-modules", "java.base", + "-Djdk.system.logger.level=DEBUG"), + List.of("echo", "echo3"), + NORMAL_STATUS, + "dir: null, cmd: \"echo\""), + // Console logger TRACE + Arguments.of(List.of("--limit-modules", "java.base", + "-Djdk.system.logger.level=TRACE", + "-Ddirectory=."), + List.of("echo", "echo4"), + NORMAL_STATUS, + "dir: ., cmd: \"echo\" \"echo4\""), + // No Logging configured + Arguments.of(List.of(), + List.of("echo", "echo5"), + NORMAL_STATUS, + ""), + // Throwing Handler + Arguments.of(List.of("-DThrowingHandler", + "-Djava.util.logging.config.file=" + + Path.of(TEST_SRC, "ProcessLogging-FINE.properties").toString()), + List.of("echo", "echo6"), + ERROR_STATUS, + "Exception in thread \"main\" java.lang.RuntimeException: Exception in publish") + ); + } + + /** + * Check that the logger output of a launched process contains the expected message. + * + * @param logArgs Arguments to configure logging in the java test process + * @param childArgs the args passed to the child to be invoked as a Process + * @param expectMessage log should contain the message + */ + @ParameterizedTest + @MethodSource("logParamProvider") + public void checkLogger(List logArgs, List childArgs, + int expectedStatus, String expectMessage) { + ProcessBuilder pb = new ProcessBuilder(); + pb.redirectErrorStream(true); + + List cmd = pb.command(); + cmd.add(Path.of(TEST_JDK,"bin", "java").toString()); + cmd.addAll(logArgs); + cmd.add(this.getClass().getName()); + cmd.addAll(childArgs); + try { + Process process = pb.start(); + try (BufferedReader reader = process.inputReader()) { + List lines = reader.lines().toList(); + boolean match = (expectMessage.isEmpty()) + ? lines.size() == 0 + : lines.stream().filter(s -> s.contains(expectMessage)).findFirst().isPresent(); + if (!match) { + // Output lines for debug + System.err.println("Expected> \"" + expectMessage + "\""); + lines.forEach(l -> System.err.println("Actual> \"" + l+ "\"")); + fail("Unexpected log contents"); + } + } + int result = process.waitFor(); + assertEquals(expectedStatus, result, "Exit status"); + } catch (IOException | InterruptedException ex) { + fail(ex); + } + } + + /** + * A LoggingHandler that throws an Exception. + */ + public static class ThrowingHandler extends StreamHandler { + + // Install this handler for java.lang.ProcessBuilder + public static Logger installHandler() { + Logger logger = Logger.getLogger("java.lang.ProcessBuilder"); + logger.addHandler(new ProcessStartLoggingTest.ThrowingHandler()); + return logger; + } + + @Override + public synchronized void publish(LogRecord record) { + throw new RuntimeException("Exception in publish"); + } + } +}