Skip to content
Permalink
Browse files
[FIXED JENKINS-42551] Reject mismatched quotes and other invalid Groovy
So now the JSON string "hello\\'" will cause a parse-time error. I
also tweaked JSON validation endpoint to also convert to Jenkinsfile
and validate *that*, so as to be sure we've actually got JSON that can
be a valid Jenkinsfile, even if it has some kind of gotcha in it like
the one fixed here.
  • Loading branch information
abayer committed Mar 10, 2017
1 parent 2b20c58 commit c8d12fa9fd397d9443f4ea40e9d77035b2bd295f
@@ -23,6 +23,7 @@
*/
package org.jenkinsci.plugins.pipeline.modeldefinition.parser

import org.apache.commons.lang.StringEscapeUtils
import org.jenkinsci.plugins.pipeline.modeldefinition.shaded.com.fasterxml.jackson.databind.JsonNode
import com.github.fge.jsonschema.exceptions.JsonReferenceException
import com.github.fge.jsonschema.exceptions.ProcessingException
@@ -419,19 +420,26 @@ class JSONParser implements Parser {
}

public @CheckForNull ModelASTValue parseValue(JsonTree o) {
ModelASTValue val = null
if (o.node.get("isLiteral").asBoolean()) {
if (o.node.get("value").isBoolean()) {
return ModelASTValue.fromConstant(o.node.get("value").booleanValue(), o)
val = ModelASTValue.fromConstant(o.node.get("value").booleanValue(), o)
} else if (o.node.get("value").isNumber()) {
return ModelASTValue.fromConstant(o.node.get("value").numberValue(), o)
val = ModelASTValue.fromConstant(o.node.get("value").numberValue(), o)
} else if (o.node.get("value").isTextual()) {
return ModelASTValue.fromConstant(o.node.get("value").textValue(), o)
val = ModelASTValue.fromConstant(o.node.get("value").textValue(), o)
} else {
return ModelASTValue.fromConstant(o.node.get("value").textValue(), o)
val = ModelASTValue.fromConstant(o.node.get("value").textValue(), o)
}
} else {
return ModelASTValue.fromGString(o.node.get("value").textValue(), o)
val = ModelASTValue.fromGString(o.node.get("value").textValue(), o)
}

String valGroovy = val?.toGroovy()
if (valGroovy != null && valGroovy != StringEscapeUtils.escapeJava(valGroovy)) {
errorCollector.error(val, Messages.JSONParser_MismatchedQuotes())
}
return val;
}

public @CheckForNull ModelASTScriptBlock parseScriptBlock(JsonTree j) {
@@ -266,9 +266,21 @@ public HttpResponse doValidateJson(StaplerRequest req) {

JSONParser parser = new JSONParser(new SimpleJsonTree(json));

parser.parse();
ModelASTPipelineDef pipelineDef = parser.parse();

if (!collectErrors(result, parser.getErrorCollector())) {
if (pipelineDef != null) {
try {
Converter.scriptToPipelineDef(pipelineDef.toPrettyGroovy());
} catch (Exception e) {
JSONObject jfErrors = new JSONObject();
reportFailure(jfErrors, e);
JSONArray errors = new JSONArray();
errors.add(new JSONObject().accumulate("jenkinsfileErrors", jfErrors));
reportFailure(result, errors);
}
}

if (!collectErrors(result, parser.getErrorCollector()) && result.isEmpty()) {
result.accumulate("result", "success");
}
} catch (Exception je) {
@@ -90,6 +90,7 @@ JSONParser.MissingRequiredProperties=Missing one or more required properties: {0
JSONParser.ObjNotJSON=Object {0} is neither a JSON array nor a JSON object
JSONParser.TooFewItems=Array has {0} entries, requires minimum of {1}
JSONParser.MissingPipelineRoot=No pipeline block or invalid pipeline block content
JSONParser.MismatchedQuotes=Contains an uneven number of single or double quotes. Quotes need to be used in pairs.

ModelValidatorImpl.EmptySection={0} can not be empty
ModelValidatorImpl.DuplicateBuildCondition=Duplicate build condition name: "{0}"
@@ -86,4 +86,10 @@ public void invalidIdentifierInEnv() throws Exception {
public void jsonParameterTypeCoercion() throws Exception {
findErrorInJSON(Messages.ModelValidatorImpl_InvalidParameterType(int.class, "time", "5", String.class), "jsonParameterTypeCoercion");
}

@Issue("JENKINS-42551")
@Test
public void jsonMismatchedQuotes() throws Exception {
findErrorInJSON(Messages.JSONParser_MismatchedQuotes(), "jsonMismatchedQuotes");
}
}
@@ -0,0 +1,38 @@
{"pipeline": {
"stages": [ {
"name": "foo",
"branches": [ {
"name": "default",
"steps": [ {
"name": "echo",
"arguments": [ {
"key": "message",
"value": {
"isLiteral": true,
"value": "hello\\'"
}
}]
}]
}]
}],
"agent": {"type": "none"},
"options": {"options": [ {
"name": "timeout",
"arguments": [
{
"key": "time",
"value": {
"isLiteral": true,
"value": "5"
}
},
{
"key": "unit",
"value": {
"isLiteral": true,
"value": "MINUTES"
}
}
]
}]}
}}

0 comments on commit c8d12fa

Please sign in to comment.