Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change ScriptException status to 400 (bad request) #30861

Merged
merged 8 commits into from
May 30, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/painless/painless-debugging.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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/]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,12 +31,15 @@
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;

/**
Expand Down Expand Up @@ -66,9 +69,14 @@ public <T> T compile(String templateName, String templateSource, ScriptContext<T
}
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<String, String> options) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,15 +38,6 @@
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;
Expand Down Expand Up @@ -225,11 +225,17 @@ public void testSimpleListToJSON() throws Exception {
}

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 {
Expand Down Expand Up @@ -312,11 +318,17 @@ public void testJoinWithToJson() {
}

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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
id: "non_existing"

- do:
catch: request
catch: bad_request
put_script:
id: "1"
context: "search"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ setup:
---
"Scripted Field with script error":
- do:
catch: request
catch: bad_request
search:
body:
script_fields:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
indices.refresh: {}

- do:
catch: request
catch: bad_request
reindex:
body:
source:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@
indices.refresh: {}

- do:
catch: request
catch: bad_request
reindex:
refresh: true
body:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
indices.refresh: {}

- do:
catch: request
catch: bad_request
update_by_query:
index: source
body:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@
indices.refresh: {}

- do:
catch: request
catch: bad_request
update_by_query:
index: twitter
refresh: true
Expand Down
22 changes: 14 additions & 8 deletions server/src/main/java/org/elasticsearch/script/ScriptException.java
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -25,14 +34,6 @@
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.
* <p>
Expand Down Expand Up @@ -132,4 +133,9 @@ public String toJsonString() {
throw new RuntimeException(e);
}
}

@Override
public RestStatus status() {
return RestStatus.BAD_REQUEST;
}
}