diff --git a/core/queryparser/sparql/src/main/java/org/eclipse/rdf4j/query/parser/sparql/TupleExprBuilder.java b/core/queryparser/sparql/src/main/java/org/eclipse/rdf4j/query/parser/sparql/TupleExprBuilder.java index 2fa952ee62..a52d72d887 100644 --- a/core/queryparser/sparql/src/main/java/org/eclipse/rdf4j/query/parser/sparql/TupleExprBuilder.java +++ b/core/queryparser/sparql/src/main/java/org/eclipse/rdf4j/query/parser/sparql/TupleExprBuilder.java @@ -180,6 +180,7 @@ import org.eclipse.rdf4j.query.parser.sparql.ast.ASTPathAlternative; import org.eclipse.rdf4j.query.parser.sparql.ast.ASTPathElt; import org.eclipse.rdf4j.query.parser.sparql.ast.ASTPathMod; +import org.eclipse.rdf4j.query.parser.sparql.ast.ASTPathNegatedPropertySet; import org.eclipse.rdf4j.query.parser.sparql.ast.ASTPathOneInPropertySet; import org.eclipse.rdf4j.query.parser.sparql.ast.ASTPathSequence; import org.eclipse.rdf4j.query.parser.sparql.ast.ASTProjectionElem; @@ -251,9 +252,6 @@ public class TupleExprBuilder extends AbstractASTVisitor { GraphPattern graphPattern = new GraphPattern(); - // private Map mappedValueConstants = new - // HashMap(); - /*--------------* * Constructors * *--------------*/ @@ -820,6 +818,12 @@ public TupleExpr visit(ASTConstructQuery node, Object data) throws VisitorExcept @Override public TupleExpr visit(ASTConstruct node, Object data) throws VisitorException { + + // check if the construct template contains any invalid nodes + if (isInvalidConstructQueryTemplate(node, true)) { + throw new MalformedQueryException("Invalid construct clause."); + } + TupleExpr result = (TupleExpr) data; // Collect construct triples @@ -872,7 +876,7 @@ public TupleExpr visit(ASTConstruct node, Object data) throws VisitorException { // assign non-anonymous vars not present in where clause as // extension elements. This is necessary to make external // binding - // assingnment possible (see SES-996) + // assignment possible (see SES-996) extElemMap.put(var, new ExtensionElem(var.clone(), var.getName())); } } @@ -910,6 +914,41 @@ public TupleExpr visit(ASTConstruct node, Object data) throws VisitorException { return new Reduced(result); } + /** + * Checks if the given node is invalid in a CONSTRUCT template. + * + * @param node The node to check. + * @param isInConstructTemplate Indicates if the check is being performed within a CONSTRUCT clause. + * @return true if the node is invalid, false otherwise. + */ + private static boolean isInvalidConstructQueryTemplate(Node node, boolean isInConstructTemplate) { + if (isInConstructTemplate && isInvalidConstructNode(node)) { + return true; + } + + // recursively check child nodes + for (int i = 0; i < node.jjtGetNumChildren(); i++) { + if (isInvalidConstructQueryTemplate(node.jjtGetChild(i), isInConstructTemplate)) { + return true; + } + } + + return false; + } + + /** + * Checks if the given node is an invalid construct node. + * + * @param node The node to check. + * @return true if the node is invalid, false otherwise. + */ + private static boolean isInvalidConstructNode(Node node) { + return node instanceof ASTPathMod + || node instanceof ASTPathNegatedPropertySet + || node instanceof ASTPathOneInPropertySet + || (node instanceof ASTPathAlternative && node.jjtGetNumChildren() > 1); + } + /** * Gets the set of variables that are relevant for the constructor. This method accumulates all subject, predicate * and object variables from the supplied statement patterns, but ignores any context variables. @@ -1359,7 +1398,7 @@ public TupleExpr visit(ASTPathAlternative pathAltNode, Object data) throws Visit } } - // when using union to execute path expressions, the scope does not not change + // when using union to execute path expressions, the scope does not change union.setVariableScopeChange(false); return union; } @@ -1548,7 +1587,7 @@ private TupleExpr createTupleExprForNegatedPropertySets(List np TupleExpr patternMatchInverse = null; - // build a inverse statement pattern if needed + // build an inverse statement pattern if needed if (filterConditionInverse != null) { patternMatchInverse = new StatementPattern(pathSequenceContext.scope, endVar.clone(), predVar.clone(), subjVar.clone(), @@ -2362,7 +2401,7 @@ public Object visit(ASTBind node, Object data) throws VisitorException { ValueExpr ve = child0 instanceof ValueExpr ? (ValueExpr) child0 : (child0 instanceof TripleRef) ? ((TripleRef) child0).getExprVar() : null; if (ve == null) { - throw new IllegalArgumentException("Unexpected expressin on bind"); + throw new IllegalArgumentException("Unexpected expression on bind"); } // name to bind the expression outcome to @@ -2381,7 +2420,7 @@ public Object visit(ASTBind node, Object data) throws VisitorException { // check if alias is not previously used in the BGP if (arg.getBindingNames().contains(alias)) { - // SES-2314 we need to doublecheck that the reused varname is not just + // SES-2314 we need to double-check that the reused varname is not just // for an anonymous var or a constant. VarCollector collector = new VarCollector(); arg.visit(collector); diff --git a/core/queryparser/sparql/src/test/java/org/eclipse/rdf4j/query/parser/sparql/SPARQLParserTest.java b/core/queryparser/sparql/src/test/java/org/eclipse/rdf4j/query/parser/sparql/SPARQLParserTest.java index fa96363378..e52e101677 100644 --- a/core/queryparser/sparql/src/test/java/org/eclipse/rdf4j/query/parser/sparql/SPARQLParserTest.java +++ b/core/queryparser/sparql/src/test/java/org/eclipse/rdf4j/query/parser/sparql/SPARQLParserTest.java @@ -43,6 +43,7 @@ import org.eclipse.rdf4j.model.util.Values; import org.eclipse.rdf4j.query.BindingSet; import org.eclipse.rdf4j.query.MalformedQueryException; +import org.eclipse.rdf4j.query.QueryLanguage; import org.eclipse.rdf4j.query.algebra.AggregateFunctionCall; import org.eclipse.rdf4j.query.algebra.ArbitraryLengthPath; import org.eclipse.rdf4j.query.algebra.DeleteData; @@ -70,6 +71,8 @@ import org.eclipse.rdf4j.query.parser.ParsedQuery; import org.eclipse.rdf4j.query.parser.ParsedTupleQuery; import org.eclipse.rdf4j.query.parser.ParsedUpdate; +import org.eclipse.rdf4j.query.parser.QueryParserUtil; +import org.eclipse.rdf4j.query.parser.sparql.SPARQLParser; import org.eclipse.rdf4j.query.parser.sparql.aggregate.AggregateCollector; import org.eclipse.rdf4j.query.parser.sparql.aggregate.AggregateFunction; import org.eclipse.rdf4j.query.parser.sparql.aggregate.AggregateFunctionFactory; @@ -1002,6 +1005,158 @@ public void testApostropheInsertData() { parser.parseUpdate(query, null); } + @Test + public void testInvalidConstructQueryWithPropertyPathInConstructClause() { + String invalidSparqlQuery = " CONSTRUCT {\n" + + " ?s (!a)* ?p .\n" + + " }\n" + + " WHERE {\n" + + " ?s (!a)* ?p .\n" + + " }"; + + assertThrows(MalformedQueryException.class, () -> { + parser.parseQuery(invalidSparqlQuery, null); + }); + + } + + @Test + public void testValidConstructQueryWithPropertyPathInWhereClause() { + String validSparqlQuery = "PREFIX foaf: " + + "CONSTRUCT {\n" + + " ?person foaf:knows ?friend .\n" + + "}\n" + + "WHERE {\n" + + " ?person foaf:knows+ ?friend .\n" + + "}"; + + ParsedQuery parsedQuery = parser.parseQuery(validSparqlQuery, null); + assertThat(parsedQuery.getSourceString()).isEqualTo(validSparqlQuery); + + } + + @Test + public void testValidConstructQueryWithBlankNodeAsSubject() { + String validSparqlQuery = "PREFIX foaf: \n" + + "PREFIX site: \n" + + "\n" + + "CONSTRUCT { [] foaf:name ?name }\n" + + "WHERE\n" + + "{ [] foaf:name ?name ;\n" + + " site:hits ?hits .\n" + + "}\n" + + "ORDER BY desc(?hits)\n" + + "LIMIT 2"; + + ParsedQuery parsedQuery = parser.parseQuery(validSparqlQuery, null); + assertThat(parsedQuery.getSourceString()).isEqualTo(validSparqlQuery); + + } + + @Test + public void testInvalidConstructQueryWithPropertyPathInPredicatePosition() { + String invalidSparqlQuery = "PREFIX foaf: " + + "CONSTRUCT {\n" + + " ?person foaf:knows+ ?friend . " + + "}\n" + + "WHERE {\n" + + " ?person foaf:knows+ ?friend .\n" + + "}"; + assertThrows(MalformedQueryException.class, () -> { + parser.parseQuery(invalidSparqlQuery, null); + }); + + } + + @Test + public void testInvalidConstructQueryWithPropertyPathAlternationInPredicate() { + String invalidSparqlQuery = "PREFIX foaf: " + + "CONSTRUCT {\n" + + " ?x (foaf:knows|foaf:friendOf) ?y . " + + "}\n" + + "WHERE {\n" + + " ?x (foaf:knows|foaf:friendOf) ?y .\n" + + "} "; + assertThrows(MalformedQueryException.class, () -> { + parser.parseQuery(invalidSparqlQuery, null); + }); + + } + + @Test + public void testInvalidConstructQueryWithPathSequenceInPredicate() { + String invalidSparqlQuery = "PREFIX foaf: " + + "CONSTRUCT {\n" + + " ?a foaf:knows/foaf:knows ?c . " + + "}\n" + + "WHERE {\n" + + " ?a foaf:knows/foaf:knows ?c . .\n" + + "} "; + assertThrows(MalformedQueryException.class, () -> { + parser.parseQuery(invalidSparqlQuery, null); + }); + + } + + @Test + public void testInvalidConstructQueryWithNegatedPropertySet() { + String invalidSparqlQuery = "PREFIX foaf: " + + "CONSTRUCT {\n" + + " ?s !foaf:knows ?o . " + + "}\n" + + "WHERE {\n" + + " ?s !foaf:knows ?o .\n" + + "} "; + assertThrows(MalformedQueryException.class, () -> { + parser.parseQuery(invalidSparqlQuery, null); + }); + + } + + @Test + public void testInvalidConstructQueryWithZeroOrMorePathInPredicate() { + String invalidSparqlQuery = "PREFIX foaf: " + + "CONSTRUCT {\n" + + " ?s foaf:knows* ?o . " + + "}\n" + + "WHERE {\n" + + " ?s foaf:knows* ?o .\n" + + "} "; + assertThrows(MalformedQueryException.class, () -> { + parser.parseQuery(invalidSparqlQuery, null); + }); + } + + @Test + public void testInvalidConstructQueryWithZeroOrMorePathInPredicates() { + String invalidSparqlQuery = "PREFIX foaf: " + + "CONSTRUCT {\n" + + " ?s foaf:knows ?o . " + + " ?s foaf:name+ ?o . " + + "}\n" + + "WHERE {\n" + + " ?s foaf:knows* ?o .\n" + + " ?s foaf:name ?o .\n" + + "} "; + assertThrows(MalformedQueryException.class, () -> { + parser.parseQuery(invalidSparqlQuery, null); + }); + } + + @Test + public void testInvalidConstructQueryWithMalformedTriple() { + String invalidSparqlQuery = "PREFIX foaf: " + + "CONSTRUCT {\n" + + " ?person foaf:name \n" + + "}\n" + + "WHERE {\n" + + " ?person foaf:name ?name .\n" + + "} "; + assertThrows(MalformedQueryException.class, () -> { + parser.parseQuery(invalidSparqlQuery, null); + }); + } + private AggregateFunctionFactory buildDummyFactory() { return new AggregateFunctionFactory() { @Override