[FIXED JENKINS-23614] URL isn't escaped properly (leads to an 404 error)

jenkinsci · Jan 11, 2015 · f106844 · f106844
1 parent cfdf9e3
commit f106844
Show file tree

Hide file tree

Showing 5 changed files with 145 additions and 6 deletions.
diff --git a/src/main/java/hudson/matrix/AxisDescriptor.java b/src/main/java/hudson/matrix/AxisDescriptor.java
@@ -26,8 +26,10 @@
 import hudson.Util;
 import hudson.model.Descriptor;
 import hudson.model.Failure;
+import hudson.model.Messages;
 import jenkins.model.Jenkins;
 import hudson.util.FormValidation;
+
 import org.kohsuke.stapler.QueryParameter;
 
 /**
@@ -52,11 +54,48 @@ public boolean isInstantiable() {
 
     /**
      * Makes sure that the given name is good as a axis name.
+     *
+     * Aside from {@link Jenkins#checkGoodName()} this disallows ',' and
+     * '=' as special characters used in Combination presentation.
      */
     public FormValidation doCheckName(@QueryParameter String value) {
         if(Util.fixEmpty(value)==null)
             return FormValidation.ok();
 
+        if (value.contains(",")) return FormValidation.error(
+                Messages.Hudson_UnsafeChar(',')
+        );
+
+        if (value.contains("=")) return FormValidation.error(
+                Messages.Hudson_UnsafeChar('=')
+        );
+
+        try {
+            Jenkins.checkGoodName(value);
+            return FormValidation.ok();
+        } catch (Failure e) {
+            return FormValidation.error(e.getMessage());
+        }
+    }
+
+    /**
+     * Makes sure that the given name is good as a axis name.
+     *
+     * Aside from {@link Jenkins#checkGoodName()} this disallows ',' as
+     * special character used in Combination presentation. Note it is not
+     * necessary to disallow '=' in value as everything after the first
+     * occurrence is considered to be a value.
+     *
+     * Subclasses are expected to expose own <tt>doCheck</tt> method possibly delegating to this one.
+     */
+    public FormValidation checkValue(@QueryParameter String value) {
+        if(Util.fixEmpty(value)==null)
+            return FormValidation.ok();
+
+        if (value.contains(",")) return FormValidation.error(
+                Messages.Hudson_UnsafeChar(',')
+        );
+
         try {
             Jenkins.checkGoodName(value);
             return FormValidation.ok();

diff --git a/src/main/java/hudson/matrix/MatrixProject.java b/src/main/java/hudson/matrix/MatrixProject.java
@@ -28,6 +28,7 @@
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
+
 import hudson.CopyOnWrite;
 import hudson.Extension;
 import hudson.Util;
@@ -69,6 +70,7 @@
 import hudson.util.DescribableList;
 import hudson.util.FormValidation;
 import hudson.util.FormValidation.Kind;
+
 import java.io.File;
 import java.io.FileFilter;
 import java.io.IOException;
@@ -86,11 +88,14 @@
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+
 import javax.annotation.Nullable;
 import javax.servlet.ServletException;
+
 import jenkins.model.Jenkins;
 import jenkins.scm.SCMCheckoutStrategyDescriptor;
 import net.sf.json.JSONObject;
+
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.HttpResponse;
@@ -874,10 +879,9 @@ protected void submit(StaplerRequest req, StaplerResponse rsp) throws IOExceptio
         else
             executionStrategy = req.bindJSON(esd.get(0).clazz,json.getJSONObject("executionStrategy"));
 
-        // parse system axes
         DescribableList<Axis,AxisDescriptor> newAxes = new DescribableList<Axis,AxisDescriptor>(this);
         newAxes.rebuildHetero(req, json, Axis.all(),"axis");
-        checkAxisNames(newAxes);
+        checkAxes(newAxes);
         this.axes = new AxisList(newAxes.toList());
 
         buildWrappers.rebuild(req, json, BuildWrappers.getFor(this));
@@ -895,12 +899,25 @@ public AggregatedTestResultAction getAggregatedTestResultAction() {
     /**
      * Verifies that Axis names are valid and unique.
      */
-    private void checkAxisNames(Iterable<Axis> newAxes) throws FormException {
+    private void checkAxes(Iterable<Axis> newAxes) throws FormException {
         HashSet<String> axisNames = new HashSet<String>();
         for (Axis a : newAxes) {
-            FormValidation fv = a.getDescriptor().doCheckName(a.getName());
-            if (fv.kind!=Kind.OK)
-                throw new FormException(Messages.MatrixProject_DuplicateAxisName(),fv,"axis.name");
+            final AxisDescriptor desc = a.getDescriptor();
+            FormValidation fv = desc.doCheckName(a.getName());
+            if (fv.kind!=Kind.OK) {
+                final String msg = Messages.MatrixProject_InvalidAxisName(a.getName(), fv.getMessage());
+                throw new FormException(msg,fv,"axis.name");
+            }
+
+            for (String value: a.getValues()) {
+                fv = desc.checkValue(value);
+                if (fv.kind!=Kind.OK) {
+                    final String msg = Messages.MatrixProject_InvalidAxisValue(value, fv.getMessage());
+                    // This is done on wrong place, MatrixProject is not supposed
+                    // to know field names of arbitrary axis implementations
+                    throw new FormException(msg,fv,"axis.value");
+                }
+            }
 
             if (axisNames.contains(a.getName()))
                 throw new FormException(Messages.MatrixProject_DuplicateAxisName(),"axis.name");

diff --git a/src/main/java/hudson/matrix/TextAxis.java b/src/main/java/hudson/matrix/TextAxis.java
@@ -1,7 +1,12 @@
 package hudson.matrix;
 
 import hudson.Extension;
+import hudson.util.FormValidation;
+
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
 
 import java.util.List;
 
@@ -30,5 +35,11 @@ public static class DescriptorImpl extends AxisDescriptor {
         public String getDisplayName() {
             return Messages.TextArea_DisplayName();
         }
+
+        @Restricted(NoExternalUse.class)
+        // TODO: expandableTextbox does not support form validation
+        public FormValidation doCheckValueString(@QueryParameter String value) {
+            return super.checkValue(value);
+        }
     }
 }
diff --git a/src/main/resources/hudson/matrix/Messages.properties b/src/main/resources/hudson/matrix/Messages.properties
@@ -24,6 +24,8 @@ MatrixBuild.depends_on_this={0} depends on this.
 MatrixProject.DisplayName=Multi-configuration project
 MatrixProject.Pronoun=Multi-configuration project
 MatrixProject.DuplicateAxisName=Duplicate axis name
+MatrixProject.InvalidAxisName=Matrix axis name ''{0}'' is invalid: {1}
+MatrixProject.InvalidAxisValue=Matrix axis value ''{0}'' is invalid: {1}
 
 MatrixBuild.Triggering=Triggering {0}
 MatrixBuild.AppearsCancelled={0} appears to be cancelled

diff --git a/src/test/java/hudson/matrix/AxisDescriptorTest.java b/src/test/java/hudson/matrix/AxisDescriptorTest.java
@@ -0,0 +1,70 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * 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.matrix;
+
+import static org.junit.Assert.*;
+import hudson.util.FormValidation;
+
+import org.junit.Test;
+
+public class AxisDescriptorTest {
+
+    TextAxis.DescriptorImpl descriptor = new TextAxis.DescriptorImpl();
+
+    @Test
+    public void combinationNameSpecialChars() {
+        assertEquals(
+                FormValidation.Kind.ERROR,
+                descriptor.doCheckName("a=b").kind
+        );
+
+        assertEquals(
+                FormValidation.Kind.ERROR,
+                descriptor.doCheckName("a,b").kind
+        );
+
+        assertEquals(
+                FormValidation.Kind.ERROR,
+                descriptor.doCheckName("a/b").kind
+        );
+    }
+
+    @Test
+    public void combinationValueSpecialChars() {
+        assertEquals(
+                FormValidation.Kind.OK,
+                descriptor.checkValue("a=b").kind
+        );
+
+        assertEquals(
+                FormValidation.Kind.ERROR,
+                descriptor.checkValue("a,b").kind
+        );
+
+        assertEquals(
+                FormValidation.Kind.ERROR,
+                descriptor.checkValue("a/b").kind
+        );
+    }
+}