Adjusted TaskListener flushing in remote code

core/src/main/java/hudson/Launcher.java

@@ -761,6 +761,7 @@ protected final void printCommandLine(@Nonnull String[] cmd, @CheckForNull FileP
                 buf.append(c);
         }
         listener.getLogger().println(buf.toString());
+        listener.getLogger().flush();
     }
 
     /**

core/src/main/java/hudson/console/AnnotatedLargeText.java

@@ -52,6 +52,7 @@
 import com.jcraft.jzlib.GZIPOutputStream;
 
 import static java.lang.Math.abs;
+import javax.annotation.CheckReturnValue;
 import org.jenkinsci.remoting.util.AnonymousClassWarnings;
 
 /**
@@ -138,6 +139,7 @@ private ConsoleAnnotator<T> createAnnotator(StaplerRequest req) throws IOExcepti
         return ConsoleAnnotator.initial(context);
     }
 
+    @CheckReturnValue
     @Override
     public long writeLogTo(long start, Writer w) throws IOException {
         if (isHtml())
@@ -150,6 +152,7 @@ public long writeLogTo(long start, Writer w) throws IOException {
      * Strips annotations using a {@link PlainTextConsoleOutputStream}.
      * {@inheritDoc}
      */
+    @CheckReturnValue
     @Override
     public long writeLogTo(long start, OutputStream out) throws IOException {
         return super.writeLogTo(start, new PlainTextConsoleOutputStream(out));
@@ -159,10 +162,12 @@ public long writeLogTo(long start, OutputStream out) throws IOException {
      * Calls {@link LargeText#writeLogTo(long, OutputStream)} without stripping annotations as {@link #writeLogTo(long, OutputStream)} would.
      * @since 1.577
      */
+    @CheckReturnValue
     public long writeRawLogTo(long start, OutputStream out) throws IOException {
         return super.writeLogTo(start, out);
     }
 
+    @CheckReturnValue
     public long writeHtmlTo(long start, Writer w) throws IOException {
         ConsoleAnnotationOutputStream<T> caw = new ConsoleAnnotationOutputStream<>(
                 w, createAnnotator(Stapler.getCurrentRequest()), context, charset);

core/src/main/java/hudson/console/ConsoleAnnotationOutputStream.java

@@ -49,7 +49,7 @@
  * @since 1.349
  */
 public class ConsoleAnnotationOutputStream<T> extends LineTransformationOutputStream {
-    private final Writer out;
+    private final Writer out; // not an OutputStream so cannot use LineTransformationOutputStream.Delegating
     private final T context;
     private ConsoleAnnotator<T> ann;
 

core/src/main/java/hudson/console/ConsoleLogFilter.java

@@ -36,13 +36,19 @@
 import javax.annotation.Nonnull;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.io.Serializable;
+import jenkins.util.JenkinsJVM;
 
 /**
  * A hook to allow filtering of information that is written to the console log.
  * Unlike {@link ConsoleAnnotator} and {@link ConsoleNote}, this class provides
  * direct access to the underlying {@link OutputStream} so it's possible to suppress
  * data, which isn't possible from the other interfaces.
  * ({@link ArgumentListBuilder#add(String, boolean)} is a simpler way to suppress a single password.)
+ * <p>Implementations which are {@link Serializable} may be sent to an agent JVM for processing.
+ * In particular, this happens under <a href="https://jenkins.io/jep/210">JEP-210</a>.
+ * In this case, the implementation should not assume that {@link JenkinsJVM#isJenkinsJVM},
+ * and if generating {@link ConsoleNote}s will need to encode them on the master side first.
  * @author dty
  * @since 1.383
  * @see BuildWrapper#decorateLogger

core/src/main/java/hudson/console/ConsoleNote.java

@@ -107,6 +107,17 @@
  * is also important, although {@link ConsoleNote}s that failed to deserialize will be simply ignored, so the
  * worst thing that can happen is that you just lose some notes.
  *
+ * <p>
+ * Note that {@link #encode}, {@link #encodeTo(OutputStream)}, and {@link #encodeTo(Writer)}
+ * should be called on the Jenkins master.
+ * If called from an agent JVM, a signature will be missing and so as per
+ * <a href="https://jenkins.io/security/advisory/2017-02-01/#persisted-cross-site-scripting-vulnerability-in-console-notes">SECURITY-382</a>
+ * the console note will be ignored.
+ * This may happen, in particular, if the note was generated by a {@link ConsoleLogFilter} sent to the agent.
+ * Alternative solutions include using a {@link ConsoleAnnotatorFactory} where practical;
+ * or generating the encoded form of the note on the master side and sending it to the agent,
+ * for example by saving that form as instance fields in a {@link ConsoleLogFilter} implementation.
+ *
  * <h2>Behaviour, JavaScript, and CSS</h2>
  * <p>
  * {@link ConsoleNote} can have associated {@code script.js} and {@code style.css} (put them

core/src/main/java/hudson/console/LineTransformationOutputStream.java

@@ -32,7 +32,7 @@
  * Filtering {@link OutputStream} that buffers text by line, so that the derived class
  * can perform some manipulation based on the contents of the whole line.
  *
- * TODO: Mac is supposed to be CR-only. This class needs to handle that.
+ * <p>Subclass {@link Delegating} in the typical case that you are decorating an underlying stream.
  *
  * @author Kohsuke Kawaguchi
  * @since 1.349
@@ -110,4 +110,32 @@ protected String trimEOL(String line) {
     }
 
     private static final int LF = 0x0A;
+
+    /**
+     * Convenience subclass for cases where you wish to process lines being sent to an underlying stream.
+     * {@link #eol} will typically {@link OutputStream#write(byte[], int, int)} to {@link #out}.
+     * Flushing or closing the decorated stream will behave properly.
+     * @since FIXME
+     */
+    public static abstract class Delegating extends LineTransformationOutputStream {
+
+        protected final OutputStream out;
+
+        protected Delegating(OutputStream out) {
+            this.out = out;
+        }
+
+        @Override
+        public void flush() throws IOException {
+            out.flush();
+        }
+
+        @Override
+        public void close() throws IOException {
+            super.close();
+            out.close();
+        }
+
+    }
+
 }

core/src/main/java/hudson/console/PlainTextConsoleOutputStream.java

@@ -28,21 +28,19 @@
 import java.io.DataInputStream;
 import java.io.IOException;
 import java.io.OutputStream;
-import java.util.logging.Logger;
 
 /**
  * Filters out console notes.
  *
  * @author Kohsuke Kawaguchi
  */
-public class PlainTextConsoleOutputStream extends LineTransformationOutputStream {
-    private final OutputStream out;
+public class PlainTextConsoleOutputStream extends LineTransformationOutputStream.Delegating {
 
     /**
      *
      */
     public PlainTextConsoleOutputStream(OutputStream out) {
-        this.out = out;
+        super(out);
     }
 
     /**
@@ -77,17 +75,4 @@ protected void eol(byte[] in, int sz) throws IOException {
         out.write(in,written,sz-written);
     }
 
-    @Override
-    public void flush() throws IOException {
-        out.flush();
-    }
-
-    @Override
-    public void close() throws IOException {
-        super.close();
-        out.close();
-    }
-
-
-    private static final Logger LOGGER = Logger.getLogger(PlainTextConsoleOutputStream.class.getName());
 }

core/src/main/java/hudson/tasks/_maven/MavenConsoleAnnotator.java

@@ -36,12 +36,11 @@
  *
  * @author Kohsuke Kawaguchi
  */
-public class MavenConsoleAnnotator extends LineTransformationOutputStream {
-    private final OutputStream out;
+public class MavenConsoleAnnotator extends LineTransformationOutputStream.Delegating {
     private final Charset charset;
 
     public MavenConsoleAnnotator(OutputStream out, Charset charset) {
-        this.out = out;
+        super(out);
         this.charset = charset;
     }
 
@@ -75,9 +74,4 @@ protected void eol(byte[] b, int len) throws IOException {
         out.write(b,0,len);
     }
 
-    @Override
-    public void close() throws IOException {
-        super.close();
-        out.close();
-    }
 }

core/src/main/java/hudson/util/StreamCopyThread.java

@@ -63,8 +63,11 @@ public void run() {
                 // it doesn't make sense not to close InputStream that's already EOF-ed,
                 // so there's no 'closeIn' flag.
                 in.close();
-                if(closeOut)
+                if (closeOut) {
                     out.close();
+                } else {
+                    out.flush();
+                }
             }
         } catch (IOException e) {
             // TODO: what to do?

core/src/main/java/hudson/util/StreamTaskListener.java

@@ -169,7 +169,7 @@ private void writeObject(ObjectOutputStream out) throws IOException {
     }
 
     private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
-        out = new PrintStream((OutputStream)in.readObject(),true);
+        out = new PrintStream((OutputStream)in.readObject(), false);
         String name = (String)in.readObject();
         charset = name==null ? null : Charset.forName(name);
     }

test/src/test/java/hudson/console/ConsoleLogFilterTest.java

@@ -38,8 +38,8 @@ public OutputStream decorateLogger(Run build, OutputStream logger) throws IOExce
         }
 
         @Override
-        public OutputStream decorateLogger(final Computer c, final OutputStream out) throws IOException, InterruptedException {
-            return new LineTransformationOutputStream() {
+        public OutputStream decorateLogger(final Computer c, OutputStream out) throws IOException, InterruptedException {
+            return new LineTransformationOutputStream.Delegating(out) {
                 @Override
                 protected void eol(byte[] b, int len) throws IOException {
                     out.write(("[["+c.getName()+"]] ").getBytes());

test/src/test/java/hudson/util/ArgumentListBuilder2Test.java

@@ -27,7 +27,6 @@
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.*;
 import hudson.Functions;
 import hudson.Launcher.LocalLauncher;
@@ -46,7 +45,6 @@
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.StringWriter;
-import java.net.URL;
 
 /**
  * @author Kohsuke Kawaguchi
@@ -71,8 +69,7 @@ public void slaveMask() throws Exception {
 
         StringWriter out = new StringWriter();
         assertEquals(0,s.createLauncher(new StreamTaskListener(out)).launch().cmds(args).join());
-        System.out.println(out);
-        assertTrue(out.toString().contains("$ java ********"));
+        assertThat(out.toString(), containsString("$ java ********"));
     }
 
     @Test

test/src/test/java/jenkins/tasks/SimpleBuildWrapperTest.java

@@ -187,10 +187,10 @@ public static class WrapperWithLogger extends SimpleBuildWrapper {
         private static class UpcaseFilter extends ConsoleLogFilter implements Serializable {
             private static final long serialVersionUID = 1;
             @SuppressWarnings("rawtypes") // inherited
-            @Override public OutputStream decorateLogger(AbstractBuild _ignore, final OutputStream logger) throws IOException, InterruptedException {
-                return new LineTransformationOutputStream() {
+            @Override public OutputStream decorateLogger(AbstractBuild _ignore, OutputStream logger) throws IOException, InterruptedException {
+                return new LineTransformationOutputStream.Delegating(logger) {
                     @Override protected void eol(byte[] b, int len) throws IOException {
-                        logger.write(new String(b, 0, len).toUpperCase(Locale.ROOT).getBytes());
+                        out.write(new String(b, 0, len).toUpperCase(Locale.ROOT).getBytes());
                     }
                 };
             }