[FIXED JENKINS-23522] Defend against stack overflows when a listener …

…(unnecessarily) called super from a deprecated method. Fixes #1295 a little differently.
jenkinsci · Aug 29, 2014 · cb6e568 · cb6e568
1 parent d33a632
commit cb6e568
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 1 deletion.
diff --git a/changelog.html b/changelog.html
@@ -58,6 +58,9 @@
   <li class=bug>
     <code>ConcurrentModificationException</code> in <code>RunListProgressiveRendering</code>.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-21437">issue 21437</a>)
+  <li class=bug>
+    <code>StackOverflowError</code> for some old <code>SCMListener</code>s.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-23522">issue 23522</a>)
   <li class=bug>
     Job status page shows "Build has been executing for null on master" for flyweight tasks.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-20307">issue 20307</a>)

diff --git a/core/src/main/java/hudson/model/listeners/SCMListener.java b/core/src/main/java/hudson/model/listeners/SCMListener.java
@@ -116,7 +116,9 @@ public void onChangeLogParsed(Run<?,?> build, SCM scm, TaskListener listener, Ch
 
     @Deprecated
     public void onChangeLogParsed(AbstractBuild<?,?> build, BuildListener listener, ChangeLogSet<?> changelog) throws Exception {
-        onChangeLogParsed((Run) build, build.getProject().getScm(), listener, changelog);
+        if (Util.isOverridden(SCMListener.class, getClass(), "onChangeLogParsed", Run.class, SCM.class, TaskListener.class, ChangeLogSet.class)) {
+            onChangeLogParsed((Run) build, build.getProject().getScm(), listener, changelog);
+        }
     }
 
     /**

diff --git a/core/src/test/java/hudson/model/listeners/SCMListenerTest.java b/core/src/test/java/hudson/model/listeners/SCMListenerTest.java
@@ -0,0 +1,95 @@
+/*
+ * The MIT License
+ *
+ * Copyright 2014 Jesse Glick.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package hudson.model.listeners;
+
+import hudson.model.AbstractBuild;
+import hudson.model.AbstractProject;
+import hudson.model.BuildListener;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.scm.ChangeLogSet;
+import hudson.scm.SCM;
+import org.junit.Test;
+import static org.junit.Assert.*;
+import org.jvnet.hudson.test.Issue;
+import org.mockito.Mockito;
+
+@SuppressWarnings("deprecation")
+public class SCMListenerTest {
+
+    @Issue("JENKINS-23522")
+    @SuppressWarnings({"rawtypes", "unchecked"})
+    @Test public void onChangeLogParsed() throws Exception {
+        SCM scm = Mockito.mock(SCM.class);
+        BuildListener bl = Mockito.mock(BuildListener.class);
+        ChangeLogSet cls = Mockito.mock(ChangeLogSet.class);
+        AbstractBuild ab = Mockito.mock(AbstractBuild.class);
+        AbstractProject ap = Mockito.mock(AbstractProject.class);
+        Mockito.when(ab.getProject()).thenReturn(ap);
+        Mockito.when(ap.getScm()).thenReturn(scm);
+        for (L l : new L[] {new L1(), new L2(), new L3()}) {
+            assertEquals(0, l.cnt);
+            l.onChangeLogParsed(ab, bl, cls);
+            assertEquals(1, l.cnt);
+            l.onChangeLogParsed(ab, scm, bl, cls);
+            assertEquals(2, l.cnt);
+        }
+        Run r = Mockito.mock(Run.class);
+        TaskListener tl = Mockito.mock(TaskListener.class);
+        L l = new L1();
+        l.onChangeLogParsed(r, scm, tl, cls);
+        assertEquals("cannot handle this", 0, l.cnt);
+        l = new L2();
+        l.onChangeLogParsed(r, scm, tl, cls);
+        assertEquals("does handle this", 1, l.cnt);
+        l = new L3();
+        l.onChangeLogParsed(r, scm, tl, cls);
+        assertEquals("cannot handle this", 0, l.cnt);
+    }
+
+    private static class L extends SCMListener {
+        int cnt;
+    }
+
+    private static class L1 extends L {
+        @Override public void onChangeLogParsed(AbstractBuild<?,?> build, BuildListener listener, ChangeLogSet<?> changelog) throws Exception {
+            cnt++;
+        }
+    }
+
+    private static class L2 extends L {
+        @Override public void onChangeLogParsed(Run<?,?> build, SCM scm, TaskListener listener, ChangeLogSet<?> changelog) throws Exception {
+            cnt++;
+        }
+    }
+
+    private static class L3 extends L {
+        @Override public void onChangeLogParsed(AbstractBuild<?,?> build, BuildListener listener, ChangeLogSet<?> changelog) throws Exception {
+            cnt++;
+            super.onChangeLogParsed(build, listener, changelog);
+        }
+    }
+
+}