Check that scripts produce correct json in render template action (#1…

…01518)

If a mustache script that outputs badly-formed json is referred to in a render
template request, then the error returned will be a 500 server error, rather than
a 400 json parsing error. This is because rendering templates skips json parsing,
and so the error ends up being caught in the REST layer instead.

This commit changes the template rendering logic to always parse the output of
the script, catching json errors higher in the stack and allowing us to return
the correct status code. This also means that errors are correctly detected and
returned as part of multi search template requests.

Fixes #101477

docs/changelog/101518.yaml

@@ -0,0 +1,6 @@
+pr: 101518
+summary: Check that scripts produce correct json in render template action
+area: Search
+type: bug
+issues:
+ - 101477

docs/reference/search/search-your-data/search-template.asciidoc

@@ -795,7 +795,7 @@ PUT _scripts/my-search-template
     "lang": "mustache",
     "source": {
       "query":{
-        "multi-match":{
+        "multi_match":{
           "query": "{{query_string}}",
           "fields": """[{{#text_fields}}{{user_name}},{{/text_fields}}]"""
         }
@@ -834,7 +834,7 @@ When rendered, template outputs:
 {
   "template_output": {
     "query": {
-      "multi-match": {
+      "multi_match": {
         "query": "My string",
         "fields": "[John,kimchy,]"
       }
@@ -855,7 +855,7 @@ PUT _scripts/my-search-template
     "lang": "mustache",
     "source": {
       "query":{
-        "multi-match":{
+        "multi_match":{
           "query": "{{query_string}}",
           "fields": """[{{#text_fields}}{{user_name}}{{^last}},{{/last}}{{/text_fields}}]"""
         }
@@ -895,7 +895,7 @@ When rendered the template outputs:
 {
   "template_output": {
     "query": {
-      "multi-match": {
+      "multi_match": {
         "query": "My string",
         "fields": "[John,kimchy]"
       }

modules/lang-mustache/src/internalClusterTest/java/org/elasticsearch/script/mustache/MultiSearchTemplateIT.java

@@ -21,6 +21,7 @@
 import org.elasticsearch.search.FailBeforeCurrentVersionQueryBuilder;
 import org.elasticsearch.search.SearchService;
 import org.elasticsearch.test.ESIntegTestCase;
+import org.elasticsearch.xcontent.XContentParseException;
 import org.elasticsearch.xcontent.json.JsonXContent;
 
 import java.util.Arrays;
@@ -175,10 +176,9 @@ public void testBasic() throws Exception {
         assertThat(response4.getFailure().getMessage(), equalTo("no such index [unknown]"));
 
         MultiSearchTemplateResponse.Item response5 = response.getResponses()[4];
-        assertThat(response5.isFailure(), is(false));
-        SearchTemplateResponse searchTemplateResponse5 = response5.getResponse();
-        assertThat(searchTemplateResponse5.hasResponse(), is(false));
-        assertThat(searchTemplateResponse5.getSource().utf8ToString(), equalTo("{\"query\":{\"terms\":{\"group\":[1,2,3,]}}}"));
+        assertThat(response5.isFailure(), is(true));
+        assertNull(response5.getResponse());
+        assertThat(response5.getFailure(), instanceOf(XContentParseException.class));
     }
 
     /**

modules/lang-mustache/src/internalClusterTest/java/org/elasticsearch/script/mustache/SearchTemplateIT.java

@@ -11,9 +11,11 @@
 import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptResponse;
 import org.elasticsearch.action.bulk.BulkRequestBuilder;
 import org.elasticsearch.action.search.SearchRequest;
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.search.DummyQueryParserPlugin;
 import org.elasticsearch.search.SearchService;
@@ -189,6 +191,69 @@ public void testIndexedTemplateClient() throws Exception {
         assertNull(getResponse.getSource());
     }
 
+    public void testBadTemplate() {
+
+        // This template will produce badly formed json if given a multi-valued `text_fields` parameter,
+        // as it does not add commas between the entries. We test that it produces a 400 json parsing
+        // error both when used directly and when used in a render template request.
+
+        String script = """
+            {
+              "script": {
+                "lang": "mustache",
+                "source": ""\"
+                {
+                  "query": {
+                    "multi_match": {
+                      "query": "{{query_string}}",
+                      "fields": [{{#text_fields}}"{{name}}^{{boost}}"{{/text_fields}}]
+                    }
+                  },
+                  "from": "{{from}}",
+                  "size": "{{size}}"
+                }
+                ""\",
+                "params": {
+                  "query_string": "My query string"
+                }
+              }
+            }""";
+
+        Map<String, Object> params = Map.of(
+            "text_fields",
+            List.of(Map.of("name", "title", "boost", 10), Map.of("name", "description", "boost", 2)),
+            "from",
+            0,
+            "size",
+            0
+        );
+
+        {
+            ParsingException e = expectThrows(ParsingException.class, () -> {
+                new SearchTemplateRequestBuilder(client()).setRequest(new SearchRequest())
+                    .setScript(script)
+                    .setScriptParams(params)
+                    .setScriptType(ScriptType.INLINE)
+                    .get();
+            });
+            assertThat(e.getMessage(), containsString("Unknown key"));
+            assertThat(e.status(), equalTo(RestStatus.BAD_REQUEST));
+        }
+
+        {
+            ParsingException e = expectThrows(ParsingException.class, () -> {
+                new SearchTemplateRequestBuilder(client()).setRequest(new SearchRequest())
+                    .setScript(script)
+                    .setScriptParams(params)
+                    .setScriptType(ScriptType.INLINE)
+                    .setSimulate(true)
+                    .get();
+            });
+            assertThat(e.getMessage(), containsString("Unknown key"));
+            assertThat(e.status(), equalTo(RestStatus.BAD_REQUEST));
+        }
+    }
+
     public void testIndexedTemplate() throws Exception {
 
         String script = """

modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java

@@ -104,20 +104,22 @@ static SearchRequest convert(
         response.setSource(new BytesArray(source));
 
         SearchRequest searchRequest = searchTemplateRequest.getRequest();
-        if (searchTemplateRequest.isSimulate()) {
-            return null;
-        }
+
+        SearchSourceBuilder builder = SearchSourceBuilder.searchSource();
 
         XContentParserConfiguration parserConfig = XContentParserConfiguration.EMPTY.withRegistry(xContentRegistry)
             .withDeprecationHandler(LoggingDeprecationHandler.INSTANCE);
         try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(parserConfig, source)) {
-            SearchSourceBuilder builder = SearchSourceBuilder.searchSource();
             builder.parseXContent(parser, false, searchUsageHolder);
-            builder.explain(searchTemplateRequest.isExplain());
-            builder.profile(searchTemplateRequest.isProfile());
-            checkRestTotalHitsAsInt(searchRequest, builder);
-            searchRequest.source(builder);
         }
+
+        if (searchTemplateRequest.isSimulate()) {
+            return null;
+        }
+        builder.explain(searchTemplateRequest.isExplain());
+        builder.profile(searchTemplateRequest.isProfile());
+        checkRestTotalHitsAsInt(searchRequest, builder);
+        searchRequest.source(builder);
         return searchRequest;
     }