Improve painless error wrapping (#100872) (#102394)

* Improve painless error wrapping (#100872)

Painless sandboxes some errors from Java for which it can recover. These
errors are wrapped within a ScriptException. However, retaining the
error as a cause can be confusing when walking the error chain. This
commit wraps the error so that the real error type does not appear,
but maintains the same error message in xcontent serialized form.

* fix compile

Author: rjernst

docs/changelog/100872.yaml

@@ -0,0 +1,5 @@
+pr: 100872
+summary: Improve painless error wrapping
+area: Infra/Scripting
+type: bug
+issues: []

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.
+ * <p>
+ * Only errors which should be sandboxed and not cause the node to crash are wrapped.
+ */
+class ErrorCauseWrapper extends ElasticsearchException {
+
+    private static final List<Class<? extends Error>> 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;
+    }
+}

modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScript.java

@@ -82,7 +82,15 @@ default ScriptException convertToScriptException(Throwable t, Map<String, List<S
                 scriptStack.add(element.toString());
             }
         }
-        ScriptException scriptException = new ScriptException("runtime error", t, scriptStack, getName(), PainlessScriptEngine.NAME, pos);
+        Throwable cause = ErrorCauseWrapper.maybeWrap(t);
+        ScriptException scriptException = new ScriptException(
+            "runtime error",
+            cause,
+            scriptStack,
+            getName(),
+            PainlessScriptEngine.NAME,
+            pos
+        );
         for (Map.Entry<String, List<String>> entry : extraMetadata.entrySet()) {
             scriptException.addMetadata(entry.getKey(), entry.getValue());
         }

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.

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,

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<String, String> content = parser.mapStrings();
+            assertThat(content, hasEntry("type", "painless_error"));
+            assertThat(content, hasEntry("reason", "some error"));
+        }
+
+    }
+}

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() {

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() {