From ffe99f00b800e259b0953c161cfa31f6f8cf0ef5 Mon Sep 17 00:00:00 2001
From: Basil Crow <me@basilcrow.com>
Date: Fri, 23 Dec 2022 09:41:39 -0800
Subject: [PATCH] De-duplicate file-based logging implementations (#7256)

---
 pom.xml                                       |   2 +-
 war/pom.xml                                   |  14 +-
 .../java/executable/LogFileOutputStream.java  | 123 ------------------
 war/src/main/java/executable/Main.java        |  74 ++++-------
 4 files changed, 31 insertions(+), 182 deletions(-)
 delete mode 100644 war/src/main/java/executable/LogFileOutputStream.java

diff --git a/pom.xml b/pom.xml
index 814a50ef12..6fc4010b33 100644
--- a/pom.xml
+++ b/pom.xml
@@ -98,7 +98,7 @@ THE SOFTWARE.
     <antlr.version>4.11.1</antlr.version>
     <bridge-method-injector.version>1.25</bridge-method-injector.version>
     <spotless.version>2.28.0</spotless.version>
-    <winstone.version>6.6</winstone.version>
+    <winstone.version>6.7</winstone.version>
   </properties>
 
   <!--
diff --git a/war/pom.xml b/war/pom.xml
index 1e94707b21..b41d36144c 100644
--- a/war/pom.xml
+++ b/war/pom.xml
@@ -200,23 +200,17 @@ THE SOFTWARE.
           </execution>
         </executions>
       </plugin>
-      <!-- JENKINS-68021: Work around JDK-8206937 by clearing the release=8 flag. -->
       <plugin>
         <artifactId>maven-compiler-plugin</artifactId>
         <configuration>
-          <source>1.8</source>
-          <target>1.8</target>
-          <testSource>1.8</testSource>
-          <testTarget>1.8</testTarget>
-          <release combine.self="override" />
-          <testRelease combine.self="override" />
+          <release>8</release>
+          <testRelease>8</testRelease>
         </configuration>
       </plugin>
       <plugin>
         <artifactId>maven-javadoc-plugin</artifactId>
         <configuration>
-          <source>1.8</source>
-          <release combine.self="override" />
+          <release>8</release>
         </configuration>
       </plugin>
       <plugin>
@@ -656,7 +650,7 @@ THE SOFTWARE.
       <plugin>
         <groupId>org.eclipse.jetty</groupId>
         <artifactId>jetty-maven-plugin</artifactId>
-        <version>10.0.12</version>
+        <version>10.0.13</version>
         <configuration>
           <!--
             Reload webapp when you hit ENTER. (See JETTY-282 for more)
diff --git a/war/src/main/java/executable/LogFileOutputStream.java b/war/src/main/java/executable/LogFileOutputStream.java
deleted file mode 100644
index d860dce2a9..0000000000
--- a/war/src/main/java/executable/LogFileOutputStream.java
+++ /dev/null
@@ -1,123 +0,0 @@
-/*
- * The MIT License
- *
- * Copyright (c) 2008, Sun Microsystems, Inc.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-package executable;
-
-import edu.umd.cs.findbugs.annotations.NonNull;
-import java.io.File;
-import java.io.FileNotFoundException;
-import java.io.FileOutputStream;
-import java.io.FilterOutputStream;
-import java.io.IOException;
-import java.io.OutputStream;
-import java.io.UncheckedIOException;
-import sun.misc.Signal;
-
-/**
- * {@link OutputStream} that writes to a log file.
- *
- * <p>
- * Unlike the plain {@link FileOutputStream}, this implementation
- * listens to SIGALRM and reopens the log file. This behavior is
- * necessary for allowing log rotations to happen smoothly.
- *
- * <p>
- * Because the reopen operation needs to happen atomically,
- * write operations are synchronized.
- *
- * @author Kohsuke Kawaguchi
- */
-final class LogFileOutputStream extends FilterOutputStream {
-    /**
-     * This is where we are writing.
-     */
-    private final File file;
-
-    LogFileOutputStream(File file) throws FileNotFoundException {
-        super(null);
-        this.file = file;
-        out = new FileOutputStream(file, true);
-
-        if (File.pathSeparatorChar == ':') {
-            Signal.handle(new Signal("ALRM"), signal -> {
-                try {
-                    reopen();
-                } catch (IOException e) {
-                    throw new UncheckedIOException(e); // failed to reopen
-                }
-            });
-        }
-    }
-
-    public synchronized void reopen() throws IOException {
-        out.close();
-        out = NULL; // in case reopen fails, initialize with NULL first
-        out = new FileOutputStream(file, true);
-    }
-
-    @Override
-    public synchronized void write(@NonNull byte[] b) throws IOException {
-        out.write(b);
-    }
-
-    @Override
-    public synchronized void write(@NonNull byte[] b, int off, int len) throws IOException {
-        out.write(b, off, len);
-    }
-
-    @Override
-    public synchronized void flush() throws IOException {
-        out.flush();
-    }
-
-    @Override
-    public synchronized void close() throws IOException {
-        out.close();
-    }
-
-    @Override
-    public synchronized void write(int b) throws IOException {
-        out.write(b);
-    }
-
-    @Override
-    public String toString() {
-        return getClass().getName() + " -> " + file;
-    }
-
-    /**
-     * /dev/null
-     */
-    private static final OutputStream NULL = new OutputStream() {
-        @Override
-        public void write(int b) {
-            // noop
-        }
-
-        @Override
-        public void write(@NonNull byte[] b, int off, int len) {
-            // noop
-        }
-    };
-}
diff --git a/war/src/main/java/executable/Main.java b/war/src/main/java/executable/Main.java
index 88dda31e86..4b3656372c 100644
--- a/war/src/main/java/executable/Main.java
+++ b/war/src/main/java/executable/Main.java
@@ -28,12 +28,10 @@ package executable;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.File;
-import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.io.PrintStream;
 import java.io.UncheckedIOException;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
@@ -52,13 +50,24 @@ import java.util.TreeSet;
 import java.util.UUID;
 import java.util.jar.JarFile;
 import java.util.jar.Manifest;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 /**
  * Launcher class for stand-alone execution of Jenkins as
  * {@code java -jar jenkins.war}.
  *
+ * <p>On a high-level architectural note, this class is intended to be a very thin wrapper whose
+ * primary purpose is to extract Winstone and delegate to Winstone's own initialization mechanism.
+ * The logic in this class should only perform Jenkins-specific argument and environment validation
+ * and Jenkins-specific Winstone customization prior to delegating to Winstone.
+ *
+ * <p>In particular, managing the logging subsystem is completely delegated to Winstone, and this
+ * class should neither assume that logging has been initialized nor take advantage of the logging
+ * subsystem. In the event that this class needs to print information to the user, it should do so
+ * via the standard output (stdout) and standard error (stderr) streams rather than via the logging
+ * subsystem. Such messages should generally be avoided except for fatal scenarios, such as an
+ * inappropriate Java Virtual Machine (JVM) or some other serious failure that would preclude
+ * starting Winstone.
+ *
  * @author Kohsuke Kawaguchi
  */
 public class Main {
@@ -66,8 +75,6 @@ public class Main {
     private static final NavigableSet<Integer> SUPPORTED_JAVA_VERSIONS =
             new TreeSet<>(Arrays.asList(11, 17));
 
-    private static final Logger LOGGER = Logger.getLogger(Main.class.getName());
-
     /**
      * Sets custom session cookie name.
      * It may be used to prevent randomization of JSESSIONID cookies and issues like
@@ -98,18 +105,16 @@ public class Main {
             // Great!
         } else if (releaseVersion >= SUPPORTED_JAVA_VERSIONS.first()) {
             if (enableFutureJava) {
-                LOGGER.log(
-                        Level.WARNING,
-                        "Running with Java {0} from {1}, which is not fully supported."
-                            + " Continuing because {2} is set."
-                            + " Supported Java versions are: {3}."
-                            + " See https://jenkins.io/redirect/java-support/ for more information.",
-                        new Object[] {
-                            releaseVersion,
-                            System.getProperty("java.home"),
-                            ENABLE_FUTURE_JAVA_CLI_SWITCH,
-                            SUPPORTED_JAVA_VERSIONS,
-                        });
+                System.err.println(
+                        String.format(
+                                "Running with Java %d from %s, which is not fully supported. "
+                                        + "Continuing because %s is set. "
+                                        + "Supported Java versions are: %s. "
+                                        + "See https://jenkins.io/redirect/java-support/ for more information.",
+                                releaseVersion,
+                                System.getProperty("java.home"),
+                                ENABLE_FUTURE_JAVA_CLI_SWITCH,
+                                SUPPORTED_JAVA_VERSIONS));
             } else if (releaseVersion > SUPPORTED_JAVA_VERSIONS.last()) {
                 throw new UnsupportedClassVersionError(
                         String.format(
@@ -219,19 +224,6 @@ public class Main {
             }
         }
 
-        // if the output should be redirect to a file, do it now
-        for (int i = 0; i < args.length; i++) {
-            if (args[i].startsWith("--logfile=")) {
-                PrintStream ps = createLogFileStream(new File(args[i].substring("--logfile=".length())));
-                System.setOut(ps);
-                System.setErr(ps);
-                // don't let winstone see this
-                List<String> _args = new ArrayList<>(Arrays.asList(args));
-                _args.remove(i);
-                args = _args.toArray(new String[0]);
-                break;
-            }
-        }
         for (String arg : args) {
             if (arg.startsWith("--pluginroot=")) {
                 System.setProperty("hudson.PluginManager.workDir",
@@ -280,7 +272,7 @@ public class Main {
         }
         deleteWinstoneTempContents(new File(tempFile.getParent(), "winstone/" + me.getName()));
         if (!tempFile.delete()) {
-            LOGGER.log(Level.WARNING, "Failed to delete the temporary file {0}", tempFile);
+            System.err.println("Failed to delete temporary file: " + tempFile);
         }
 
         // locate the Winstone launcher
@@ -314,7 +306,6 @@ public class Main {
                 "   --pluginroot             = folder where the plugin archives are expanded into. Default is ${JENKINS_HOME}/plugins\n" +
                 "                              (NOTE: this option does not change the directory where the plugin archives are stored)\n" +
                 "   --extractedFilesFolder   = folder where extracted files are to be located. Default is the temp folder\n" +
-                "   --logfile                = redirect log messages to this file\n" +
                 "   " + ENABLE_FUTURE_JAVA_CLI_SWITCH + "     = allows running with Java versions which are not fully supported\n" +
                 "   --paramsFromStdIn        = Read parameters from standard input (stdin)\n" +
                 "   --version                = Print version to standard output (stdout) and exit\n" +
@@ -370,17 +361,6 @@ public class Main {
         }
     }
 
-    @SuppressFBWarnings(value = "DM_DEFAULT_ENCODING", justification = "--logfile relies on the default encoding, fine")
-    private static PrintStream createLogFileStream(File file) {
-        LogFileOutputStream los;
-        try {
-            los = new LogFileOutputStream(file);
-        } catch (FileNotFoundException e) {
-            throw new UncheckedIOException(e);
-        }
-        return new PrintStream(los);
-    }
-
     // TODO: Get rid of FB warning after updating to Java 7
     /**
      * reads up to maxRead bytes from InputStream if available into a String
@@ -402,7 +382,7 @@ public class Main {
     }
 
     private static void trimOffOurOptions(List<String> arguments) {
-        arguments.removeIf(arg -> arg.startsWith("--daemon") || arg.startsWith("--logfile") || arg.startsWith("--extractedFilesFolder")
+        arguments.removeIf(arg -> arg.startsWith("--extractedFilesFolder")
                 || arg.startsWith("--pluginroot") || arg.startsWith(ENABLE_FUTURE_JAVA_CLI_SWITCH));
     }
 
@@ -516,7 +496,6 @@ public class Main {
             for (File file : files) {
                 for (String pattern : patterns) {
                     if (file.getName().matches(pattern)) {
-                        LOGGER.log(Level.FINE, "Deleting the temporary file {0}", file);
                         deleteWinstoneTempContents(file);
                     }
                 }
@@ -526,7 +505,6 @@ public class Main {
 
     private static void deleteWinstoneTempContents(File file) {
         if (!file.exists()) {
-            LOGGER.log(Level.FINEST, "No file found at {0}, nothing to delete.", file);
             return;
         }
         if (file.isDirectory()) {
@@ -538,7 +516,7 @@ public class Main {
             }
         }
         if (!file.delete()) {
-            LOGGER.log(Level.WARNING, "Failed to delete the temporary Winstone file {0}", file);
+            System.err.println("Failed to delete temporary Winstone file: " + file);
         }
     }