Fix more DM_DEFAULT_ENCODING SpotBugs violations (jenkinsci#6098)

basil · Jan 7, 2022 · 3b29e9d · 3b29e9d
1 parent 520e114
commit 3b29e9d
Show file tree

Hide file tree

Showing 12 changed files with 52 additions and 28 deletions.
diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java
@@ -70,7 +70,6 @@
 import java.io.BufferedOutputStream;
 import java.io.File;
 import java.io.FileFilter;
-import java.io.FileWriter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InterruptedIOException;
@@ -1656,7 +1655,7 @@ public String invoke(File dir, VirtualChannel channel) throws IOException {
                 throw new IOException("Failed to create a temporary directory in " + dir, e);
             }
 
-            try (Writer w = new FileWriter(f)) {
+            try (Writer w = Files.newBufferedWriter(Util.fileToPath(f), Charset.defaultCharset())) {
                 w.write(contents);
             }
 

diff --git a/core/src/main/java/hudson/cli/CLICommand.java b/core/src/main/java/hudson/cli/CLICommand.java
@@ -433,7 +433,12 @@ public final String getUsage() {
     @Restricted(NoExternalUse.class)
     public final String getLongDescription() {
         ByteArrayOutputStream out = new ByteArrayOutputStream();
-        PrintStream ps = new PrintStream(out);
+        PrintStream ps;
+        try {
+            ps = new PrintStream(out, false, getClientCharset().name());
+        } catch (UnsupportedEncodingException e) {
+            throw new AssertionError(e);
+        }
 
         printUsageSummary(ps);
         ps.close();

diff --git a/core/src/main/java/hudson/cli/GroovyCommand.java b/core/src/main/java/hudson/cli/GroovyCommand.java
@@ -28,6 +28,7 @@
 import groovy.lang.GroovyShell;
 import hudson.Extension;
 import java.io.IOException;
+import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.List;
@@ -63,7 +64,7 @@ protected int run() throws Exception {
         Jenkins.get().checkPermission(Jenkins.ADMINISTER);
 
         Binding binding = new Binding();
-        binding.setProperty("out", new PrintWriter(stdout, true));
+        binding.setProperty("out", new PrintWriter(new OutputStreamWriter(stdout, getClientCharset()), true));
         binding.setProperty("stdin", stdin);
         binding.setProperty("stdout", stdout);
         binding.setProperty("stderr", stderr);

diff --git a/core/src/main/java/hudson/cli/GroovyshCommand.java b/core/src/main/java/hudson/cli/GroovyshCommand.java
@@ -30,6 +30,7 @@
 import hudson.Extension;
 import java.io.BufferedInputStream;
 import java.io.InputStream;
+import java.io.OutputStreamWriter;
 import java.io.PrintStream;
 import java.io.PrintWriter;
 import java.util.ArrayList;
@@ -84,7 +85,7 @@ protected Groovysh createShell(InputStream stdin, PrintStream stdout,
 
         Binding binding = new Binding();
         // redirect "println" to the CLI
-        binding.setProperty("out", new PrintWriter(stdout, true));
+        binding.setProperty("out", new PrintWriter(new OutputStreamWriter(stdout, getClientCharset()), true));
         binding.setProperty("hudson", Jenkins.get()); // backward compatibility
         binding.setProperty("jenkins", Jenkins.get());
 

diff --git a/core/src/main/java/hudson/cli/ListChangesCommand.java b/core/src/main/java/hudson/cli/ListChangesCommand.java
@@ -5,6 +5,7 @@
 import hudson.scm.ChangeLogSet;
 import hudson.util.QuotedStringTokenizer;
 import java.io.IOException;
+import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
 import java.util.List;
 import jenkins.scm.RunWithSCM;
@@ -46,7 +47,7 @@ protected int act(List<Run<?, ?>> builds) throws IOException {
         // No other permission check needed.
         switch (format) {
         case XML:
-            PrintWriter w = new PrintWriter(stdout);
+            PrintWriter w = new PrintWriter(new OutputStreamWriter(stdout, getClientCharset()));
             w.println("<changes>");
             for (Run<?, ?> build : builds) {
                 if (build instanceof RunWithSCM) {

diff --git a/core/src/main/java/hudson/lifecycle/WindowsServiceLifecycle.java b/core/src/main/java/hudson/lifecycle/WindowsServiceLifecycle.java
@@ -33,9 +33,12 @@
 import hudson.util.StreamTaskListener;
 import hudson.util.jna.Kernel32;
 import java.io.File;
-import java.io.FileWriter;
 import java.io.IOException;
+import java.io.Writer;
 import java.net.URL;
+import java.nio.charset.Charset;
+import java.nio.file.Files;
+import java.nio.file.StandardOpenOption;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import jenkins.model.Jenkins;
@@ -110,7 +113,7 @@ public void rewriteHudsonWar(File by) throws IOException {
         File baseDir = getBaseDir();
         File copyFiles = new File(baseDir, baseName + ".copies");
 
-        try (FileWriter w = new FileWriter(copyFiles, true)) {
+        try (Writer w = Files.newBufferedWriter(Util.fileToPath(copyFiles), Charset.defaultCharset(), StandardOpenOption.CREATE, StandardOpenOption.APPEND)) {
             w.write(by.getAbsolutePath() + '>' + getHudsonWar().getAbsolutePath() + '\n');
         }
     }

diff --git a/core/src/main/java/hudson/model/StreamBuildListener.java b/core/src/main/java/hudson/model/StreamBuildListener.java
@@ -47,6 +47,12 @@ public StreamBuildListener(File out, Charset charset) throws IOException {
         super(out, charset);
     }
 
+    /**
+     * @deprecated as of TODO
+     *      The caller should use {@link #StreamBuildListener(OutputStream, Charset)} to pass in
+     *      the charset and output stream separately, so that this class can handle encoding correctly.
+     */
+    @Deprecated
     public StreamBuildListener(OutputStream w) {
         super(w);
     }

diff --git a/core/src/main/java/hudson/os/SU.java b/core/src/main/java/hudson/os/SU.java
@@ -43,6 +43,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.PrintStream;
+import java.nio.charset.Charset;
 import java.util.Collections;
 
 /**
@@ -83,7 +84,7 @@ protected String sudoExe() {
                     return "sudo";
                 }
 
-                @SuppressFBWarnings(value = {"COMMAND_INJECTION", "DM_DEFAULT_ENCODING"}, justification = "TODO needs triage")
+                @SuppressFBWarnings(value = "COMMAND_INJECTION", justification = "TODO needs triage")
                 @Override
                 protected Process sudoWithPass(ArgumentListBuilder args) throws IOException {
                     args.prepend(sudoExe(), "-S");
@@ -92,7 +93,7 @@ protected Process sudoWithPass(ArgumentListBuilder args) throws IOException {
                     Process p = pb.start();
                     // TODO: use -p to detect prompt
                     // TODO: detect if the password didn't work
-                    try (PrintStream ps = new PrintStream(p.getOutputStream())) {
+                    try (PrintStream ps = new PrintStream(p.getOutputStream(), false, Charset.defaultCharset().name())) {
                         ps.println(rootPassword);
                         ps.println(rootPassword);
                         ps.println(rootPassword);

diff --git a/core/src/main/java/hudson/util/StreamTaskListener.java b/core/src/main/java/hudson/util/StreamTaskListener.java
@@ -76,14 +76,21 @@ public StreamTaskListener(@NonNull PrintStream out) {
         this(out, null);
     }
 
+    /**
+     * @deprecated as of TODO
+     *      The caller should use {@link #StreamTaskListener(OutputStream, Charset)} to pass in
+     *      the charset and output stream separately, so that this class can handle encoding correctly,
+     *      or use {@link #fromStdout()} or {@link #fromStderr()}.
+     */
+    @Deprecated
     public StreamTaskListener(@NonNull OutputStream out) {
         this(out, null);
     }
 
     public StreamTaskListener(@NonNull OutputStream out, @CheckForNull Charset charset) {
         try {
             if (charset == null)
-                this.out = out instanceof PrintStream ? (PrintStream) out : new PrintStream(out, false);
+                this.out = out instanceof PrintStream ? (PrintStream) out : new PrintStream(out, false, Charset.defaultCharset().name());
             else
                 this.out = new PrintStream(out, false, charset.name());
             this.charset = charset;
@@ -93,6 +100,12 @@ public StreamTaskListener(@NonNull OutputStream out, @CheckForNull Charset chars
         }
     }
 
+    /**
+     * @deprecated as of TODO
+     *      The caller should use {@link #StreamTaskListener(File, Charset)} to pass in
+     *      the charset and file separately, so that this class can handle encoding correctly.
+     */
+    @Deprecated
     public StreamTaskListener(@NonNull File out) throws IOException {
         this(out, null);
     }
@@ -187,8 +200,9 @@ private void writeObject(ObjectOutputStream out) throws IOException {
     private static /* not final */ boolean AUTO_FLUSH = SystemProperties.getBoolean(KEY_AUTO_FLUSH);
 
     private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
-        out = new PrintStream((OutputStream) in.readObject(), AUTO_FLUSH);
+        OutputStream os = (OutputStream) in.readObject();
         String name = (String) in.readObject();
+        out = new PrintStream(os, AUTO_FLUSH, name != null ? name : Charset.defaultCharset().name());
         charset = name == null ? null : Charset.forName(name);
         if (LOGGER.isLoggable(Level.FINE)) {
             LOGGER.log(Level.FINE, null, new Throwable("deserializing here with AUTO_FLUSH=" + AUTO_FLUSH));

diff --git a/core/src/main/java/jenkins/diagnosis/HsErrPidList.java b/core/src/main/java/jenkins/diagnosis/HsErrPidList.java
@@ -7,12 +7,13 @@
 import hudson.util.jna.Kernel32Utils;
 import java.io.BufferedReader;
 import java.io.File;
-import java.io.FileReader;
 import java.io.IOException;
 import java.io.Reader;
 import java.nio.MappedByteBuffer;
 import java.nio.channels.FileChannel;
 import java.nio.channels.FileChannel.MapMode;
+import java.nio.charset.Charset;
+import java.nio.file.Files;
 import java.nio.file.InvalidPathException;
 import java.nio.file.StandardOpenOption;
 import java.util.ArrayList;
@@ -123,7 +124,7 @@ private void scan(String pattern) {
     private void scanFile(File log) {
         LOGGER.fine("Scanning " + log);
 
-        try (Reader rawReader = new FileReader(log);
+        try (Reader rawReader = Files.newBufferedReader(log.toPath(), Charset.defaultCharset());
              BufferedReader r = new BufferedReader(rawReader)) {
 
             if (!findHeader(r))
@@ -140,7 +141,7 @@ private void scanFile(File log) {
                     return;
                 }
             }
-        } catch (IOException e) {
+        } catch (IOException | InvalidPathException e) {
             // not a big enough deal.
             LOGGER.log(Level.FINE, "Failed to parse hs_err_pid file: " + log, e);
         }

diff --git a/core/src/main/java/jenkins/slaves/DefaultJnlpSlaveReceiver.java b/core/src/main/java/jenkins/slaves/DefaultJnlpSlaveReceiver.java
@@ -16,10 +16,12 @@
 import hudson.slaves.SlaveComputer;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.nio.channels.ClosedChannelException;
+import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
 import java.util.concurrent.ExecutionException;
@@ -152,7 +154,7 @@ public void beforeChannel(@NonNull JnlpConnectionState event) {
         final SlaveComputer computer = state.getNode();
         final OutputStream log = computer.openLogFile();
         state.setLog(log);
-        try (PrintWriter logw = new PrintWriter(log, true)) {
+        try (PrintWriter logw = new PrintWriter(new OutputStreamWriter(log, /* TODO switch agent logs to UTF-8 */ Charset.defaultCharset()), true)) {
             logw.println("Inbound agent connected from " + event.getRemoteEndpointDescription());
         }
         for (ChannelConfigurator cc : ChannelConfigurator.all()) {
@@ -172,7 +174,7 @@ public void afterChannel(@NonNull JnlpConnectionState event) {
         try {
             computer.setChannel(event.getChannel(), state.getLog(), null);
         } catch (IOException | InterruptedException e) {
-            PrintWriter logw = new PrintWriter(state.getLog(), true);
+            PrintWriter logw = new PrintWriter(new OutputStreamWriter(state.getLog(), /* TODO switch agent logs to UTF-8 */ Charset.defaultCharset()), true);
             Functions.printStackTrace(e, logw);
             try {
                 event.getChannel().close();

diff --git a/src/spotbugs/spotbugs-excludes.xml b/src/spotbugs/spotbugs-excludes.xml
@@ -45,22 +45,12 @@
       annotation indicating the reason why it is a false positive, then remove the exclusion from
       this section.
     - If it is not a false positive, fix the bug, then remove the exclusion from this section.
-  -->
+   -->
   <Match>
     <And>
       <Bug pattern="DM_DEFAULT_ENCODING"/>
       <Or>
-        <Class name="hudson.cli.CLICommand"/>
-        <Class name="hudson.cli.GroovyCommand"/>
-        <Class name="hudson.cli.GroovyshCommand"/>
-        <Class name="hudson.cli.ListChangesCommand"/>
-        <Class name="hudson.FilePath$CreateTextTempFile"/>
-        <Class name="hudson.lifecycle.WindowsServiceLifecycle"/>
-        <Class name="hudson.util.StreamTaskListener"/>
         <Class name="hudson.util.TextFile"/>
-        <Class name="jenkins.diagnosis.HsErrPidList"/>
-        <Class name="jenkins.security.s2m.AdminWhitelistRule"/>
-        <Class name="jenkins.slaves.DefaultJnlpSlaveReceiver"/>
       </Or>
     </And>
   </Match>