diff --git a/docs/changelog/100872.yaml b/docs/changelog/100872.yaml new file mode 100644 index 0000000000000..9877afa28982e --- /dev/null +++ b/docs/changelog/100872.yaml @@ -0,0 +1,5 @@ +pr: 100872 +summary: Improve painless error wrapping +area: Infra/Scripting +type: bug +issues: [] diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ErrorCauseWrapper.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ErrorCauseWrapper.java new file mode 100644 index 0000000000000..29cc98d1405e4 --- /dev/null +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ErrorCauseWrapper.java @@ -0,0 +1,50 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.painless; + +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.List; + +/** + * A wrapper for Error which hides the underlying Error from the exception cause chain. + *

+ * Only errors which should be sandboxed and not cause the node to crash are wrapped. + */ +class ErrorCauseWrapper extends ElasticsearchException { + + private static final List> wrappedErrors = org.elasticsearch.core.List.of( + PainlessError.class, + OutOfMemoryError.class, + StackOverflowError.class, + LinkageError.class + ); + + final Throwable realCause; + + private ErrorCauseWrapper(Throwable realCause) { + super(realCause.getMessage()); + this.realCause = realCause; + } + + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.field("type", getExceptionName(realCause)); + builder.field("reason", realCause.getMessage()); + return builder; + } + + static Throwable maybeWrap(Throwable t) { + if (wrappedErrors.contains(t.getClass())) { + return new ErrorCauseWrapper(t); + } + return t; + } +} diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScript.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScript.java index 2bd9acf5bae0c..c56cd397ecf07 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScript.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScript.java @@ -82,7 +82,15 @@ default ScriptException convertToScriptException(Throwable t, Map> entry : extraMetadata.entrySet()) { scriptException.addMetadata(entry.getKey(), entry.getValue()); } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java index 27e02a4a7c30c..a40b096894db8 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java @@ -492,7 +492,8 @@ private ScriptException convertToScriptException(String scriptSource, Throwable break; } } - throw new ScriptException("compile error", t, scriptStack, scriptSource, PainlessScriptEngine.NAME, pos); + Throwable cause = ErrorCauseWrapper.maybeWrap(t); + throw new ScriptException("compile error", cause, scriptStack, scriptSource, PainlessScriptEngine.NAME, pos); } // very simple heuristic: +/- 25 chars. can be improved later. diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java index 26dcbd6533b47..7f2dd914843f0 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java @@ -671,8 +671,8 @@ public void testNoLoopCounterInForEach() { int total = (int) exec("int total = 0; for (int value : params['values']) total += value; return total", params, false); assertEquals(total, 20000000); - PainlessError pe = expectScriptThrows( - PainlessError.class, + ErrorCauseWrapper pe = expectScriptThrows( + ErrorCauseWrapper.class, () -> exec( "int total = 0; for (int value = 0; value < params['values'].length; ++value) total += value; return total", params, diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/ErrorCauseWrapperTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/ErrorCauseWrapperTests.java new file mode 100644 index 0000000000000..1c3a49491954d --- /dev/null +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/ErrorCauseWrapperTests.java @@ -0,0 +1,77 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.painless; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xcontent.ToXContent; +import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.XContentFactory; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.json.JsonXContent; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.util.Map; + +import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; + +public class ErrorCauseWrapperTests extends ESTestCase { + + ErrorCauseWrapper assertWraps(Error realError) { + Throwable e = ErrorCauseWrapper.maybeWrap(realError); + assertThat(e.getCause(), nullValue()); + assertThat(e, instanceOf(ErrorCauseWrapper.class)); + ErrorCauseWrapper wrapper = (ErrorCauseWrapper) e; + assertThat(wrapper.realCause, is(realError)); + return wrapper; + } + + public void testOutOfMemoryError() { + assertWraps(new OutOfMemoryError("oom")); + } + + public void testStackOverflowError() { + assertWraps(new StackOverflowError("soe")); + } + + public void testLinkageError() { + assertWraps(new LinkageError("le")); + } + + public void testPainlessError() { + assertWraps(new PainlessError("pe")); + } + + public void testNotWrapped() { + AssertionError realError = new AssertionError("not wrapped"); + Throwable e = ErrorCauseWrapper.maybeWrap(realError); + assertThat(e, is(realError)); + } + + public void testXContent() throws IOException { + ErrorCauseWrapper e = assertWraps(new PainlessError("some error")); + + ByteArrayOutputStream output = new ByteArrayOutputStream(); + XContentBuilder builder = XContentFactory.jsonBuilder(output); + builder.startObject(); + e.toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + builder.flush(); + + try (XContentParser parser = createParser(JsonXContent.jsonXContent, output.toByteArray())) { + Map content = parser.mapStrings(); + assertThat(content, hasEntry("type", "painless_error")); + assertThat(content, hasEntry("reason", "some error")); + } + + } +} diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionTests.java index 23af1adb93124..6a4ec00d340a5 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionTests.java @@ -9,6 +9,7 @@ package org.elasticsearch.painless; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; public class FunctionTests extends ScriptTestCase { @@ -75,11 +76,12 @@ public void testBadCastFromMethod() { } public void testInfiniteLoop() { - Error expected = expectScriptThrows(PainlessError.class, () -> { exec("void test() {boolean x = true; while (x) {}} test()"); }); - assertThat( - expected.getMessage(), - containsString("The maximum number of statements that can be executed in a loop has been reached.") + ErrorCauseWrapper e = expectScriptThrows( + ErrorCauseWrapper.class, + () -> { exec("void test() {boolean x = true; while (x) {}} test()"); } ); + assertThat(e.realCause.getClass(), equalTo(PainlessError.class)); + assertThat(e.getMessage(), containsString("The maximum number of statements that can be executed in a loop has been reached.")); } public void testReturnVoid() { diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/WhenThingsGoWrongTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/WhenThingsGoWrongTests.java index e68e2f6ba15cd..f709649d0e338 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/WhenThingsGoWrongTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/WhenThingsGoWrongTests.java @@ -18,6 +18,7 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; public class WhenThingsGoWrongTests extends ScriptTestCase { @@ -121,34 +122,34 @@ public void testBogusParameter() { } public void testInfiniteLoops() { - PainlessError expected = expectScriptThrows(PainlessError.class, () -> { exec("boolean x = true; while (x) {}"); }); + ErrorCauseWrapper expected = expectScriptThrows(ErrorCauseWrapper.class, () -> { exec("boolean x = true; while (x) {}"); }); assertTrue(expected.getMessage().contains("The maximum number of statements that can be executed in a loop has been reached.")); - expected = expectScriptThrows(PainlessError.class, () -> { exec("while (true) {int y = 5;}"); }); + expected = expectScriptThrows(ErrorCauseWrapper.class, () -> { exec("while (true) {int y = 5;}"); }); assertTrue(expected.getMessage().contains("The maximum number of statements that can be executed in a loop has been reached.")); - expected = expectScriptThrows(PainlessError.class, () -> { exec("while (true) { boolean x = true; while (x) {} }"); }); + expected = expectScriptThrows(ErrorCauseWrapper.class, () -> { exec("while (true) { boolean x = true; while (x) {} }"); }); assertTrue(expected.getMessage().contains("The maximum number of statements that can be executed in a loop has been reached.")); - expected = expectScriptThrows(PainlessError.class, () -> { + expected = expectScriptThrows(ErrorCauseWrapper.class, () -> { exec("while (true) { boolean x = false; while (x) {} }"); fail("should have hit PainlessError"); }); assertTrue(expected.getMessage().contains("The maximum number of statements that can be executed in a loop has been reached.")); - expected = expectScriptThrows(PainlessError.class, () -> { + expected = expectScriptThrows(ErrorCauseWrapper.class, () -> { exec("boolean x = true; for (;x;) {}"); fail("should have hit PainlessError"); }); assertTrue(expected.getMessage().contains("The maximum number of statements that can be executed in a loop has been reached.")); - expected = expectScriptThrows(PainlessError.class, () -> { + expected = expectScriptThrows(ErrorCauseWrapper.class, () -> { exec("for (;;) {int x = 5;}"); fail("should have hit PainlessError"); }); assertTrue(expected.getMessage().contains("The maximum number of statements that can be executed in a loop has been reached.")); - expected = expectScriptThrows(PainlessError.class, () -> { + expected = expectScriptThrows(ErrorCauseWrapper.class, () -> { exec("def x = true; do {int y = 5;} while (x)"); fail("should have hit PainlessError"); }); @@ -165,7 +166,7 @@ public void testLoopLimits() { // right below limit: ok exec("for (int x = 0; x < 999999; ++x) {}"); - PainlessError expected = expectScriptThrows(PainlessError.class, () -> { exec("for (int x = 0; x < 1000000; ++x) {}"); }); + ErrorCauseWrapper expected = expectScriptThrows(ErrorCauseWrapper.class, () -> { exec("for (int x = 0; x < 1000000; ++x) {}"); }); assertTrue(expected.getMessage().contains("The maximum number of statements that can be executed in a loop has been reached.")); } @@ -211,11 +212,16 @@ public void testBadBoxingCast() { public void testOutOfMemoryError() { assumeTrue("test only happens to work for sure on oracle jre", Constants.JAVA_VENDOR.startsWith("Oracle")); - expectScriptThrows(OutOfMemoryError.class, () -> { exec("int[] x = new int[Integer.MAX_VALUE - 1];"); }); + ErrorCauseWrapper e = expectScriptThrows(ErrorCauseWrapper.class, () -> { exec("int[] x = new int[Integer.MAX_VALUE - 1];"); }); + assertThat(e.realCause.getClass(), equalTo(OutOfMemoryError.class)); } public void testStackOverflowError() { - expectScriptThrows(StackOverflowError.class, () -> { exec("void recurse(int x, int y) {recurse(x, y)} recurse(1, 2);"); }); + ErrorCauseWrapper e = expectScriptThrows( + ErrorCauseWrapper.class, + () -> { exec("void recurse(int x, int y) {recurse(x, y)} recurse(1, 2);"); } + ); + assertThat(e.realCause.getClass(), equalTo(StackOverflowError.class)); } public void testCanNotOverrideRegexEnabled() {