[JENKINS-34753] Provide safe parameters to ParametersAction

And fixed some checkstyle errors in the tests
jenkinsci · May 18, 2016 · 3116820 · 3116820
1 parent b1c6b52
commit 3116820
Show file tree

Hide file tree

Showing 7 changed files with 43 additions and 7 deletions.
diff --git a/...java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java b/...java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java
@@ -40,7 +40,6 @@
 import hudson.model.Result;
 import hudson.model.Run;
 import hudson.model.TaskListener;
-import hudson.model.Hudson;
 
 import java.util.Arrays;
 import java.util.Collections;

diff --git a/...main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java b/...main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java
@@ -48,7 +48,9 @@
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
+import java.lang.reflect.Constructor;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.Future;
 
@@ -251,6 +253,17 @@ protected Job asJob() {
     protected ParametersAction createParameters(GerritTriggeredEvent event, Job project) {
         List<ParameterValue> parameters = getDefaultParametersValues(project);
         setOrCreateParameters(event, project, parameters);
+        try {
+            Constructor<ParametersAction> constructor = ParametersAction.class.getConstructor(List.class,
+                                                                                              Collection.class);
+            return constructor.newInstance(parameters, GerritTriggerParameters.getNamesSet());
+        } catch (NoSuchMethodException e) {
+            logger.debug("Running on an old core before safe parameters, we are safe.", e);
+        } catch (IllegalAccessException e) {
+            logger.warn("Running on a core with safe parameters fix available, but not allowed to specify them", e);
+        } catch (Exception e) {
+            logger.warn("Running on a core with safe parameters fix available, but failed to provide them", e);
+        }
         return new ParametersAction(parameters);
     }
 

diff --git a/...com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerParameters.java b/...com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerParameters.java
@@ -50,6 +50,8 @@
 import java.lang.reflect.Constructor;
 import java.nio.charset.Charset;
 import java.util.List;
+import java.util.Set;
+import java.util.TreeSet;
 
 /**
  * The parameters to add to a build.
@@ -217,6 +219,20 @@ public enum GerritTriggerParameters {
 
     private static final Logger logger = LoggerFactory.getLogger(GerritTriggerParameters.class);
 
+    /**
+     * A set of all the declared parameter names.
+     * @return the names of the parameters
+     * @see #values()
+     * @see #name()
+     */
+    public static Set<String> getNamesSet() {
+        Set<String> names = new TreeSet<String>();
+        for (GerritTriggerParameters p : GerritTriggerParameters.values()) {
+            names.add(p.name());
+        }
+        return names;
+    }
+
     /**
      * Creates a {@link hudson.model.ParameterValue} and adds it to the provided list.
      * If the parameter with the same name already exists in the list it will be replaced by the new parameter,

diff --git a/...sson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderParameterizedTest.java b/...sson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderParameterizedTest.java
@@ -72,6 +72,9 @@ public ParameterExpanderParameterizedTest(TestParameters parameters) {
         this.parameters = parameters;
     }
 
+    /**
+     * Mock Jenkins.
+     */
     @Before
     public void setup() {
         PowerMockito.mockStatic(Jenkins.class);

diff --git a/.../hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java b/.../hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java
@@ -73,6 +73,9 @@ public ParameterExpanderSkipVoteParameterTest(TestParameter parameter) {
         this.parameter = parameter;
     }
 
+    /**
+     * Mock Jenkins.
+     */
     @Before
     public void setup() {
         PowerMockito.mockStatic(Jenkins.class);

diff --git a/.../com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java b/.../com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java
@@ -79,6 +79,9 @@ public class ParameterExpanderTest {
 
     private Jenkins jenkins;
 
+    /**
+     * Mock Jenkins.
+     */
     @Before
     public void setup() {
         PowerMockito.mockStatic(Jenkins.class);

diff --git a/...ricsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/TriggerContextConverterTest.java b/...ricsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/TriggerContextConverterTest.java
@@ -36,9 +36,7 @@
 import com.thoughtworks.xstream.XStream;
 import hudson.ExtensionList;
 import hudson.diagnosis.OldDataMonitor;
-import hudson.matrix.MatrixProject;
 import hudson.matrix.MatrixRun;
-import hudson.model.AbstractItem;
 import hudson.model.Cause;
 import hudson.model.Saveable;
 import hudson.util.XStream2;
@@ -67,8 +65,6 @@
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyCollection;
-import static org.mockito.Matchers.anyString;
-import static org.mockito.Matchers.eq;
 import static org.mockito.Matchers.same;
 import static org.powermock.api.mockito.PowerMockito.doNothing;
 import static org.powermock.api.mockito.PowerMockito.doReturn;
@@ -83,12 +79,15 @@
  * @author Robert Sandell &lt;robert.sandell@sonyericsson.com&gt;
  */
 @RunWith(PowerMockRunner.class)
-@PrepareForTest({Jenkins.class, OldDataMonitor.class, ExtensionList.class})
+@PrepareForTest({Jenkins.class, OldDataMonitor.class, ExtensionList.class })
 public class TriggerContextConverterTest {
     private Jenkins jenkins;
 
-    //CS IGNORE MagicNumber FOR NEXT 500 LINES. REASON: test data.
+    //CS IGNORE MagicNumber FOR NEXT 600 LINES. REASON: test data.
 
+    /**
+     * Mock Jenkins.
+     */
     @Before
     public void setup() {
         PowerMockito.mockStatic(Jenkins.class);