Skip to content

Commit f5a5f99

Browse files
dakronedakrone
committed
Wrap groovy script exceptions in a serializable Exception object
Fixes #6598
1 parent 673194a commit f5a5f99

File tree

4 files changed

+156
-7
lines changed

4 files changed

+156
-7
lines changed

‎src/main/java/org/elasticsearch/script/groovy/GroovyScriptCompilationException.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.script.groovy;
21+
22+
import org.elasticsearch.ElasticsearchException;
23+
import org.elasticsearch.rest.RestStatus;
24+
25+
/**
26+
* Exception used to wrap groovy script compilation exceptions so they are
27+
* correctly serialized between nodes.
28+
*/
29+
public class GroovyScriptCompilationException extends ElasticsearchException {
30+
public GroovyScriptCompilationException(String message) {
31+
super(message);
32+
}
33+
34+
@Override
35+
public RestStatus status() {
36+
return RestStatus.BAD_REQUEST;
37+
}
38+
}

‎src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,11 @@
3535
import org.codehaus.groovy.control.SourceUnit;
3636
import org.codehaus.groovy.control.customizers.CompilationCustomizer;
3737
import org.codehaus.groovy.control.customizers.ImportCustomizer;
38+
import org.elasticsearch.ExceptionsHelper;
3839
import org.elasticsearch.common.Nullable;
3940
import org.elasticsearch.common.component.AbstractComponent;
4041
import org.elasticsearch.common.inject.Inject;
42+
import org.elasticsearch.common.logging.ESLogger;
4143
import org.elasticsearch.common.settings.Settings;
4244
import org.elasticsearch.script.ExecutableScript;
4345
import org.elasticsearch.script.ScriptEngineService;
@@ -106,7 +108,14 @@ public boolean sandboxed() {
106108

107109
@Override
108110
public Object compile(String script) {
109-
return loader.parseClass(script, generateScriptName());
111+
try {
112+
return loader.parseClass(script, generateScriptName());
113+
} catch (Throwable e) {
114+
if (logger.isTraceEnabled()) {
115+
logger.trace("exception compiling Groovy script:", e);
116+
}
117+
throw new GroovyScriptCompilationException(ExceptionsHelper.detailedMessage(e));
118+
}
110119
}
111120

112121
/**
@@ -129,7 +138,7 @@ public ExecutableScript executable(Object compiledScript, Map<String, Object> va
129138
if (vars != null) {
130139
allVars.putAll(vars);
131140
}
132-
return new GroovyScript(createScript(compiledScript, allVars));
141+
return new GroovyScript(createScript(compiledScript, allVars), this.logger);
133142
} catch (Exception e) {
134143
throw new ScriptException("failed to build executable script", e);
135144
}
@@ -145,7 +154,7 @@ public SearchScript search(Object compiledScript, SearchLookup lookup, @Nullable
145154
allVars.putAll(vars);
146155
}
147156
Script scriptObject = createScript(compiledScript, allVars);
148-
return new GroovyScript(scriptObject, lookup);
157+
return new GroovyScript(scriptObject, lookup, this.logger);
149158
} catch (Exception e) {
150159
throw new ScriptException("failed to build search script", e);
151160
}
@@ -180,14 +189,16 @@ public static final class GroovyScript implements ExecutableScript, SearchScript
180189
private final SearchLookup lookup;
181190
private final Map<String, Object> variables;
182191
private final UpdateableFloat score;
192+
private final ESLogger logger;
183193

184-
public GroovyScript(Script script) {
185-
this(script, null);
194+
public GroovyScript(Script script, ESLogger logger) {
195+
this(script, null, logger);
186196
}
187197

188-
public GroovyScript(Script script, SearchLookup lookup) {
198+
public GroovyScript(Script script, SearchLookup lookup, ESLogger logger) {
189199
this.script = script;
190200
this.lookup = lookup;
201+
this.logger = logger;
191202
this.variables = script.getBinding().getVariables();
192203
this.score = new UpdateableFloat(0);
193204
// Add the _score variable, which will be updated per-document by
@@ -237,7 +248,14 @@ public void setNextSource(Map<String, Object> source) {
237248

238249
@Override
239250
public Object run() {
240-
return script.run();
251+
try {
252+
return script.run();
253+
} catch (Throwable e) {
254+
if (logger.isTraceEnabled()) {
255+
logger.trace("exception running Groovy script", e);
256+
}
257+
throw new GroovyScriptExecutionException(ExceptionsHelper.detailedMessage(e));
258+
}
241259
}
242260

243261
@Override

‎src/main/java/org/elasticsearch/script/groovy/GroovyScriptExecutionException.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.script.groovy;
21+
22+
import org.elasticsearch.ElasticsearchException;
23+
import org.elasticsearch.rest.RestStatus;
24+
25+
/**
26+
* Exception used to wrap groovy script execution exceptions so they are
27+
* correctly serialized between nodes.
28+
*/
29+
public class GroovyScriptExecutionException extends ElasticsearchException {
30+
public GroovyScriptExecutionException(String message) {
31+
super(message);
32+
}
33+
34+
@Override
35+
public RestStatus status() {
36+
return RestStatus.BAD_REQUEST;
37+
}
38+
}

‎src/test/java/org/elasticsearch/script/GroovyScriptTests.java

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,20 @@
1919

2020
package org.elasticsearch.script;
2121

22+
import org.elasticsearch.ExceptionsHelper;
23+
import org.elasticsearch.action.index.IndexRequestBuilder;
24+
import org.elasticsearch.action.search.SearchPhaseExecutionException;
2225
import org.elasticsearch.action.search.SearchResponse;
2326
import org.elasticsearch.test.ElasticsearchIntegrationTest;
2427
import org.junit.Test;
2528

29+
import java.util.List;
30+
31+
import static com.google.common.collect.Lists.newArrayList;
32+
import static org.elasticsearch.index.query.FilterBuilders.scriptFilter;
33+
import static org.elasticsearch.index.query.QueryBuilders.constantScoreQuery;
2634
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
35+
import static org.hamcrest.Matchers.equalTo;
2736

2837
/**
2938
* Various tests for Groovy scripting
@@ -47,4 +56,50 @@ public void assertScript(String script) {
4756
"; 1\", \"type\": \"number\", \"lang\": \"groovy\"}}}").get();
4857
assertNoFailures(resp);
4958
}
59+
60+
@Test
61+
public void testGroovyExceptionSerialization() throws Exception {
62+
List<IndexRequestBuilder> reqs = newArrayList();
63+
for (int i = 0; i < randomIntBetween(50, 500); i++) {
64+
reqs.add(client().prepareIndex("test", "doc", "" + i).setSource("foo", "bar"));
65+
}
66+
indexRandom(true, false, reqs);
67+
try {
68+
client().prepareSearch("test").setQuery(constantScoreQuery(scriptFilter("1 == not_found").lang("groovy"))).get();
69+
fail("should have thrown an exception");
70+
} catch (SearchPhaseExecutionException e) {
71+
assertThat(ExceptionsHelper.detailedMessage(e) + "should not contained NotSerializableTransportException",
72+
ExceptionsHelper.detailedMessage(e).contains("NotSerializableTransportException"), equalTo(false));
73+
assertThat(ExceptionsHelper.detailedMessage(e) + "should have contained GroovyScriptExecutionException",
74+
ExceptionsHelper.detailedMessage(e).contains("GroovyScriptExecutionException"), equalTo(true));
75+
assertThat(ExceptionsHelper.detailedMessage(e) + "should have contained not_found",
76+
ExceptionsHelper.detailedMessage(e).contains("No such property: not_found"), equalTo(true));
77+
}
78+
79+
try {
80+
client().prepareSearch("test").setQuery(constantScoreQuery(
81+
scriptFilter("pr = Runtime.getRuntime().exec(\"touch /tmp/gotcha\"); pr.waitFor()").lang("groovy"))).get();
82+
fail("should have thrown an exception");
83+
} catch (SearchPhaseExecutionException e) {
84+
assertThat(ExceptionsHelper.detailedMessage(e) + "should not contained NotSerializableTransportException",
85+
ExceptionsHelper.detailedMessage(e).contains("NotSerializableTransportException"), equalTo(false));
86+
assertThat(ExceptionsHelper.detailedMessage(e) + "should have contained GroovyScriptCompilationException",
87+
ExceptionsHelper.detailedMessage(e).contains("GroovyScriptCompilationException"), equalTo(true));
88+
assertThat(ExceptionsHelper.detailedMessage(e) + "should have contained Method calls not allowed on [java.lang.Runtime]",
89+
ExceptionsHelper.detailedMessage(e).contains("Method calls not allowed on [java.lang.Runtime]"), equalTo(true));
90+
}
91+
92+
try {
93+
client().prepareSearch("test").setQuery(constantScoreQuery(
94+
scriptFilter("assert false").lang("groovy"))).get();
95+
fail("should have thrown an exception");
96+
} catch (SearchPhaseExecutionException e) {
97+
assertThat(ExceptionsHelper.detailedMessage(e) + "should not contained NotSerializableTransportException",
98+
ExceptionsHelper.detailedMessage(e).contains("NotSerializableTransportException"), equalTo(false));
99+
assertThat(ExceptionsHelper.detailedMessage(e) + "should have contained GroovyScriptExecutionException",
100+
ExceptionsHelper.detailedMessage(e).contains("GroovyScriptExecutionException"), equalTo(true));
101+
assertThat(ExceptionsHelper.detailedMessage(e) + "should have contained an assert error",
102+
ExceptionsHelper.detailedMessage(e).contains("PowerAssertionError[assert false"), equalTo(true));
103+
}
104+
}
50105
}

0 commit comments

Comments
 (0)