Skip to content

Commit

Permalink
[OLINGO-1028] stricter multiplicity tests in expression parser + clea…
Browse files Browse the repository at this point in the history 
…n-up

Signed-off-by: Christian Amend <christian.amend@sap.com>
  • Loading branch information
Klaus Straubinger authored and Christian Amend committed Sep 29, 2016
1 parent b9a71ff commit 72fcaa1
Show file tree
Hide file tree
Showing 13 changed files with 6,388 additions and 7,217 deletions.
59 changes: 47 additions & 12 deletions lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/ExpressionParser.java
View file
Expand Up @@ -180,15 +180,19 @@ public Expression parse(UriTokenizer tokenizer, final EdmType referringType,
this.crossjoinEntitySetNames = crossjoinEntitySetNames;
this.aliases = aliases;

return parseExpression();
final Expression expression = parseExpression();
checkNoCollection(expression);
return expression;
}

private Expression parseExpression() throws UriParserException, UriValidationException {
Expression left = parseAnd();
while (tokenizer.next(TokenKind.OrOperator)) {
final Expression right = parseAnd();
checkType(left, EdmPrimitiveTypeKind.Boolean);
checkNoCollection(left);
final Expression right = parseAnd();
checkType(right, EdmPrimitiveTypeKind.Boolean);
checkNoCollection(right);
left = new BinaryImpl(left, BinaryOperatorKind.OR, right,
odata.createPrimitiveTypeInstance(EdmPrimitiveTypeKind.Boolean));
}
Expand All @@ -198,9 +202,11 @@ private Expression parseExpression() throws UriParserException, UriValidationExc
private Expression parseAnd() throws UriParserException, UriValidationException {
Expression left = parseExprEquality();
while (tokenizer.next(TokenKind.AndOperator)) {
final Expression right = parseExprEquality();
checkType(left, EdmPrimitiveTypeKind.Boolean);
checkNoCollection(left);
final Expression right = parseExprEquality();
checkType(right, EdmPrimitiveTypeKind.Boolean);
checkNoCollection(right);
left = new BinaryImpl(left, BinaryOperatorKind.AND, right,
odata.createPrimitiveTypeInstance(EdmPrimitiveTypeKind.Boolean));
}
Expand Down Expand Up @@ -285,8 +291,8 @@ private Expression parseExprMul() throws UriParserException, UriValidationExcept
TokenKind.MulOperator, TokenKind.DivOperator, TokenKind.ModOperator);
// Null for everything other than MUL or DIV or MOD
while (operatorTokenKind != null) {
final Expression right = parseExprUnary();
checkNumericType(left);
final Expression right = parseExprUnary();
checkNumericType(right);
left = new BinaryImpl(left, tokenToBinaryOperator.get(operatorTokenKind), right,
odata.createPrimitiveTypeInstance(EdmPrimitiveTypeKind.Double));
Expand All @@ -306,6 +312,7 @@ private Expression parseExprUnary() throws UriParserException, UriValidationExce
} else if (tokenizer.next(TokenKind.NotOperator)) {
final Expression expression = parseExprPrimary();
checkType(expression, EdmPrimitiveTypeKind.Boolean);
checkNoCollection(expression);
return new UnaryImpl(UnaryOperatorKind.NOT, expression, getType(expression));
} else if (tokenizer.next(TokenKind.CastMethod)) {
return parseIsOfOrCastMethod(MethodKind.CAST);
Expand Down Expand Up @@ -443,13 +450,15 @@ private List<Expression> parseMethodParameters(final MethodKind methodKind)
case TRIM:
final Expression stringParameter = parseExpression();
checkType(stringParameter, EdmPrimitiveTypeKind.String);
checkNoCollection(stringParameter);
parameters.add(stringParameter);
break;
case YEAR:
case MONTH:
case DAY:
final Expression dateParameter = parseExpression();
checkType(dateParameter, EdmPrimitiveTypeKind.Date, EdmPrimitiveTypeKind.DateTimeOffset);
checkNoCollection(dateParameter);
parameters.add(dateParameter);
break;
case HOUR:
Expand All @@ -458,18 +467,21 @@ private List<Expression> parseMethodParameters(final MethodKind methodKind)
case FRACTIONALSECONDS:
final Expression timeParameter = parseExpression();
checkType(timeParameter, EdmPrimitiveTypeKind.TimeOfDay, EdmPrimitiveTypeKind.DateTimeOffset);
checkNoCollection(timeParameter);
parameters.add(timeParameter);
break;
case DATE:
case TIME:
case TOTALOFFSETMINUTES:
final Expression dateTimeParameter = parseExpression();
checkType(dateTimeParameter, EdmPrimitiveTypeKind.DateTimeOffset);
checkNoCollection(dateTimeParameter);
parameters.add(dateTimeParameter);
break;
case TOTALSECONDS:
final Expression durationParameter = parseExpression();
checkType(durationParameter, EdmPrimitiveTypeKind.Duration);
checkNoCollection(durationParameter);
parameters.add(durationParameter);
break;
case ROUND:
Expand All @@ -478,12 +490,14 @@ private List<Expression> parseMethodParameters(final MethodKind methodKind)
final Expression decimalParameter = parseExpression();
checkType(decimalParameter,
EdmPrimitiveTypeKind.Decimal, EdmPrimitiveTypeKind.Single, EdmPrimitiveTypeKind.Double);
checkNoCollection(decimalParameter);
parameters.add(decimalParameter);
break;
case GEOLENGTH:
final Expression geoParameter = parseExpression();
checkType(geoParameter,
EdmPrimitiveTypeKind.GeographyLineString, EdmPrimitiveTypeKind.GeometryLineString);
checkNoCollection(geoParameter);
parameters.add(geoParameter);
break;

Expand All @@ -495,37 +509,44 @@ private List<Expression> parseMethodParameters(final MethodKind methodKind)
case CONCAT:
final Expression stringParameter1 = parseExpression();
checkType(stringParameter1, EdmPrimitiveTypeKind.String);
checkNoCollection(stringParameter1);
parameters.add(stringParameter1);
ParserHelper.requireNext(tokenizer, TokenKind.COMMA);
final Expression stringParameter2 = parseExpression();
checkType(stringParameter2, EdmPrimitiveTypeKind.String);
checkNoCollection(stringParameter2);
parameters.add(stringParameter2);
break;
case GEODISTANCE:
final Expression geoParameter1 = parseExpression();
checkType(geoParameter1, EdmPrimitiveTypeKind.GeographyPoint, EdmPrimitiveTypeKind.GeometryPoint);
checkNoCollection(geoParameter1);
parameters.add(geoParameter1);
ParserHelper.requireNext(tokenizer, TokenKind.COMMA);
final Expression geoParameter2 = parseExpression();
checkType(geoParameter2, EdmPrimitiveTypeKind.GeographyPoint, EdmPrimitiveTypeKind.GeometryPoint);
checkNoCollection(geoParameter2);
parameters.add(geoParameter2);
break;
case GEOINTERSECTS:
final Expression geoPointParameter = parseExpression();
checkType(geoPointParameter,
EdmPrimitiveTypeKind.GeographyPoint, EdmPrimitiveTypeKind.GeometryPoint);
checkNoCollection(geoPointParameter);
parameters.add(geoPointParameter);
ParserHelper.requireNext(tokenizer, TokenKind.COMMA);
final Expression geoPolygonParameter = parseExpression();
checkType(geoPolygonParameter,
EdmPrimitiveTypeKind.GeographyPolygon, EdmPrimitiveTypeKind.GeometryPolygon);
checkNoCollection(geoPolygonParameter);
parameters.add(geoPolygonParameter);
break;

// Can have two or three parameters.
case SUBSTRING:
final Expression parameterFirst = parseExpression();
checkType(parameterFirst, EdmPrimitiveTypeKind.String);
checkNoCollection(parameterFirst);
parameters.add(parameterFirst);
ParserHelper.requireNext(tokenizer, TokenKind.COMMA);
final Expression parameterSecond = parseExpression();
Expand Down Expand Up @@ -905,8 +926,9 @@ private void parseCollectionPathExpr(UriInfoImpl uriInfo, final UriResourcePartT
} else if (tokenizer.next(TokenKind.ALL)) {
uriInfo.addResourcePart(parseLambdaRest(TokenKind.ALL, lastResource));
} else if (tokenizer.next(TokenKind.QualifiedName)) {
final FullQualifiedName fullQualifiedName = new FullQualifiedName(tokenizer.getText());
parseBoundFunction(fullQualifiedName, uriInfo, lastResource);
parseBoundFunction(new FullQualifiedName(tokenizer.getText()), uriInfo, lastResource);
} else {
throw new UriParserSyntaxException("Unexpected token.", UriParserSyntaxException.MessageKeys.SYNTAX);
}
}

Expand Down Expand Up @@ -1071,20 +1093,32 @@ private void checkType(final Expression expression, final EdmPrimitiveTypeKind..
}
}

