SPR-6059 Avoiding NPE for OR operator. If the value is null instead of a valid boolean or Boolean, it will now trigger an EvaluationException.

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@2223 50f2f4bb-b051-0410-bef5-90022cba6387

org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/OpAnd.java

@@ -20,12 +20,14 @@ import org.springframework.expression.EvaluationException;
 import org.springframework.expression.TypedValue;
 import org.springframework.expression.spel.ExpressionState;
 import org.springframework.expression.spel.SpelEvaluationException;
+import org.springframework.expression.spel.SpelMessage;
 import org.springframework.expression.spel.support.BooleanTypedValue;
 
 /**
  * Represents the boolean AND operation.
  *
  * @author Andy Clement
+ * @author Mark Fisher
  * @since 3.0
  */
 public class OpAnd extends Operator {
@@ -40,7 +42,9 @@ public class OpAnd extends Operator {
 		boolean rightValue;
 
 		try {
-			leftValue = (Boolean)state.convertValue(getLeftOperand().getValueInternal(state), BOOLEAN_TYPE_DESCRIPTOR);
+			TypedValue typedValue = getLeftOperand().getValueInternal(state);
+			this.assertTypedValueNotNull(typedValue);
+			leftValue = (Boolean)state.convertValue(typedValue, BOOLEAN_TYPE_DESCRIPTOR);
 		}
 		catch (SpelEvaluationException ee) {
 			ee.setPosition(getLeftOperand().getStartPosition());
@@ -52,7 +56,9 @@ public class OpAnd extends Operator {
 		}
 
 		try {
-			rightValue = (Boolean)state.convertValue(getRightOperand().getValueInternal(state), BOOLEAN_TYPE_DESCRIPTOR);
+			TypedValue typedValue = getRightOperand().getValueInternal(state);
+			this.assertTypedValueNotNull(typedValue);
+			rightValue = (Boolean)state.convertValue(typedValue, BOOLEAN_TYPE_DESCRIPTOR);
 		}
 		catch (SpelEvaluationException ee) {
 			ee.setPosition(getRightOperand().getStartPosition());
@@ -62,4 +68,10 @@ public class OpAnd extends Operator {
 		return /* leftValue && */BooleanTypedValue.forValue(rightValue);
 	}
 
+	private void assertTypedValueNotNull(TypedValue typedValue) {
+		if (TypedValue.NULL_TYPED_VALUE.equals(typedValue)) {
+			throw new SpelEvaluationException(SpelMessage.TYPE_CONVERSION_ERROR, "null", "boolean");
+		}
+	}
+
 }

org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/OpOr.java

@@ -17,14 +17,17 @@
 package org.springframework.expression.spel.ast;
 
 import org.springframework.expression.EvaluationException;
+import org.springframework.expression.TypedValue;
 import org.springframework.expression.spel.ExpressionState;
 import org.springframework.expression.spel.SpelEvaluationException;
+import org.springframework.expression.spel.SpelMessage;
 import org.springframework.expression.spel.support.BooleanTypedValue;
 
 /**
  * Represents the boolean OR operation.
  *
  * @author Andy Clement
+ * @author Mark Fisher
  * @since 3.0
  */
 public class OpOr extends Operator {
@@ -37,8 +40,10 @@ public class OpOr extends Operator {
 	public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException {
 		boolean leftValue;
 		boolean rightValue;
-		try { 
+		try {
-			leftValue = (Boolean)state.convertValue(getLeftOperand().getValueInternal(state), BOOLEAN_TYPE_DESCRIPTOR);
+			TypedValue typedValue = getLeftOperand().getValueInternal(state);
+			this.assertTypedValueNotNull(typedValue);
+			leftValue = (Boolean)state.convertValue(typedValue, BOOLEAN_TYPE_DESCRIPTOR);
 		}
 		catch (SpelEvaluationException see) {
 			see.setPosition(getLeftOperand().getStartPosition());
@@ -50,7 +55,9 @@ public class OpOr extends Operator {
 		}
 
 		try {
-			rightValue = (Boolean)state.convertValue(getRightOperand().getValueInternal(state), BOOLEAN_TYPE_DESCRIPTOR);
+			TypedValue typedValue = getRightOperand().getValueInternal(state);
+			this.assertTypedValueNotNull(typedValue);
+			rightValue = (Boolean)state.convertValue(typedValue, BOOLEAN_TYPE_DESCRIPTOR);
 		}
 		catch (SpelEvaluationException see) {
 			see.setPosition(getRightOperand().getStartPosition()); // TODO end positions here and in similar situations
@@ -60,4 +67,10 @@ public class OpOr extends Operator {
 		return BooleanTypedValue.forValue(leftValue || rightValue);
 	}
 
+	private void assertTypedValueNotNull(TypedValue typedValue) {
+		if (TypedValue.NULL_TYPED_VALUE.equals(typedValue)) {
+			throw new SpelEvaluationException(SpelMessage.TYPE_CONVERSION_ERROR, "null", "boolean");
+		}
+	}
+
 }

org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/OperatorNot.java

@@ -16,16 +16,18 @@
 
 package org.springframework.expression.spel.ast;
 
-import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.expression.EvaluationException;
+import org.springframework.expression.TypedValue;
 import org.springframework.expression.spel.ExpressionState;
 import org.springframework.expression.spel.SpelEvaluationException;
+import org.springframework.expression.spel.SpelMessage;
 import org.springframework.expression.spel.support.BooleanTypedValue;
 
 /**
  * Represents a NOT operation.
  *
  * @author Andy Clement
+ * @author Mark Fisher
  * @since 3.0
  */
 public class OperatorNot extends SpelNodeImpl { // Not is a unary operator so do not extend BinaryOperator
@@ -37,7 +39,11 @@ public class OperatorNot extends SpelNodeImpl { // Not is a unary operator so do
 	@Override
 	public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException {
 		try {
-			boolean value = (Boolean) state.convertValue(children[0].getValueInternal(state), TypeDescriptor.valueOf(boolean.class));
+			TypedValue typedValue = children[0].getValueInternal(state);
+			if (TypedValue.NULL_TYPED_VALUE.equals(typedValue)) {
+				throw new SpelEvaluationException(SpelMessage.TYPE_CONVERSION_ERROR, "null", "boolean");
+			}
+			boolean value = (Boolean) state.convertValue(typedValue, BOOLEAN_TYPE_DESCRIPTOR);
 			return BooleanTypedValue.forValue(!value);
 		}
 		catch (SpelEvaluationException see) {

org.springframework.expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java

@@ -308,6 +308,26 @@ public class EvaluationTests extends ExpressionTestCase {
 		parser.parseExpression("!null").getValue();
 	}
 
+	@Test(expected = EvaluationException.class)
+	public void testAndWithNullValueOnLeft() {
+		parser.parseExpression("null and true").getValue();
+	}
+
+	@Test(expected = EvaluationException.class)
+	public void testAndWithNullValueOnRight() {
+		parser.parseExpression("true and null").getValue();
+	}
+
+	@Test(expected = EvaluationException.class)
+	public void testOrWithNullValueOnLeft() {
+		parser.parseExpression("null or false").getValue();
+	}
+
+	@Test(expected = EvaluationException.class)
+	public void testOrWithNullValueOnRight() {
+		parser.parseExpression("false or null").getValue();
+	}
+
 	// assignment
 	@Test
 	public void testAssignmentToVariables01() {