Bulletproof against JENKINS-50752 by catching unserializable step arg…

…uments
jenkinsci · Apr 11, 2018 · e9fe93d · e9fe93d
1 parent 64187fd
commit e9fe93d
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 2 deletions.
diff --git a/pom.xml b/pom.xml
@@ -80,7 +80,8 @@
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-api</artifactId>
-            <version>2.25</version>
+            <!-- Temp version until upstream PR released -->
+            <version>2.27-20180411.200653-14</version>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/cps/actions/ArgumentsActionImpl.java b/src/main/java/org/jenkinsci/plugins/workflow/cps/actions/ArgumentsActionImpl.java
@@ -25,7 +25,9 @@
 package org.jenkinsci.plugins.workflow.cps.actions;
 
 import com.google.common.collect.Maps;
+import com.google.common.io.NullOutputStream;
 import hudson.EnvVars;
+import jenkins.model.Jenkins;
 import org.jenkinsci.plugins.structs.describable.UninstantiatedDescribable;
 import org.jenkinsci.plugins.workflow.actions.ArgumentsAction;
 import org.jenkinsci.plugins.workflow.steps.Step;
@@ -34,6 +36,7 @@
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
+import java.io.OutputStream;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -42,6 +45,8 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
+import java.util.logging.Logger;
 import java.util.regex.Pattern;
 
 
@@ -59,7 +64,7 @@ public class ArgumentsActionImpl extends ArgumentsAction {
     boolean isUnmodifiedBySanitization = true;
 
     public ArgumentsActionImpl(@Nonnull Map<String, Object> stepArguments, @CheckForNull EnvVars env) {
-        arguments = sanitizeMapAndRecordMutation(stepArguments, env);
+        arguments = serializationCheck(sanitizeMapAndRecordMutation(stepArguments, env));
     }
 
     /** Create a step, sanitizing strings for secured content */
@@ -267,6 +272,35 @@ Object sanitizeObjectAndRecordMutation(@CheckForNull Object o, @CheckForNull Env
         }
     }
 
+    static final OutputStream NULL_STRM = new NullOutputStream();
+
+    /** Verify that all the arguments WILL serialize and if not replace with {@link org.jenkinsci.plugins.workflow.actions.ArgumentsAction.NotStoredReason#UNSERIALIZABLE}
+     *  See JENKINS-50752 for details, but the gist is we need to avoid problems before physical persistence to prevent data loss.
+     *  @return Arguments
+     */
+    Map<String, Object> serializationCheck(@Nonnull Map<String, Object> arguments) {
+        boolean isMutated = false;
+        HashMap<String, Object> out = Maps.newHashMapWithExpectedSize(arguments.size());
+        NullOutputStream strm = new NullOutputStream();
+        for (Map.Entry<String, Object> entry : arguments.entrySet()) {
+            Object val = entry.getValue();
+            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(), NULL_STRM);  // Hacky but can't find a better way
+                }
+                out.put(entry.getKey(), entry.getValue());
+            } catch (Exception ex) {
+                out.put(entry.getKey(), NotStoredReason.UNSERIALIZABLE);
+                isMutated = true;
+            }
+        }
+        if (isMutated) {
+            this.isUnmodifiedBySanitization = false;
+        }
+        return out;
+    }
+
     /**
      * Goes through {@link #sanitizeObjectAndRecordMutation(Object, EnvVars)} for each value in a map input.
      */

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/cps/actions/ArgumentsActionImplTest.java b/src/test/java/org/jenkinsci/plugins/workflow/cps/actions/ArgumentsActionImplTest.java
@@ -9,15 +9,20 @@
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
+import groovy.lang.MissingMethodException;
 import hudson.EnvVars;
 import hudson.Functions;
 import hudson.XmlFile;
 import hudson.model.Action;
+import hudson.model.Result;
+import hudson.remoting.ProxyException;
 import hudson.tasks.ArtifactArchiver;
 import jenkins.model.Jenkins;
 import org.apache.commons.lang.RandomStringUtils;
+import org.hamcrest.Matchers;
 import org.jenkinsci.plugins.credentialsbinding.impl.BindingStep;
 import org.jenkinsci.plugins.workflow.actions.ArgumentsAction;
+import org.jenkinsci.plugins.workflow.actions.ErrorAction;
 import org.jenkinsci.plugins.workflow.actions.StageAction;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowExecution;
@@ -46,6 +51,10 @@
 import org.junit.Rule;
 import org.junit.Test;
 import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
 import org.jvnet.hudson.test.BuildWatcher;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
@@ -271,6 +280,22 @@ public void testBasicCreateAndMask() throws Exception {
         Assert.assertEquals(ArgumentsAction.NotStoredReason.OVERSIZE_VALUE, argumentsActionImpl.getArgumentValueOrReason("text"));
     }
 
+    @Test
+    @Issue("JENKINS-50752")
+    public void testHandleUnserializableArguments() throws Exception {
+        HashMap<String, Object> unserializable = new HashMap<String, Object>(3);
+        Object me = new Object() {
+            Object writeReplace() {
+                throw new RuntimeException("Can't serialize me nyah nyah!");
+            }
+        };
+        unserializable.put("ex", me);
+
+        ArgumentsActionImpl impl = new ArgumentsActionImpl(unserializable);
+        Assert.assertEquals(ArgumentsAction.NotStoredReason.UNSERIALIZABLE, impl.getArgumentValueOrReason("ex"));
+        Assert.assertFalse("Should show argument removed by sanitization", impl.isUnmodifiedArguments());
+    }
+
     @Test
     public void testBasicCredentials() throws Exception {
         String username = "bob";