Skip to content

Commit bff36fb

Browse files
philwebbphilwebb
committed
Improve exceptions for multi-operand expressions
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
1 parent d40bd8b commit bff36fb

File tree

4 files changed

+62
-17
lines changed

4 files changed

+62
-17
lines changed

‎spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2012 the original author or authors.
2+
* Copyright 2002-2013 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -108,7 +108,8 @@ public enum SpelMessage {
108108
OPERAND_NOT_INCREMENTABLE(Kind.ERROR,1066,"the expression component ''{0}'' does not support increment"), //
109109
OPERAND_NOT_DECREMENTABLE(Kind.ERROR,1067,"the expression component ''{0}'' does not support decrement"), //
110110
NOT_ASSIGNABLE(Kind.ERROR,1068,"the expression component ''{0}'' is not assignable"), //
111-
;
111+
MISSING_CHARACTER(Kind.ERROR,1069,"missing expected character ''{0}''"),
112+
LEFT_OPERAND_PROBLEM(Kind.ERROR,1070, "Problem parsing left operand");
112113

113114
private Kind kind;
114115
private int code;

‎spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2012 the original author or authors.
2+
* Copyright 2002-2013 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -37,6 +37,7 @@
3737
* Hand written SpEL parser. Instances are reusable but are not thread safe.
3838
*
3939
* @author Andy Clement
40+
* @author Phillip Webb
4041
* @since 3.0
4142
*/
4243
class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
@@ -104,8 +105,8 @@ private SpelNodeImpl eatExpression() {
104105
Token t = peekToken();
105106
if (t.kind==TokenKind.ASSIGN) { // a=b
106107
if (expr==null) {
107-
expr = new NullLiteral(toPos(t.startpos-1,t.endpos-1));
108-
}
108+
expr = new NullLiteral(toPos(t.startpos-1,t.endpos-1));
109+
}
109110
nextToken();
110111
SpelNodeImpl assignedValue = eatLogicalOrExpression();
111112
return new Assign(toPos(t),expr,assignedValue);
@@ -139,7 +140,7 @@ private SpelNodeImpl eatLogicalOrExpression() {
139140
while (peekIdentifierToken("or") || peekToken(TokenKind.SYMBOLIC_OR)) {
140141
Token t = nextToken(); //consume OR
141142
SpelNodeImpl rhExpr = eatLogicalAndExpression();
142-
checkRightOperand(t,rhExpr);
143+
checkOperands(t,expr,rhExpr);
143144
expr = new OpOr(toPos(t),expr,rhExpr);
144145
}
145146
return expr;
@@ -151,7 +152,7 @@ private SpelNodeImpl eatLogicalAndExpression() {
151152
while (peekIdentifierToken("and") || peekToken(TokenKind.SYMBOLIC_AND)) {
152153
Token t = nextToken();// consume 'AND'
153154
SpelNodeImpl rhExpr = eatRelationalExpression();
154-
checkRightOperand(t,rhExpr);
155+
checkOperands(t,expr,rhExpr);
155156
expr = new OpAnd(toPos(t),expr,rhExpr);
156157
}
157158
return expr;
@@ -164,7 +165,7 @@ private SpelNodeImpl eatRelationalExpression() {
164165
if (relationalOperatorToken != null) {
165166
Token t = nextToken(); //consume relational operator token
166167
SpelNodeImpl rhExpr = eatSumExpression();
167-
checkRightOperand(t,rhExpr);
168+
checkOperands(t,expr,rhExpr);
168169
TokenKind tk = relationalOperatorToken.kind;
169170
if (relationalOperatorToken.isNumericRelationalOperator()) {
170171
int pos = toPos(t);
@@ -217,7 +218,7 @@ private SpelNodeImpl eatProductExpression() {
217218
while (peekToken(TokenKind.STAR,TokenKind.DIV,TokenKind.MOD)) {
218219
Token t = nextToken(); // consume STAR/DIV/MOD
219220
SpelNodeImpl rhExpr = eatPowerIncDecExpression();
220-
checkRightOperand(t,rhExpr);
221+
checkOperands(t,expr,rhExpr);
221222
if (t.kind==TokenKind.STAR) {
222223
expr = new OpMultiply(toPos(t),expr,rhExpr);
223224
} else if (t.kind==TokenKind.DIV) {
@@ -836,6 +837,17 @@ public String toString(Token t) {
836837
}
837838
}
838839

840+
private void checkOperands(Token token, SpelNodeImpl left, SpelNodeImpl right) {
841+
checkLeftOperand(token, left);
842+
checkRightOperand(token, right);
843+
}
844+
845+
private void checkLeftOperand(Token token, SpelNodeImpl operandExpression) {
846+
if (operandExpression==null) {
847+
raiseInternalException(token.startpos,SpelMessage.LEFT_OPERAND_PROBLEM);
848+
}
849+
}
850+
839851
private void checkRightOperand(Token token, SpelNodeImpl operandExpression) {
840852
if (operandExpression==null) {
841853
raiseInternalException(token.startpos,SpelMessage.RIGHT_OPERAND_PROBLEM);

‎spring-expression/src/main/java/org/springframework/expression/spel/standard/Tokenizer.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2012 the original author or authors.
2+
* Copyright 2002-2013 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -29,6 +29,7 @@
2929
* Lex some input data into a stream of tokens that can then be parsed.
3030
*
3131
* @author Andy Clement
32+
* @author Phillip Webb
3233
* @since 3.0
3334
*/
3435
class Tokenizer {
@@ -137,14 +138,20 @@ public void process() {
137138
}
138139
break;
139140
case '&':
140-
if (isTwoCharToken(TokenKind.SYMBOLIC_AND)) {
141-
pushPairToken(TokenKind.SYMBOLIC_AND);
141+
if (!isTwoCharToken(TokenKind.SYMBOLIC_AND)) {
142+
throw new InternalParseException(new SpelParseException(
143+
expressionString, pos,
144+
SpelMessage.MISSING_CHARACTER, "&"));
142145
}
146+
pushPairToken(TokenKind.SYMBOLIC_AND);
143147
break;
144148
case '|':
145-
if (isTwoCharToken(TokenKind.SYMBOLIC_OR)) {
146-
pushPairToken(TokenKind.SYMBOLIC_OR);
149+
if (!isTwoCharToken(TokenKind.SYMBOLIC_OR)) {
150+
throw new InternalParseException(new SpelParseException(
151+
expressionString, pos,
152+
SpelMessage.MISSING_CHARACTER, "|"));
147153
}
154+
pushPairToken(TokenKind.SYMBOLIC_OR);
148155
break;
149156
case '?':
150157
if (isTwoCharToken(TokenKind.SELECT)) {

‎spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@
1616

1717
package org.springframework.expression.spel;
1818

19+
import static org.junit.Assert.assertEquals;
20+
import static org.junit.Assert.assertFalse;
21+
import static org.junit.Assert.assertTrue;
22+
import static org.junit.Assert.fail;
23+
1924
import java.lang.reflect.Field;
2025
import java.lang.reflect.Method;
2126
import java.util.ArrayList;
@@ -26,8 +31,9 @@
2631
import java.util.Properties;
2732

2833
import org.junit.Ignore;
34+
import org.junit.Rule;
2935
import org.junit.Test;
30-
36+
import org.junit.rules.ExpectedException;
3137
import org.springframework.core.convert.TypeDescriptor;
3238
import org.springframework.expression.AccessException;
3339
import org.springframework.expression.BeanResolver;
@@ -48,8 +54,6 @@
4854
import org.springframework.expression.spel.support.StandardTypeLocator;
4955
import org.springframework.expression.spel.testresources.le.div.mod.reserved.Reserver;
5056

51-
import static org.junit.Assert.*;
52-
5357
/**
5458
* Reproduction tests cornering various SpEL JIRA issues.
5559
*
@@ -60,6 +64,9 @@
6064
*/
6165
public class SpelReproTests extends ExpressionTestCase {
6266

67+
@Rule
68+
public ExpectedException thrown = ExpectedException.none();
69+
6370
@Test
6471
public void testNPE_SPR5661() {
6572
evaluate("joinThreeStrings('a',null,'c')", "anullc", String.class);
@@ -1694,6 +1701,24 @@ public void SPR_10091_primitiveTestValue() {
16941701
Object value = parser.parseExpression("primitiveProperty").getValue(evaluationContext);
16951702
}
16961703

1704+
@Test
1705+
public void SPR_10146_malformedExpressions() throws Exception {
1706+
doTestSpr10146("/foo", "EL1070E:(pos 0): Problem parsing left operand");
1707+
doTestSpr10146("*foo", "EL1070E:(pos 0): Problem parsing left operand");
1708+
doTestSpr10146("%foo", "EL1070E:(pos 0): Problem parsing left operand");
1709+
doTestSpr10146("<foo", "EL1070E:(pos 0): Problem parsing left operand");
1710+
doTestSpr10146(">foo", "EL1070E:(pos 0): Problem parsing left operand");
1711+
doTestSpr10146("&&foo", "EL1070E:(pos 0): Problem parsing left operand");
1712+
doTestSpr10146("||foo", "EL1070E:(pos 0): Problem parsing left operand");
1713+
doTestSpr10146("&foo", "EL1069E:(pos 0): missing expected character '&'");
1714+
doTestSpr10146("|foo", "EL1069E:(pos 0): missing expected character '|'");
1715+
}
1716+
1717+
private void doTestSpr10146(String expression, String expectedMessage) {
1718+
thrown.expect(SpelParseException.class);
1719+
thrown.expectMessage(expectedMessage);
1720+
new SpelExpressionParser().parseExpression(expression);
1721+
}
16971722

16981723
public static class BooleanHolder {
16991724

0 commit comments

Comments
 (0)