From 1319b402916e8895ac99d61003e27ffe37c46e03 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 1dcbce037b264..fc2a10891f623 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 BindingTest(int state0, int state1) { 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 7adc816252059..a64814f866113 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 void addPainlessBinding(Class targetClass, String methodName, Class 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 b74720b2d61f2..853a48c918e20 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 @@ -181,5 +181,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 c6d4e1974c14b..4bcc557d3dcff 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 @@ 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 void testRepeatedBinding() { } 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);