Skip to content

Commit

Permalink
Painless: Fix Bindings Bug (#33274)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jdconrad committed Aug 30, 2018
1 parent 61e0ce7 commit 1319b40
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Object> params = new HashMap<>();
ExecutableScript.Factory factory = scriptEngine.compile(null, script, ExecutableScript.CONTEXT, Collections.emptyMap());
ExecutableScript executableScript = factory.newInstance(params);
Expand All @@ -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<String, Object> params = new HashMap<>();
ExecutableScript.Factory factory = scriptEngine.compile(null, script, ExecutableScript.CONTEXT, Collections.emptyMap());
ExecutableScript executableScript = factory.newInstance(params);
Expand Down

0 comments on commit 1319b40

Please sign in to comment.