[JENKINS-71926] Robustness against NULs in ArgumentsActionImpl

Author: jglick

plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/actions/ArgumentsActionImpl.java

@@ -31,7 +31,6 @@
 import hudson.model.Result;
 import java.util.Collection;
 import jenkins.model.Jenkins;
-import org.apache.commons.io.output.NullOutputStream;
 import org.jenkinsci.plugins.structs.describable.DescribableModel;
 import org.jenkinsci.plugins.structs.describable.UninstantiatedDescribable;
 import org.jenkinsci.plugins.workflow.actions.ArgumentsAction;
@@ -41,6 +40,7 @@
 
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
+import java.io.OutputStream;
 import java.lang.reflect.Type;
 import java.net.URL;
 import java.util.ArrayList;
@@ -240,6 +240,9 @@ Object sanitizeObjectAndRecordMutation(@CheckForNull Object o, @CheckForNull Env
                 this.isUnmodifiedBySanitization = true;
                 return NotStoredReason.UNSERIALIZABLE;
             }
+        } else if (modded instanceof String && ((String) modded).contains("\0")) {
+            this.isUnmodifiedBySanitization = false;
+            return "<contains ASCII NUL>";
         } else if (modded instanceof String && vars != null && !vars.isEmpty()) {
             String replaced = replaceSensitiveVariables((String)modded, vars, sensitiveVariables);
             if (!replaced.equals(modded)) {
@@ -282,7 +285,7 @@ Map<String, Object> serializationCheck(@NonNull Map<String, Object> arguments) {
             try {
                 if (val != null && !(val instanceof String) && !(val instanceof Boolean) && !(val instanceof Number) && !(val instanceof NotStoredReason) && !(val instanceof TimeUnit)) {
                     // We only need to check serialization for nontrivial types
-                    Jenkins.XSTREAM2.toXMLUTF8(entry.getValue(), NullOutputStream.NULL_OUTPUT_STREAM);  // Hacky but can't find a better way
+                    Jenkins.XSTREAM2.toXMLUTF8(entry.getValue(), OutputStream.nullOutputStream());  // Hacky but can't find a better way
                 }
                 out.put(entry.getKey(), entry.getValue());
             } catch (Exception ex) {

plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/actions/ArgumentsActionImplTest.java

@@ -70,6 +70,7 @@
 import java.io.Serializable;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
+import java.nio.file.Files;
 import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -566,6 +567,14 @@ public void simpleSemaphoreStep() throws Exception {
         testDeserialize(run.getExecution());
     }
 
+    @Test
+    public void nul() throws Exception {
+        var job = r.createProject(WorkflowJob.class);
+        job.setDefinition(new CpsFlowDefinition("echo 'one\\0two'; echo 'this part is fine'", true));
+        var store = r.buildAndAssertSuccess(job).getRootDir().toPath().resolve("workflow-completed/flowNodeStore.xml");
+        assertThat(store + " was written", Files.readString(store), containsString("this part is fine"));
+    }
+
     @Test
     public void testArgumentDescriptions() throws Exception {
         WorkflowJob job = r.createProject(WorkflowJob.class);