[JEP-206] Define API for gathering command output in a local encoding

src/main/java/org/jenkinsci/plugins/durabletask/Controller.java

@@ -49,6 +49,8 @@ public abstract class Controller implements Serializable {
      * @param workspace the workspace in use
      * @param sink where to send new log output
      * @return true if something was written and the controller should be resaved, false if everything is idle
+     * @see DurableTask#charset
+     * @see DurableTask#defaultCharset
      */
     public abstract boolean writeLog(FilePath workspace, OutputStream sink) throws IOException, InterruptedException;
 
@@ -88,6 +90,8 @@ public abstract class Controller implements Serializable {
      * @param workspace the workspace in use
      * @param launcher a way to start processes
      * @return the output of the process as raw bytes (may be empty but not null)
+     * @see DurableTask#charset
+     * @see DurableTask#defaultCharset
      */
     public @Nonnull byte[] getOutput(@Nonnull FilePath workspace, @Nonnull Launcher launcher) throws IOException, InterruptedException {
         throw new IOException("Did not implement getOutput in " + getClass().getName());

src/main/java/org/jenkinsci/plugins/durabletask/DurableTask.java

@@ -31,13 +31,19 @@
 import hudson.model.AbstractDescribableImpl;
 import hudson.model.TaskListener;
 import java.io.IOException;
+import java.nio.charset.Charset;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import javax.annotation.Nonnull;
 
 /**
  * A task which may be run asynchronously on a build node and withstand disconnection of the slave agent.
  * Should have a descriptor, and a {@code config.jelly} for form data binding.
  */
 public abstract class DurableTask extends AbstractDescribableImpl<DurableTask> implements ExtensionPoint {
 
+    private static final Logger LOGGER = Logger.getLogger(DurableTask.class.getName());
+
     @Override public DurableTaskDescriptor getDescriptor() {
         return (DurableTaskDescriptor) super.getDescriptor();
     }
@@ -63,4 +69,23 @@ public void captureOutput() throws UnsupportedOperationException {
         throw new UnsupportedOperationException("Capturing of output is not implemented in " + getClass().getName());
     }
 
+    /**
+     * Requests that a specified charset be used to transcode process output.
+     * The encoding of {@link Controller#writeLog} and {@link Controller#getOutput} is then presumed to be UTF-8.
+     * If not called, no translation is performed.
+     * @param cs the character set in which process output is expected to be
+     */
+    public void charset(@Nonnull Charset cs) {
+        LOGGER.log(Level.WARNING, "The charset method should be overridden in {0}", getClass().getName());
+    }
+
+    /**
+     * Requests that the node’s system charset be used to transcode process output.
+     * The encoding of {@link Controller#writeLog} and {@link Controller#getOutput} is then presumed to be UTF-8.
+     * If not called, no translation is performed.
+     */
+    public void defaultCharset() {
+        LOGGER.log(Level.WARNING, "The defaultCharset method should be overridden in {0}", getClass().getName());
+    }
+
 }

src/main/java/org/jenkinsci/plugins/durabletask/FileMonitoringTask.java

@@ -37,23 +37,26 @@
 import hudson.util.StreamTaskListener;
 import java.io.File;
 import java.io.IOException;
-import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.RandomAccessFile;
 import java.io.StringWriter;
+import java.nio.ByteBuffer;
+import java.nio.channels.Channels;
 import java.nio.charset.Charset;
+import java.nio.charset.CodingErrorAction;
+import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.Map;
 import java.util.TreeMap;
 import java.util.UUID;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
 import jenkins.MasterToSlaveFileCallable;
-import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.output.CountingOutputStream;
 
-import javax.annotation.CheckForNull;
-
 /**
  * A task which forks some external command and then waits for log and status files to be updated/created.
  */
@@ -63,19 +66,37 @@ public abstract class FileMonitoringTask extends DurableTask {
 
     private static final String COOKIE = "JENKINS_SERVER_COOKIE";
 
+    /** Value of {@link #charset} used to mean the node’s system default. */
+    private static final String SYSTEM_DEFAULT_CHARSET = "SYSTEM_DEFAULT";
+
+    /**
+     * Charset name to use for transcoding, or {@link #SYSTEM_DEFAULT_CHARSET}, or null for no transcoding.
+     */
+    private @CheckForNull String charset;
+
     private static String cookieFor(FilePath workspace) {
         return "durable-" + Util.getDigestOf(workspace.getRemote());
     }
 
     @Override public final Controller launch(EnvVars env, FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException {
-        return launchWithCookie(workspace, launcher, listener, env, COOKIE, cookieFor(workspace));
+        FileMonitoringController controller = launchWithCookie(workspace, launcher, listener, env, COOKIE, cookieFor(workspace));
+        controller.charset = charset;
+        return controller;
     }
 
     protected FileMonitoringController launchWithCookie(FilePath workspace, Launcher launcher, TaskListener listener, EnvVars envVars, String cookieVariable, String cookieValue) throws IOException, InterruptedException {
         envVars.put(cookieVariable, cookieValue); // ensure getCharacteristicEnvVars does not match, so Launcher.killAll will leave it alone
         return doLaunch(workspace, launcher, listener, envVars);
     }
 
+    @Override public final void charset(Charset cs) {
+        charset = cs.name();
+    }
+
+    @Override public final void defaultCharset() {
+        charset = SYSTEM_DEFAULT_CHARSET;
+    }
+
     /**
      * Should start a process which sends output to {@linkplain FileMonitoringController#getLogFile(FilePath) log file}
      * in the workspace and finally writes its exit code to {@linkplain FileMonitoringController#getResultFile(FilePath) result file}.
@@ -115,6 +136,9 @@ protected static class FileMonitoringController extends Controller {
          */
         private long lastLocation;
 
+        /** @see FileMonitoringTask#charset */
+        private @CheckForNull String charset;
+
         protected FileMonitoringController(FilePath ws) throws IOException, InterruptedException {
             // can't keep ws reference because Controller is expected to be serializable
             ws.mkdirs();
@@ -127,7 +151,7 @@ protected FileMonitoringController(FilePath ws) throws IOException, InterruptedE
             FilePath log = getLogFile(workspace);
             CountingOutputStream cos = new CountingOutputStream(sink);
             try {
-                log.act(new WriteLog(lastLocation, new RemoteOutputStream(cos)));
+                log.act(new WriteLog(lastLocation, new RemoteOutputStream(cos), charset));
                 return cos.getByteCount() > 0;
             } finally { // even if RemoteOutputStream write was interrupted, record what we actually received
                 long written = cos.getByteCount();
@@ -140,9 +164,11 @@ protected FileMonitoringController(FilePath ws) throws IOException, InterruptedE
         private static class WriteLog extends MasterToSlaveFileCallable<Void> {
             private final long lastLocation;
             private final OutputStream sink;
-            WriteLog(long lastLocation, OutputStream sink) {
+            private final @CheckForNull String charset;
+            WriteLog(long lastLocation, OutputStream sink, String charset) {
                 this.lastLocation = lastLocation;
                 this.sink = sink;
+                this.charset = charset;
             }
             @Override public Void invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
                 long len = f.length();
@@ -155,7 +181,12 @@ private static class WriteLog extends MasterToSlaveFileCallable<Void> {
                         }
                         byte[] buf = new byte[(int) toRead];
                         raf.readFully(buf);
-                        sink.write(buf);
+                        ByteBuffer transcoded = maybeTranscode(buf, charset);
+                        if (transcoded == null) {
+                            sink.write(buf);
+                        } else {
+                            Channels.newChannel(sink).write(transcoded);
+                        }
                     }
                 }
                 return null;
@@ -195,13 +226,42 @@ public Integer invoke(File f, VirtualChannel channel) throws IOException, Interr
             return status.act(STATUS_CHECK_INSTANCE);
         }
 
-        @Override public byte[] getOutput(FilePath workspace, Launcher launcher) throws IOException, InterruptedException {
-            // TODO could perhaps be more efficient for large files to send a MasterToSlaveFileCallable<byte[]>
-            try (InputStream is = getOutputFile(workspace).read()) {
-                return IOUtils.toByteArray(is);
+        /**
+         * Transcode process output to UTF-8 if necessary.
+         * @param data output presumed to be in local encoding
+         * @param charset a particular encoding name, or the empty string for the system default encoding, or null to skip transcoding
+         * @return a buffer of UTF-8 encoded data ({@link CodingErrorAction#REPLACE} is used),
+         *         or null if not performing transcoding because it was not requested or the data was already thought to be in UTF-8
+         */
+        private static @CheckForNull ByteBuffer maybeTranscode(@Nonnull byte[] data, @CheckForNull String charset) {
+            if (charset == null) { // no transcoding requested, do raw copy and YMMV
+                return null;
+            } else {
+                Charset cs = charset.equals(SYSTEM_DEFAULT_CHARSET) ? Charset.defaultCharset() : Charset.forName(charset);
+                if (cs.equals(StandardCharsets.UTF_8)) { // transcoding unnecessary as output was already UTF-8
+                    return null;
+                } else { // decode output in specified charset and reëncode in UTF-8
+                    return StandardCharsets.UTF_8.encode(cs.decode(ByteBuffer.wrap(data)));
+                }
             }
         }
 
+        @Override public byte[] getOutput(FilePath workspace, Launcher launcher) throws IOException, InterruptedException {
+            return getOutputFile(workspace).act(new MasterToSlaveFileCallable<byte[]>() {
+                @Override public byte[] invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
+                    byte[] buf = FileUtils.readFileToByteArray(f);
+                    ByteBuffer transcoded = maybeTranscode(buf, charset);
+                    if (transcoded == null) {
+                        return buf;
+                    } else {
+                        byte[] buf2 = new byte[transcoded.remaining()];
+                        transcoded.get(buf2);
+                        return buf2;
+                    }
+                }
+            });
+        }
+
         @Override public final void stop(FilePath workspace, Launcher launcher) throws IOException, InterruptedException {
             launcher.kill(Collections.singletonMap(COOKIE, cookieFor(workspace)));
         }

src/test/java/org/jenkinsci/plugins/durabletask/BourneShellScriptTest.java

@@ -34,8 +34,10 @@
 import hudson.util.VersionNumber;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.nio.charset.Charset;
 import java.util.Collections;
 import java.util.logging.Level;
+import jenkins.security.MasterToSlaveCallable;
 import org.apache.commons.io.output.TeeOutputStream;
 import static org.hamcrest.Matchers.*;
 import org.jenkinsci.test.acceptance.docker.Docker;
@@ -236,4 +238,61 @@ private void runOnDocker(DumbSlave s) throws Exception {
         runOnDocker(new DumbSlave("docker", "/home/jenkins/agent", new SimpleCommandLauncher("docker run -i --rm --name agent --init jenkinsci/slave:3.7-1 java -jar /usr/share/jenkins/slave.jar")));
     }
 
+    @Issue("JENKINS-31096")
+    @Test public void encoding() throws Exception {
+        JavaContainer container = dockerUbuntu.get();
+        DumbSlave s = new DumbSlave("docker", "/home/test", new SSHLauncher(container.ipBound(22), container.port(22), "test", "test", "", "-Dfile.encoding=ISO-8859-1"));
+        j.jenkins.addNode(s);
+        j.waitOnline(s);
+        assertEquals("ISO-8859-1", s.getChannel().call(new DetectCharset()));
+        FilePath dockerWS = s.getWorkspaceRoot();
+        dockerWS.child("latin").write("¡Ole!", "ISO-8859-1");
+        dockerWS.child("eastern").write("Čau!", "ISO-8859-2");
+        dockerWS.child("mixed").write("¡Čau → there!", "UTF-8");
+        Launcher dockerLauncher = s.createLauncher(listener);
+        assertEncoding("control: no transcoding", "latin", null, "¡Ole!", "ISO-8859-1", dockerWS, dockerLauncher);
+        assertEncoding("test: specify particular charset (UTF-8)", "mixed", "UTF-8", "¡Čau → there!", "UTF-8", dockerWS, dockerLauncher);
+        assertEncoding("test: specify particular charset (unrelated)", "eastern", "ISO-8859-2", "Čau!", "UTF-8", dockerWS, dockerLauncher);
+        assertEncoding("test: specify agent default charset", "latin", "", "¡Ole!", "UTF-8", dockerWS, dockerLauncher);
+        assertEncoding("test: inappropriate charset, some replacement characters", "mixed", "US-ASCII", "����au ��� there!", "UTF-8", dockerWS, dockerLauncher);
+        s.toComputer().disconnect(new OfflineCause.UserCause(null, null));
+    }
+    private static class DetectCharset extends MasterToSlaveCallable<String, RuntimeException> {
+        @Override public String call() throws RuntimeException {
+            return Charset.defaultCharset().name();
+        }
+    }
+    private void assertEncoding(String description, String file, String charset, String expected, String expectedEncoding, FilePath dockerWS, Launcher dockerLauncher) throws Exception {
+        assertEncoding(description, file, charset, expected, expectedEncoding, false, dockerWS, dockerLauncher);
+        assertEncoding(description, file, charset, expected, expectedEncoding, true, dockerWS, dockerLauncher);
+    }
+    private void assertEncoding(String description, String file, String charset, String expected, String expectedEncoding, boolean output, FilePath dockerWS, Launcher dockerLauncher) throws Exception {
+        BourneShellScript dt = new BourneShellScript("cat " + file);
+        if (charset != null) {
+            if (charset.isEmpty()) {
+                dt.defaultCharset();
+            } else {
+                dt.charset(Charset.forName(charset));
+            }
+        }
+        if (output) {
+            dt.captureOutput();
+        }
+        Controller c = dt.launch(new EnvVars(), dockerWS, dockerLauncher, listener);
+        while (c.exitStatus(dockerWS, dockerLauncher, listener) == null) {
+            Thread.sleep(100);
+        }
+        assertEquals(description, 0, c.exitStatus(dockerWS, dockerLauncher, listener).intValue());
+        if (output) {
+            c.writeLog(dockerWS, System.err);
+            assertEquals(description, expected, new String(c.getOutput(dockerWS, launcher), expectedEncoding));
+        } else {
+            ByteArrayOutputStream baos = new ByteArrayOutputStream();
+            c.writeLog(dockerWS, baos);
+            assertThat(description, baos.toString(expectedEncoding), containsString(expected));
+        }
+        c.cleanup(dockerWS);
+
+    }
+
 }