Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Improve exceptions for multi-operand expressions

Browse Source 
Fix SpEL expression parser and tokenizer to provide better exceptions
when dealing with operations that expect two operands. For example,
prior to this commit the expression '/foo' would throw a NPE due
to missing operands to the left of '/'.

Issue: SPR-10146
This commit is contained in:
Phillip Webb
2013-01-14 14:07:26 -08:00
parent d40bd8bd19
commit bff36fb145
4 changed files with 62 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2012 the original author or authors.
* Copyright 2002-2013 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.
@@ -108,7 +108,8 @@ public enum SpelMessage {
OPERAND_NOT_INCREMENTABLE(Kind.ERROR,1066,"the expression component ''{0}'' does not support increment"), //
OPERAND_NOT_DECREMENTABLE(Kind.ERROR,1067,"the expression component ''{0}'' does not support decrement"), //
NOT_ASSIGNABLE(Kind.ERROR,1068,"the expression component ''{0}'' is not assignable"), //
;
MISSING_CHARACTER(Kind.ERROR,1069,"missing expected character ''{0}''"),
LEFT_OPERAND_PROBLEM(Kind.ERROR,1070, "Problem parsing left operand");
private Kind kind;
private int code;

26
spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2012 the original author or authors.
* Copyright 2002-2013 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.
@@ -37,6 +37,7 @@ import org.springframework.util.StringUtils;
* Hand written SpEL parser. Instances are reusable but are not thread safe.
*
* @author Andy Clement
* @author Phillip Webb
* @since 3.0
*/
class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
@@ -104,8 +105,8 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
Token t = peekToken();
if (t.kind==TokenKind.ASSIGN) { // a=b
if (expr==null) {
expr = new NullLiteral(toPos(t.startpos-1,t.endpos-1));
}
expr = new NullLiteral(toPos(t.startpos-1,t.endpos-1));
}
nextToken();
SpelNodeImpl assignedValue = eatLogicalOrExpression();
return new Assign(toPos(t),expr,assignedValue);
@@ -139,7 +140,7 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
while (peekIdentifierToken("or") || peekToken(TokenKind.SYMBOLIC_OR)) {
Token t = nextToken(); //consume OR
SpelNodeImpl rhExpr = eatLogicalAndExpression();
checkRightOperand(t,rhExpr);
checkOperands(t,expr,rhExpr);
expr = new OpOr(toPos(t),expr,rhExpr);
}
return expr;
@@ -151,7 +152,7 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
while (peekIdentifierToken("and") || peekToken(TokenKind.SYMBOLIC_AND)) {
Token t = nextToken();// consume 'AND'
SpelNodeImpl rhExpr = eatRelationalExpression();
checkRightOperand(t,rhExpr);
checkOperands(t,expr,rhExpr);
expr = new OpAnd(toPos(t),expr,rhExpr);
}
return expr;
@@ -164,7 +165,7 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
if (relationalOperatorToken != null) {
Token t = nextToken(); //consume relational operator token
SpelNodeImpl rhExpr = eatSumExpression();
checkRightOperand(t,rhExpr);
checkOperands(t,expr,rhExpr);
TokenKind tk = relationalOperatorToken.kind;
if (relationalOperatorToken.isNumericRelationalOperator()) {
int pos = toPos(t);
@@ -217,7 +218,7 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
while (peekToken(TokenKind.STAR,TokenKind.DIV,TokenKind.MOD)) {
Token t = nextToken(); // consume STAR/DIV/MOD
SpelNodeImpl rhExpr = eatPowerIncDecExpression();
checkRightOperand(t,rhExpr);
checkOperands(t,expr,rhExpr);
if (t.kind==TokenKind.STAR) {
expr = new OpMultiply(toPos(t),expr,rhExpr);
} else if (t.kind==TokenKind.DIV) {
@@ -836,6 +837,17 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
}
}
private void checkOperands(Token token, SpelNodeImpl left, SpelNodeImpl right) {
checkLeftOperand(token, left);
checkRightOperand(token, right);
}
private void checkLeftOperand(Token token, SpelNodeImpl operandExpression) {
if (operandExpression==null) {
raiseInternalException(token.startpos,SpelMessage.LEFT_OPERAND_PROBLEM);
}
}
private void checkRightOperand(Token token, SpelNodeImpl operandExpression) {
if (operandExpression==null) {
raiseInternalException(token.startpos,SpelMessage.RIGHT_OPERAND_PROBLEM);

17
spring-expression/src/main/java/org/springframework/expression/spel/standard/Tokenizer.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2012 the original author or authors.
* Copyright 2002-2013 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.
@@ -29,6 +29,7 @@ import org.springframework.util.Assert;
* Lex some input data into a stream of tokens that can then be parsed.
*
* @author Andy Clement
* @author Phillip Webb
* @since 3.0
*/
class Tokenizer {
@@ -137,14 +138,20 @@ class Tokenizer {
}
break;
case '&':
if (isTwoCharToken(TokenKind.SYMBOLIC_AND)) {
pushPairToken(TokenKind.SYMBOLIC_AND);
if (!isTwoCharToken(TokenKind.SYMBOLIC_AND)) {
throw new InternalParseException(new SpelParseException(
expressionString, pos,
SpelMessage.MISSING_CHARACTER, "&"));
}
pushPairToken(TokenKind.SYMBOLIC_AND);
break;
case '|':
if (isTwoCharToken(TokenKind.SYMBOLIC_OR)) {
pushPairToken(TokenKind.SYMBOLIC_OR);
if (!isTwoCharToken(TokenKind.SYMBOLIC_OR)) {
throw new InternalParseException(new SpelParseException(
expressionString, pos,
SpelMessage.MISSING_CHARACTER, "|"));
}
pushPairToken(TokenKind.SYMBOLIC_OR);
break;
case '?':
if (isTwoCharToken(TokenKind.SELECT)) {

31
spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java
View File

@@ -16,6 +16,11 @@
package org.springframework.expression.spel;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.ArrayList;
@@ -26,8 +31,9 @@ import java.util.Map;
import java.util.Properties;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.springframework.core.convert.TypeDescriptor;
import org.springframework.expression.AccessException;
import org.springframework.expression.BeanResolver;
@@ -48,8 +54,6 @@ import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.expression.spel.support.StandardTypeLocator;
import org.springframework.expression.spel.testresources.le.div.mod.reserved.Reserver;
import static org.junit.Assert.*;
/**
* Reproduction tests cornering various SpEL JIRA issues.
*
@@ -60,6 +64,9 @@ import static org.junit.Assert.*;
*/
public class SpelReproTests extends ExpressionTestCase {
@Rule
public ExpectedException thrown = ExpectedException.none();
@Test
public void testNPE_SPR5661() {
evaluate("joinThreeStrings('a',null,'c')", "anullc", String.class);
@@ -1694,6 +1701,24 @@ public class SpelReproTests extends ExpressionTestCase {
Object value = parser.parseExpression("primitiveProperty").getValue(evaluationContext);
}
@Test
public void SPR_10146_malformedExpressions() throws Exception {
doTestSpr10146("/foo", "EL1070E:(pos 0): Problem parsing left operand");
doTestSpr10146("*foo", "EL1070E:(pos 0): Problem parsing left operand");
doTestSpr10146("%foo", "EL1070E:(pos 0): Problem parsing left operand");
doTestSpr10146("<foo", "EL1070E:(pos 0): Problem parsing left operand");
doTestSpr10146(">foo", "EL1070E:(pos 0): Problem parsing left operand");
doTestSpr10146("&&foo", "EL1070E:(pos 0): Problem parsing left operand");
doTestSpr10146("||foo", "EL1070E:(pos 0): Problem parsing left operand");
doTestSpr10146("&foo", "EL1069E:(pos 0): missing expected character '&'");
doTestSpr10146("|foo", "EL1069E:(pos 0): missing expected character '|'");
}
private void doTestSpr10146(String expression, String expectedMessage) {
thrown.expect(SpelParseException.class);
thrown.expectMessage(expectedMessage);
new SpelExpressionParser().parseExpression(expression);
}
public static class BooleanHolder {