From bbbb11a698ed556de14cc3fcfb365a60493519b9 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Thu, 30 Aug 2018 14:33:32 -0700 Subject: [PATCH] Painless: Fix Bindings Bug (#33274) When the change was made to the format for in the whitelist for bindings, parameters from both the constructor and the method were combined into a single list instead of separate lists. The check for method parameters was being executed from the start of the combined list rather than the correct position. The tests for bindings used a constructor and a method that only used the int types so this was not caught. The test has been changed to also use a double type and this issue is fixed. --- .../main/java/org/elasticsearch/painless/BindingTest.java | 4 ++-- .../painless/lookup/PainlessLookupBuilder.java | 2 +- .../org/elasticsearch/painless/spi/org.elasticsearch.txt | 2 +- .../test/java/org/elasticsearch/painless/BindingsTests.java | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/BindingTest.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/BindingTest.java index 1dcbce037b2..fc2a10891f6 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/BindingTest.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/BindingTest.java @@ -26,7 +26,7 @@ public class BindingTest { this.state = state0 + state1; } - public int testAddWithState(int stateless) { - return stateless + state; + public int testAddWithState(int istateless, double dstateless) { + return istateless + state + (int)dstateless; } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java index 7adc8162520..a64814f8661 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java @@ -908,7 +908,7 @@ public final class PainlessLookupBuilder { int methodTypeParametersSize = javaMethod.getParameterCount(); for (int typeParameterIndex = 0; typeParameterIndex < methodTypeParametersSize; ++typeParameterIndex) { - Class typeParameter = typeParameters.get(typeParameterIndex); + Class typeParameter = typeParameters.get(constructorTypeParametersSize + typeParameterIndex); if (isValidType(typeParameter) == false) { throw new IllegalArgumentException("type parameter [" + typeToCanonicalTypeName(typeParameter) + "] not found " + diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt index 65f50bbc383..444234384c6 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt @@ -177,5 +177,5 @@ class org.elasticsearch.painless.FeatureTest no_import { # for testing static { - int testAddWithState(int, int, int) bound_to org.elasticsearch.painless.BindingTest + int testAddWithState(int, int, int, double) bound_to org.elasticsearch.painless.BindingTest } \ No newline at end of file diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java index c6d4e1974c1..4bcc557d3dc 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java @@ -28,11 +28,11 @@ import java.util.Map; public class BindingsTests extends ScriptTestCase { public void testBasicBinding() { - assertEquals(15, exec("testAddWithState(4, 5, 6)")); + assertEquals(15, exec("testAddWithState(4, 5, 6, 0.0)")); } public void testRepeatedBinding() { - String script = "testAddWithState(4, 5, params.test)"; + String script = "testAddWithState(4, 5, params.test, 0.0)"; Map params = new HashMap<>(); ExecutableScript.Factory factory = scriptEngine.compile(null, script, ExecutableScript.CONTEXT, Collections.emptyMap()); ExecutableScript executableScript = factory.newInstance(params); @@ -48,7 +48,7 @@ public class BindingsTests extends ScriptTestCase { } public void testBoundBinding() { - String script = "testAddWithState(4, params.bound, params.test)"; + String script = "testAddWithState(4, params.bound, params.test, 0.0)"; Map params = new HashMap<>(); ExecutableScript.Factory factory = scriptEngine.compile(null, script, ExecutableScript.CONTEXT, Collections.emptyMap()); ExecutableScript executableScript = factory.newInstance(params);