private void checkNoCollection(final Expression expression) throws UriParserException {
if (expression instanceof Member && ((Member) expression).isCollection()) {
throw new UriParserSemanticException("Collection not allowed.",
UriParserSemanticException.MessageKeys.COLLECTION_NOT_ALLOWED);
}
}

protected void checkIntegerType(final Expression expression) throws UriParserException {
checkNoCollection(expression);
checkType(expression,
EdmPrimitiveTypeKind.Int64, EdmPrimitiveTypeKind.Int32, EdmPrimitiveTypeKind.Int16,
EdmPrimitiveTypeKind.Byte, EdmPrimitiveTypeKind.SByte);
}

protected void checkNumericType(final Expression expression) throws UriParserException {
checkNoCollection(expression);
checkType(expression,
EdmPrimitiveTypeKind.Int64, EdmPrimitiveTypeKind.Int32, EdmPrimitiveTypeKind.Int16,
EdmPrimitiveTypeKind.Byte, EdmPrimitiveTypeKind.SByte,
EdmPrimitiveTypeKind.Decimal, EdmPrimitiveTypeKind.Single, EdmPrimitiveTypeKind.Double);
}

private void checkEqualityTypes(final Expression left, final Expression right) throws UriParserException {
checkNoCollection(left);
checkNoCollection(right);

final EdmType leftType = getType(left);
final EdmType rightType = getType(right);
if (leftType == null || rightType == null || leftType.equals(rightType)) {
Expand Down Expand Up @@ -1141,11 +1175,10 @@ private Enumeration createEnumExpression(final String primitiveValueLiteral) thr
}

private void checkRelationTypes(final Expression left, final Expression right) throws UriParserException {
checkNoCollection(left);
checkNoCollection(right);
final EdmType leftType = getType(left);
final EdmType rightType = getType(right);
if (leftType == null || rightType == null) {
return;
}
checkType(left,
EdmPrimitiveTypeKind.Int16, EdmPrimitiveTypeKind.Int32, EdmPrimitiveTypeKind.Int64,
EdmPrimitiveTypeKind.Byte, EdmPrimitiveTypeKind.SByte,
Expand All @@ -1160,6 +1193,9 @@ private void checkRelationTypes(final Expression left, final Expression right) t
EdmPrimitiveTypeKind.Boolean, EdmPrimitiveTypeKind.Guid, EdmPrimitiveTypeKind.String,
EdmPrimitiveTypeKind.Date, EdmPrimitiveTypeKind.TimeOfDay,
EdmPrimitiveTypeKind.DateTimeOffset, EdmPrimitiveTypeKind.Duration);
if (leftType == null || rightType == null) {
return;
}
if (!(((EdmPrimitiveType) leftType).isCompatible((EdmPrimitiveType) rightType)
|| ((EdmPrimitiveType) rightType).isCompatible((EdmPrimitiveType) leftType))) {
throw new UriParserSemanticException("Incompatible types.",
Expand All @@ -1171,11 +1207,10 @@ private void checkRelationTypes(final Expression left, final Expression right) t

private EdmType getAddSubTypeAndCheckLeftAndRight(final Expression left, final Expression right, final boolean isSub)
throws UriParserException {
checkNoCollection(left);
checkNoCollection(right);
final EdmType leftType = getType(left);
final EdmType rightType = getType(right);
if (leftType == null || rightType == null) {
return null;
}
if (isType(leftType,
EdmPrimitiveTypeKind.Int16, EdmPrimitiveTypeKind.Int32, EdmPrimitiveTypeKind.Int64,
EdmPrimitiveTypeKind.Byte, EdmPrimitiveTypeKind.SByte,
Expand Down
3 changes: 2 additions & 1 deletion ...re/src/main/java/org/apache/olingo/server/core/uri/parser/UriParserSemanticException.java
View file
Expand Up @@ -74,7 +74,8 @@ public static enum MessageKeys implements MessageKey {
/** parameter: expression */
ONLY_FOR_PRIMITIVE_TYPES,
/** parameter: function name */
FUNCTION_MUST_USE_COLLECTIONS;
FUNCTION_MUST_USE_COLLECTIONS,
COLLECTION_NOT_ALLOWED;

@Override
public String getKey() {
Expand Down
14 changes: 6 additions & 8 deletions ...re/src/main/java/org/apache/olingo/server/core/uri/queryoption/expression/MemberImpl.java
View file
Expand Up @@ -21,6 +21,8 @@
import org.apache.olingo.commons.api.edm.EdmType;
import org.apache.olingo.server.api.ODataApplicationException;
import org.apache.olingo.server.api.uri.UriInfoResource;
import org.apache.olingo.server.api.uri.UriResource;
import org.apache.olingo.server.api.uri.UriResourcePartTyped;
import org.apache.olingo.server.api.uri.queryoption.expression.ExpressionVisitException;
import org.apache.olingo.server.api.uri.queryoption.expression.ExpressionVisitor;
import org.apache.olingo.server.api.uri.queryoption.expression.Member;
Expand Down Expand Up @@ -85,14 +87,10 @@ public EdmType getType() {
@Override
public boolean isCollection() {
UriInfoImpl uriInfo = (UriInfoImpl) path;
UriResourceImpl lastResourcePart = (UriResourceImpl) uriInfo.getLastResourcePart();
if (lastResourcePart instanceof UriResourceTypedImpl) {
UriResourceTypedImpl lastTyped = (UriResourceTypedImpl) lastResourcePart;
return lastTyped.isCollection();
} else if (lastResourcePart instanceof UriResourceActionImpl) {
return ((UriResourceActionImpl) lastResourcePart).isCollection();
}
return false;
UriResource lastResourcePart = uriInfo.getLastResourcePart();
return lastResourcePart instanceof UriResourcePartTyped ?
((UriResourcePartTyped) lastResourcePart).isCollection() :
false;
}

@Override
Expand Down
1 change: 1 addition & 0 deletions lib/server-core/src/main/resources/server-core-exceptions-i18n.properties
View file
Expand Up @@ -76,6 +76,7 @@ UriParserSemanticException.NOT_A_MEDIA_RESOURCE=The resource '%1$s' is not a med
UriParserSemanticException.IS_PROPERTY=The identifier '%1$s' is already used as a property.
UriParserSemanticException.ONLY_FOR_PRIMITIVE_TYPES='%1$s' is only allowed for primitive-type expressions.
UriParserSemanticException.FUNCTION_MUST_USE_COLLECTIONS=Only bound functions with collections of structural types as binding parameter and as return type are allowed; '%1$s' is not such a function.
UriParserSemanticException.COLLECTION_NOT_ALLOWED=A collection expression is not allowed.

UriValidationException.UNSUPPORTED_QUERY_OPTION=The query option '%1$s' is not supported.
UriValidationException.UNSUPPORTED_URI_KIND=The URI kind '%1$s' is not supported.
Expand Down

0 comments on commit 72fcaa1

Please sign in to comment.