[FIXED JENKINS-45098] Properly validate tool-without-version

jenkinsci · Jul 6, 2017 · a4ec71a · a4ec71a
1 parent 2c0b4d6
commit a4ec71a
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 11 deletions.
diff --git a/.../src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy b/.../src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy
@@ -400,18 +400,18 @@ class ModelParser implements Parser {
                 def mc = matchMethodCall(s);
                 if (mc == null) {
                     // Not sure of a better way to deal with this - it's a full-on parse-time failure.
-                    errorCollector.error(r,Messages.ModelParser_ExpectedTool());
-                }
-
-                def toolTypeKey = parseKey(mc.method);
-
-                List<Expression> args = ((TupleExpression) mc.arguments).expressions
-                if (args.isEmpty()) {
-                    errorCollector.error(toolTypeKey, Messages.ModelParser_NoArgForTool(toolTypeKey.key))
-                } else if (args.size() > 1) {
-                    errorCollector.error(toolTypeKey, Messages.ModelParser_TooManyArgsForTool(toolTypeKey.key))
+                    errorCollector.error(r, Messages.ModelParser_ExpectedTool());
                 } else {
-                    r.tools[toolTypeKey] = parseArgument(args[0])
+                    def toolTypeKey = parseKey(mc.method);
+
+                    List<Expression> args = ((TupleExpression) mc.arguments).expressions
+                    if (args.isEmpty()) {
+                        errorCollector.error(toolTypeKey, Messages.ModelParser_NoArgForTool(toolTypeKey.key))
+                    } else if (args.size() > 1) {
+                        errorCollector.error(toolTypeKey, Messages.ModelParser_TooManyArgsForTool(toolTypeKey.key))
+                    } else {
+                        r.tools[toolTypeKey] = parseArgument(args[0])
+                    }
                 }
             }
         }

diff --git a/...efinition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java b/...efinition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java
@@ -401,6 +401,14 @@ public void notInstalledToolVersion() throws Exception {
                 .go();
     }
 
+    @Issue("JENKINS-45098")
+    @Test
+    public void toolWithoutVersion() throws Exception {
+        expectError("toolWithoutVersion")
+                .logContains(Messages.ModelParser_ExpectedTool())
+                .go();
+    }
+
     @Test
     public void globalLibraryNonStepBody() throws Exception {
         initGlobalLibrary();

diff --git a/pipeline-model-definition/src/test/resources/errors/toolWithoutVersion.groovy b/pipeline-model-definition/src/test/resources/errors/toolWithoutVersion.groovy
@@ -0,0 +1,42 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2017, CloudBees, 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.
+ */
+
+pipeline {
+    agent {
+        label "some-label"
+    }
+    tools {
+        maven
+    }
+    stages {
+        stage("foo") {
+            steps {
+                sh "mvn -version"
+            }
+        }
+    }
+}
+
+
+