From 3b29e9d460381f83eba9e939f6053b8336d946f4 Mon Sep 17 00:00:00 2001 From: Basil Crow Date: Fri, 7 Jan 2022 06:29:55 -0800 Subject: [PATCH] Fix more `DM_DEFAULT_ENCODING` SpotBugs violations (#6098) --- core/src/main/java/hudson/FilePath.java | 3 +-- core/src/main/java/hudson/cli/CLICommand.java | 7 ++++++- .../main/java/hudson/cli/GroovyCommand.java | 3 ++- .../main/java/hudson/cli/GroovyshCommand.java | 3 ++- .../java/hudson/cli/ListChangesCommand.java | 3 ++- .../lifecycle/WindowsServiceLifecycle.java | 7 +++++-- .../java/hudson/model/StreamBuildListener.java | 6 ++++++ core/src/main/java/hudson/os/SU.java | 5 +++-- .../java/hudson/util/StreamTaskListener.java | 18 ++++++++++++++++-- .../java/jenkins/diagnosis/HsErrPidList.java | 7 ++++--- .../slaves/DefaultJnlpSlaveReceiver.java | 6 ++++-- src/spotbugs/spotbugs-excludes.xml | 12 +----------- 12 files changed, 52 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index ca205d353b4e..8d0faa933453 100644 --- 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 index 389f0bcbf71c..913003a8aefc 100644 --- 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 index 066fd909fa86..0d7e45e6ca18 100644 --- 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 index 75c0a9c18e37..f25a2544aa02 100644 --- 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 index 36b81e5630af..59c5b8c78904 100644 --- 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> 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(""); 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 index 08193532a274..961cf1358b52 100644 --- 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 index 2b41ca48b0b8..efa1204fe11f 100644 --- 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 index 6ba5b055fd33..185110d83781 100644 --- 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 index 8f6698a4a85a..8aad40f5b599 100644 --- a/core/src/main/java/hudson/util/StreamTaskListener.java +++ b/core/src/main/java/hudson/util/StreamTaskListener.java @@ -76,6 +76,13 @@ 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); } @@ -83,7 +90,7 @@ public StreamTaskListener(@NonNull OutputStream out) { 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 index 49b2519c655a..a8a8566eb0c6 100644 --- 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 index 00d0d7d1a4e2..dbe60372e961 100644 --- 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 index fc7fcf481dfb..4b78e8570b21 100644 --- 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. - --> + --> - - - - - - - - - -