diff --git a/docs/painless/painless-debugging.asciidoc b/docs/painless/painless-debugging.asciidoc index a909593ff17..8523116616d 100644 --- a/docs/painless/painless-debugging.asciidoc +++ b/docs/painless/painless-debugging.asciidoc @@ -48,7 +48,7 @@ Which shows that the class of `doc.first` is "java_class": "org.elasticsearch.index.fielddata.ScriptDocValues$Longs", ... }, - "status": 500 + "status": 400 } --------------------------------------------------------- // TESTRESPONSE[s/\.\.\./"script_stack": $body.error.script_stack, "script": $body.error.script, "lang": $body.error.lang, "caused_by": $body.error.caused_by, "root_cause": $body.error.root_cause, "reason": $body.error.reason/] diff --git a/docs/reference/migration/migrate_7_0/scripting.asciidoc b/docs/reference/migration/migrate_7_0/scripting.asciidoc index df43aaa92ea..79380f84204 100644 --- a/docs/reference/migration/migrate_7_0/scripting.asciidoc +++ b/docs/reference/migration/migrate_7_0/scripting.asciidoc @@ -11,3 +11,9 @@ the getter methods for date objects were deprecated. These methods have now been removed. Instead, use `.value` on `date` fields, or explicitly parse `long` fields into a date object using `Instance.ofEpochMillis(doc["myfield"].value)`. + +==== Script errors will return as `400` error codes + +Malformed scripts, either in search templates, ingest pipelines or search +requests, return `400 - Bad request` while they would previously return +`500 - Internal Server Error`. This also applies for stored scripts. diff --git a/docs/reference/migration/migrate_7_0/search.asciidoc b/docs/reference/migration/migrate_7_0/search.asciidoc index 1fe0bc62418..7505b6f14d1 100644 --- a/docs/reference/migration/migrate_7_0/search.asciidoc +++ b/docs/reference/migration/migrate_7_0/search.asciidoc @@ -43,7 +43,7 @@ The Search API returns `400 - Bad request` while it would previously return * the number of slices is too large * keep alive for scroll is too large * number of filters in the adjacency matrix aggregation is too large - +* script compilation errors ==== Scroll queries cannot use the `request_cache` anymore diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java index 5a0b2e15460..b739fc1cfd0 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java @@ -19,9 +19,9 @@ package org.elasticsearch.script.mustache; import com.github.mustachejava.Mustache; +import com.github.mustachejava.MustacheException; import com.github.mustachejava.MustacheFactory; -import java.io.StringReader; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; @@ -31,12 +31,15 @@ import org.elasticsearch.script.GeneralScriptException; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptEngine; +import org.elasticsearch.script.ScriptException; import org.elasticsearch.script.TemplateScript; import java.io.Reader; +import java.io.StringReader; import java.io.StringWriter; import java.security.AccessController; import java.security.PrivilegedAction; +import java.util.Collections; import java.util.Map; /** @@ -66,9 +69,14 @@ public final class MustacheScriptEngine implements ScriptEngine { } final MustacheFactory factory = createMustacheFactory(options); Reader reader = new StringReader(templateSource); - Mustache template = factory.compile(reader, "query-template"); - TemplateScript.Factory compiled = params -> new MustacheExecutableScript(template, params); - return context.factoryClazz.cast(compiled); + try { + Mustache template = factory.compile(reader, "query-template"); + TemplateScript.Factory compiled = params -> new MustacheExecutableScript(template, params); + return context.factoryClazz.cast(compiled); + } catch (MustacheException ex) { + throw new ScriptException(ex.getMessage(), ex, Collections.emptyList(), templateSource, NAME); + } + } private CustomMustacheFactory createMustacheFactory(Map options) { diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheTests.java b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheTests.java index ba59e9ccac0..354d9250090 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheTests.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheTests.java @@ -18,6 +18,15 @@ */ package org.elasticsearch.script.mustache; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.script.ScriptEngine; +import org.elasticsearch.script.ScriptException; +import org.elasticsearch.script.TemplateScript; +import org.elasticsearch.test.ESTestCase; +import org.hamcrest.Matcher; + import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.Arrays; @@ -29,15 +38,6 @@ import java.util.Locale; import java.util.Map; import java.util.Set; -import com.github.mustachejava.MustacheException; -import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentHelper; -import org.elasticsearch.script.ScriptEngine; -import org.elasticsearch.script.TemplateScript; -import org.elasticsearch.test.ESTestCase; -import org.hamcrest.Matcher; - import static java.util.Collections.singleton; import static java.util.Collections.singletonMap; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -225,11 +225,17 @@ public class MustacheTests extends ESTestCase { } public void testsUnsupportedTagsToJson() { - MustacheException e = expectThrows(MustacheException.class, () -> compile("{{#toJson}}{{foo}}{{bar}}{{/toJson}}")); + final String script = "{{#toJson}}{{foo}}{{bar}}{{/toJson}}"; + ScriptException e = expectThrows(ScriptException.class, () -> compile(script)); assertThat(e.getMessage(), containsString("Mustache function [toJson] must contain one and only one identifier")); + assertEquals(MustacheScriptEngine.NAME, e.getLang()); + assertEquals(script, e.getScript()); - e = expectThrows(MustacheException.class, () -> compile("{{#toJson}}{{/toJson}}")); + final String script2 = "{{#toJson}}{{/toJson}}"; + e = expectThrows(ScriptException.class, () -> compile(script2)); assertThat(e.getMessage(), containsString("Mustache function [toJson] must contain one and only one identifier")); + assertEquals(MustacheScriptEngine.NAME, e.getLang()); + assertEquals(script2, e.getScript()); } public void testEmbeddedToJSON() throws Exception { @@ -312,11 +318,17 @@ public class MustacheTests extends ESTestCase { } public void testsUnsupportedTagsJoin() { - MustacheException e = expectThrows(MustacheException.class, () -> compile("{{#join}}{{/join}}")); + final String script = "{{#join}}{{/join}}"; + ScriptException e = expectThrows(ScriptException.class, () -> compile(script)); assertThat(e.getMessage(), containsString("Mustache function [join] must contain one and only one identifier")); + assertEquals(MustacheScriptEngine.NAME, e.getLang()); + assertEquals(script, e.getScript()); - e = expectThrows(MustacheException.class, () -> compile("{{#join delimiter='a'}}{{/join delimiter='b'}}")); + final String script2 = "{{#join delimiter='a'}}{{/join delimiter='b'}}"; + e = expectThrows(ScriptException.class, () -> compile(script2)); assertThat(e.getMessage(), containsString("Mismatched start/end tags")); + assertEquals(MustacheScriptEngine.NAME, e.getLang()); + assertEquals(script2, e.getScript()); } public void testJoinWithCustomDelimiter() { diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/16_update2.yml b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/16_update2.yml index 3d6f603d4d8..253676bda8e 100644 --- a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/16_update2.yml +++ b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/16_update2.yml @@ -35,7 +35,7 @@ id: "non_existing" - do: - catch: request + catch: bad_request put_script: id: "1" context: "search" diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/20_scriptfield.yml b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/20_scriptfield.yml index e498a173757..02c17ce0e37 100644 --- a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/20_scriptfield.yml +++ b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/20_scriptfield.yml @@ -133,7 +133,7 @@ setup: --- "Scripted Field with script error": - do: - catch: request + catch: bad_request search: body: script_fields: diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml index 70e78f7e36b..89135449d69 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/35_search_failures.yml @@ -17,7 +17,7 @@ indices.refresh: {} - do: - catch: request + catch: bad_request reindex: body: source: diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/85_scripting.yml b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/85_scripting.yml index 617a46dfa66..901f24f022c 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/reindex/85_scripting.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/reindex/85_scripting.yml @@ -446,7 +446,7 @@ indices.refresh: {} - do: - catch: request + catch: bad_request reindex: refresh: true body: diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/35_search_failure.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/35_search_failure.yml index 17f422453ce..e7f3a146480 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/35_search_failure.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/35_search_failure.yml @@ -17,7 +17,7 @@ indices.refresh: {} - do: - catch: request + catch: bad_request update_by_query: index: source body: diff --git a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/80_scripting.yml b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/80_scripting.yml index 8ed94347923..1a3880f3d15 100644 --- a/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/80_scripting.yml +++ b/modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/80_scripting.yml @@ -434,7 +434,7 @@ indices.refresh: {} - do: - catch: request + catch: bad_request update_by_query: index: twitter refresh: true diff --git a/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/10_pipeline_with_mustache_templates.yml b/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/10_pipeline_with_mustache_templates.yml index 0e54ff0b7ad..6bc6219bfe5 100644 --- a/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/10_pipeline_with_mustache_templates.yml +++ b/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/10_pipeline_with_mustache_templates.yml @@ -332,7 +332,7 @@ wait_for_status: green - do: - catch: request + catch: bad_request ingest.put_pipeline: id: "my_pipeline_1" body: > @@ -348,5 +348,5 @@ ] } - match: { error.header.processor_type: "set" } - - match: { error.type: "general_script_exception" } - - match: { error.reason: "Failed to compile inline script [{{#join}}{{/join}}] using lang [mustache]" } + - match: { error.type: "script_exception" } + - match: { error.reason: "Mustache function [join] must contain one and only one identifier" } diff --git a/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/50_script_processor_using_painless.yml b/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/50_script_processor_using_painless.yml index 8c6a94b4a5c..1c027adcc80 100644 --- a/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/50_script_processor_using_painless.yml +++ b/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/50_script_processor_using_painless.yml @@ -89,7 +89,7 @@ --- "Test script processor with syntax error in inline script": - do: - catch: request + catch: bad_request ingest.put_pipeline: id: "my_pipeline" body: > diff --git a/server/src/main/java/org/elasticsearch/script/ScriptException.java b/server/src/main/java/org/elasticsearch/script/ScriptException.java index 726f2186108..2d5d6841618 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptException.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptException.java @@ -1,5 +1,14 @@ package org.elasticsearch.script; +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.rest.RestStatus; + /* * Licensed to Elasticsearch under one or more contributor * license agreements. See the NOTICE file distributed with @@ -25,14 +34,6 @@ import java.util.Collections; import java.util.List; import java.util.Objects; -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; - /** * Exception from a scripting engine. *

@@ -132,4 +133,9 @@ public class ScriptException extends ElasticsearchException { throw new RuntimeException(e); } } + + @Override + public RestStatus status() { + return RestStatus.BAD_REQUEST; + } } diff --git a/x-pack/qa/smoke-test-watcher/src/test/resources/rest-api-spec/test/painless/40_exception.yml b/x-pack/qa/smoke-test-watcher/src/test/resources/rest-api-spec/test/painless/40_exception.yml index a794316e97d..74a7d6eefb0 100644 --- a/x-pack/qa/smoke-test-watcher/src/test/resources/rest-api-spec/test/painless/40_exception.yml +++ b/x-pack/qa/smoke-test-watcher/src/test/resources/rest-api-spec/test/painless/40_exception.yml @@ -5,7 +5,7 @@ wait_for_status: green - do: - catch: request + catch: bad_request xpack.watcher.put_watch: id: "my_exe_watch" body: > @@ -33,7 +33,7 @@ } - is_true: error.script_stack - - match: { status: 500 } + - match: { status: 400 } --- "Test painless exceptions are returned when logging a broken response":