[FIXED JENKINS-31954] Stack overflow reconfiguring some settings wher…

…e Descriptor.newInstance is gratuitously overridden.
jenkinsci · Dec 11, 2015 · 60a23e5 · 60a23e5
1 parent 8f38e20
commit 60a23e5
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 4 deletions.
diff --git a/core/src/main/java/hudson/model/Descriptor.java b/core/src/main/java/hudson/model/Descriptor.java
@@ -578,13 +578,13 @@ public T newInstance(@CheckForNull StaplerRequest req, @Nonnull JSONObject formD
                 BindInterceptor oldInterceptor = req.getBindInterceptor();
                 try {
                     NewInstanceBindInterceptor interceptor;
-                    if ((oldInterceptor instanceof NewInstanceBindInterceptor)) {
+                    if (oldInterceptor instanceof NewInstanceBindInterceptor) {
                         interceptor = (NewInstanceBindInterceptor) oldInterceptor;
                     } else {
                         interceptor = new NewInstanceBindInterceptor(oldInterceptor);
                         req.setBindInterceptor(interceptor);
                     }
-                    interceptor.processed.put(formData, null);
+                    interceptor.processed.put(formData, true);
                     return verifyNewInstance(req.bindJSON(clazz, formData));
                 } finally {
                     req.setBindInterceptor(oldInterceptor);
@@ -612,7 +612,7 @@ public T newInstance(@CheckForNull StaplerRequest req, @Nonnull JSONObject formD
     private static class NewInstanceBindInterceptor extends BindInterceptor {
 
         private final BindInterceptor oldInterceptor;
-        private final Map<JSONObject,Void> processed = new IdentityHashMap<>();
+        private final Map<JSONObject,Boolean> processed = new IdentityHashMap<>();
 
         NewInstanceBindInterceptor(BindInterceptor oldInterceptor) {
             LOGGER.log(Level.FINER, "new interceptor delegating to {0}", oldInterceptor);
@@ -628,7 +628,7 @@ private boolean isApplicable(Class type, JSONObject json) {
                 LOGGER.log(Level.FINER, "ignoring non-Describable {0} {1}", new Object[] {type.getName(), json});
                 return false;
             }
-            if (processed.containsKey(json)) {
+            if (Boolean.TRUE.equals(processed.put(json, true))) {
                 LOGGER.log(Level.FINER, "already processed {0} {1}", new Object[] {type.getName(), json});
                 return false;
             }

diff --git a/test/src/test/java/hudson/model/RunParameterDefinitionTest.java b/test/src/test/java/hudson/model/RunParameterDefinitionTest.java
@@ -49,6 +49,16 @@ public class RunParameterDefinitionTest {
     @Rule
     public JenkinsRule j = new JenkinsRule();
 
+    @Issue("JENKINS-31954")
+    @Test public void configRoundtrip() throws Exception {
+        FreeStyleProject p = j.createFreeStyleProject();
+        p.addProperty(new ParametersDefinitionProperty(new RunParameterDefinition("build", "p", "", RunParameterFilter.COMPLETED)));
+        j.configRoundtrip(p);
+        RunParameterDefinition rpd = (RunParameterDefinition) p.getProperty(ParametersDefinitionProperty.class).getParameterDefinition("build");
+        assertEquals("p", rpd.getProjectName());
+        assertEquals(RunParameterFilter.COMPLETED, rpd.getFilter());
+    }
+
     @Issue("JENKINS-16462")
     @Test public void inFolders() throws Exception {
         MockFolder dir = j.createFolder("dir");