Fix compilation of expressions using instanceof and primitives

Prior to this commit the SpEL compiler would generate bad bytecode
if the left hand operand of an instanceof was a primitive or
if the right hand operand was a primitive type reference.
With the fixes primitives on the left hand side are now
correctly boxed and special handling is in place for when the
right hand side is a primitive type reference. Using a primitive
type reference on the right always causes the instanceof
check to return false.

Additionally a guard has been added such that compilation is
not allowed when the right hand side of an expression
is not a type reference. If it is, for example, a variable
reference that evaluates to a type reference then that
cannot be expressed in bytecode so compilation is not performed.

Issue: SPR-14250
This commit is contained in:
Andy Clement 2016-05-05 09:57:15 -07:00
parent 3c92ddc94b
commit a31f0bb3c0
2 changed files with 66 additions and 5 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2014 the original author or authors.
* Copyright 2002-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -53,8 +53,9 @@ public class OperatorInstanceof extends Operator {
*/
@Override
public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException {
SpelNodeImpl rightOperand = getRightOperand();
TypedValue left = getLeftOperand().getValueInternal(state);
TypedValue right = getRightOperand().getValueInternal(state);
TypedValue right = rightOperand.getValueInternal(state);
Object leftValue = left.getValue();
Object rightValue = right.getValue();
BooleanTypedValue result = null;
@ -71,7 +72,11 @@ public class OperatorInstanceof extends Operator {
result = BooleanTypedValue.forValue(rightClass.isAssignableFrom(leftValue.getClass()));
}
this.type = rightClass;
this.exitTypeDescriptor = "Z";
if (rightOperand instanceof TypeReference) {
// Can only generate bytecode where the right operand is a direct type reference,
// not if it is indirect (for example when right operand is a variable reference)
this.exitTypeDescriptor = "Z";
}
return result;
}
@ -83,7 +88,16 @@ public class OperatorInstanceof extends Operator {
@Override
public void generateCode(MethodVisitor mv, CodeFlow cf) {
getLeftOperand().generateCode(mv, cf);
mv.visitTypeInsn(INSTANCEOF,Type.getInternalName(this.type));
CodeFlow.insertBoxIfNecessary(mv, cf.lastDescriptor());
if (this.type.isPrimitive()) {
// always false - but left operand code always driven
// in case it had side effects
mv.visitInsn(POP);
mv.visitInsn(ICONST_0); // value of false
}
else {
mv.visitTypeInsn(INSTANCEOF,Type.getInternalName(this.type));
}
cf.pushDescriptor(this.exitTypeDescriptor);
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2015 the original author or authors.
* Copyright 2002-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -235,6 +235,53 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertEquals(true,expression.getValue(root));
}
@Test
public void operatorInstanceOf_SPR14250() throws Exception {
// primitive left operand - should get boxed, return true
expression = parse("3 instanceof T(Integer)");
assertEquals(true,expression.getValue());
assertCanCompile(expression);
assertEquals(true,expression.getValue());
// primitive left operand - should get boxed, return false
expression = parse("3 instanceof T(String)");
assertEquals(false,expression.getValue());
assertCanCompile(expression);
assertEquals(false,expression.getValue());
// double slot left operand - should get boxed, return false
expression = parse("3.0d instanceof T(Integer)");
assertEquals(false,expression.getValue());
assertCanCompile(expression);
assertEquals(false,expression.getValue());
// double slot left operand - should get boxed, return true
expression = parse("3.0d instanceof T(Double)");
assertEquals(true,expression.getValue());
assertCanCompile(expression);
assertEquals(true,expression.getValue());
// Only when the right hand operand is a direct type reference
// will it be compilable.
StandardEvaluationContext ctx = new StandardEvaluationContext();
ctx.setVariable("foo", String.class);
expression = parse("3 instanceof #foo");
assertEquals(false,expression.getValue(ctx));
assertCantCompile(expression);
// use of primitive as type for instanceof check - compilable
// but always false
expression = parse("3 instanceof T(int)");
assertEquals(false,expression.getValue());
assertCanCompile(expression);
assertEquals(false,expression.getValue());
expression = parse("3 instanceof T(long)");
assertEquals(false,expression.getValue());
assertCanCompile(expression);
assertEquals(false,expression.getValue());
}
@Test
public void stringLiteral() throws Exception {
expression = parser.parseExpression("'abcde'");