Merge pull request #286 from jglick/fauxDescribable-JENKINS-54186

[JENKINS-54186] ArgumentsActionImpl robustness against weird Describable
jenkinsci · Apr 26, 2019 · fb55655 · fb55655
2 parents 62fb226 + 9edfcf6
commit fb55655
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 2 deletions.
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
@@ -291,8 +291,14 @@ Object sanitizeObjectAndRecordMutation(@CheckForNull Object o, @CheckForNull Env
         } else if (tempVal instanceof UninstantiatedDescribable) {
             tempVal = ((UninstantiatedDescribable)tempVal).toMap();
         } else if (tempVal instanceof Describable) {  // Raw Describables may not be safe to store, so we should explode it
-            m = DescribableModel.of(tempVal.getClass());
-            tempVal = m.uninstantiate2(o).toMap();
+            try {
+                m = DescribableModel.of(tempVal.getClass());
+                tempVal = m.uninstantiate2(o).toMap();
+            } catch (RuntimeException x) { // usually NoStaplerConstructorException, but could also be misc. UnsupportedOperationException
+                LOGGER.log(Level.FINE, "failed to store " + tempVal, x);
+                this.isUnmodifiedBySanitization = false;
+                return NotStoredReason.UNSERIALIZABLE;
+            }
         }
 
         if (isOversized(tempVal)) {

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
@@ -59,7 +59,10 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.logging.Level;
 import org.jenkinsci.plugins.structs.describable.UninstantiatedDescribable;
+import org.jenkinsci.plugins.workflow.testMetaStep.Curve;
+import org.jvnet.hudson.test.LoggerRule;
 
 /**
  * Tests the input sanitization and step persistence here
@@ -72,6 +75,8 @@ public class ArgumentsActionImplTest {
     @Rule
     public JenkinsRule r = new JenkinsRule();
 
+    @Rule public LoggerRule logging = new LoggerRule();
+
     /** Helper function to test direct file deserialization for an execution */
     private void testDeserialize(FlowExecution execution) throws Exception {
         if (!(execution instanceof CpsFlowExecution) || !(((CpsFlowExecution)execution).getStorage() instanceof SimpleXStreamFlowNodeStorage)) {
@@ -328,6 +333,18 @@ public boolean equals(Object ob) {
         }
     }
 
+    @Issue("JENKINS-54186")
+    @Test public void fauxDescribable() throws Exception {
+        logging.record(ArgumentsActionImpl.class, Level.FINE);
+        ArgumentsActionImpl impl = new ArgumentsActionImpl(Collections.singletonMap("curve", new Fractal()));
+        Map<String, Object> args = impl.getArguments();
+        Assert.assertThat(args, IsMapContaining.hasEntry("curve", ArgumentsAction.NotStoredReason.UNSERIALIZABLE));
+    }
+    public static final class Fractal extends Curve {
+        @Override public String getDescription() {
+            return "shape way too complex to describe";
+        }
+    }
 
     @Test
     @Issue("JENKINS-54032")