Skip to content

Commit e2bf139

Browse files
linnaunglinnaung
committed
GH-5314: Add property paths validation check for construct query
1 parent 1e175b6 commit e2bf139

2 files changed

Lines changed: 204 additions & 10 deletions

File tree

‎core/queryparser/sparql/src/main/java/org/eclipse/rdf4j/query/parser/sparql/TupleExprBuilder.java‎

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2015 Eclipse RDF4J contributors, Aduna, and others.
2+
* Copyright (c) 2025 Eclipse RDF4J contributors.
33
*
44
* All rights reserved. This program and the accompanying materials
55
* are made available under the terms of the Eclipse Distribution License v1.0
@@ -180,6 +180,7 @@
180180
import org.eclipse.rdf4j.query.parser.sparql.ast.ASTPathAlternative;
181181
import org.eclipse.rdf4j.query.parser.sparql.ast.ASTPathElt;
182182
import org.eclipse.rdf4j.query.parser.sparql.ast.ASTPathMod;
183+
import org.eclipse.rdf4j.query.parser.sparql.ast.ASTPathNegatedPropertySet;
183184
import org.eclipse.rdf4j.query.parser.sparql.ast.ASTPathOneInPropertySet;
184185
import org.eclipse.rdf4j.query.parser.sparql.ast.ASTPathSequence;
185186
import org.eclipse.rdf4j.query.parser.sparql.ast.ASTProjectionElem;
@@ -251,9 +252,6 @@ public class TupleExprBuilder extends AbstractASTVisitor {
251252

252253
GraphPattern graphPattern = new GraphPattern();
253254

254-
// private Map<ValueConstant, Var> mappedValueConstants = new
255-
// HashMap<ValueConstant, Var>();
256-
257255
/*--------------*
258256
* Constructors *
259257
*--------------*/
@@ -820,6 +818,12 @@ public TupleExpr visit(ASTConstructQuery node, Object data) throws VisitorExcept
820818

821819
@Override
822820
public TupleExpr visit(ASTConstruct node, Object data) throws VisitorException {
821+
822+
// check if the construct template contains any invalid nodes
823+
if (isInvalidConstructQueryTemplate(node, true)) {
824+
throw new MalformedQueryException("Invalid construct caluse.");
825+
}
826+
823827
TupleExpr result = (TupleExpr) data;
824828

825829
// Collect construct triples
@@ -872,7 +876,7 @@ public TupleExpr visit(ASTConstruct node, Object data) throws VisitorException {
872876
// assign non-anonymous vars not present in where clause as
873877
// extension elements. This is necessary to make external
874878
// binding
875-
// assingnment possible (see SES-996)
879+
// assignment possible (see SES-996)
876880
extElemMap.put(var, new ExtensionElem(var.clone(), var.getName()));
877881
}
878882
}
@@ -910,6 +914,41 @@ public TupleExpr visit(ASTConstruct node, Object data) throws VisitorException {
910914
return new Reduced(result);
911915
}
912916

917+
/**
918+
* Checks if the given node is invalid in a CONSTRUCT template.
919+
*
920+
* @param node The node to check.
921+
* @param isInConstructTemplate Indicates if the check is being performed within a CONSTRUCT clause.
922+
* @return true if the node is invalid, false otherwise.
923+
*/
924+
private static boolean isInvalidConstructQueryTemplate(Node node, boolean isInConstructTemplate) {
925+
if (isInConstructTemplate && isInvalidConstructNode(node)) {
926+
return true;
927+
}
928+
929+
// recursively check child nodes
930+
for (int i = 0; i < node.jjtGetNumChildren(); i++) {
931+
if (isInvalidConstructQueryTemplate(node.jjtGetChild(i), isInConstructTemplate)) {
932+
return true;
933+
}
934+
}
935+
936+
return false;
937+
}
938+
939+
/**
940+
* Checks if the given node is an invalid construct node.
941+
*
942+
* @param node The node to check.
943+
* @return true if the node is invalid, false otherwise.
944+
*/
945+
private static boolean isInvalidConstructNode(Node node) {
946+
return node instanceof ASTPathMod
947+
|| node instanceof ASTPathNegatedPropertySet
948+
|| node instanceof ASTPathOneInPropertySet
949+
|| (node instanceof ASTPathAlternative && node.jjtGetNumChildren() > 1);
950+
}
951+
913952
/**
914953
* Gets the set of variables that are relevant for the constructor. This method accumulates all subject, predicate
915954
* 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
13591398
}
13601399
}
13611400

1362-
// when using union to execute path expressions, the scope does not not change
1401+
// when using union to execute path expressions, the scope does not change
13631402
union.setVariableScopeChange(false);
13641403
return union;
13651404
}
@@ -1548,7 +1587,7 @@ private TupleExpr createTupleExprForNegatedPropertySets(List<PropertySetElem> np
15481587

15491588
TupleExpr patternMatchInverse = null;
15501589

1551-
// build a inverse statement pattern if needed
1590+
// build an inverse statement pattern if needed
15521591
if (filterConditionInverse != null) {
15531592
patternMatchInverse = new StatementPattern(pathSequenceContext.scope, endVar.clone(), predVar.clone(),
15541593
subjVar.clone(),
@@ -2362,7 +2401,7 @@ public Object visit(ASTBind node, Object data) throws VisitorException {
23622401
ValueExpr ve = child0 instanceof ValueExpr ? (ValueExpr) child0
23632402
: (child0 instanceof TripleRef) ? ((TripleRef) child0).getExprVar() : null;
23642403
if (ve == null) {
2365-
throw new IllegalArgumentException("Unexpected expressin on bind");
2404+
throw new IllegalArgumentException("Unexpected expression on bind");
23662405
}
23672406

23682407
// name to bind the expression outcome to
@@ -2381,7 +2420,7 @@ public Object visit(ASTBind node, Object data) throws VisitorException {
23812420

23822421
// check if alias is not previously used in the BGP
23832422
if (arg.getBindingNames().contains(alias)) {
2384-
// SES-2314 we need to doublecheck that the reused varname is not just
2423+
// SES-2314 we need to double-check that the reused varname is not just
23852424
// for an anonymous var or a constant.
23862425
VarCollector collector = new VarCollector();
23872426
arg.visit(collector);

‎core/queryparser/sparql/src/test/java/org/eclipse/rdf4j/query/parser/sparql/SPARQLParserTest.java‎

Lines changed: 156 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2015 Eclipse RDF4J contributors, Aduna, and others.
2+
* Copyright (c) 2025 Eclipse RDF4J contributors.
33
*
44
* All rights reserved. This program and the accompanying materials
55
* are made available under the terms of the Eclipse Distribution License v1.0
@@ -43,6 +43,7 @@
4343
import org.eclipse.rdf4j.model.util.Values;
4444
import org.eclipse.rdf4j.query.BindingSet;
4545
import org.eclipse.rdf4j.query.MalformedQueryException;
46+
import org.eclipse.rdf4j.query.QueryLanguage;
4647
import org.eclipse.rdf4j.query.algebra.AggregateFunctionCall;
4748
import org.eclipse.rdf4j.query.algebra.ArbitraryLengthPath;
4849
import org.eclipse.rdf4j.query.algebra.DeleteData;
@@ -70,6 +71,8 @@
7071
import org.eclipse.rdf4j.query.parser.ParsedQuery;
7172
import org.eclipse.rdf4j.query.parser.ParsedTupleQuery;
7273
import org.eclipse.rdf4j.query.parser.ParsedUpdate;
74+
import org.eclipse.rdf4j.query.parser.QueryParserUtil;
75+
import org.eclipse.rdf4j.query.parser.sparql.SPARQLParser;
7376
import org.eclipse.rdf4j.query.parser.sparql.aggregate.AggregateCollector;
7477
import org.eclipse.rdf4j.query.parser.sparql.aggregate.AggregateFunction;
7578
import org.eclipse.rdf4j.query.parser.sparql.aggregate.AggregateFunctionFactory;
@@ -1002,6 +1005,158 @@ public void testApostropheInsertData() {
10021005
parser.parseUpdate(query, null);
10031006
}
10041007

1008+
@Test
1009+
public void testInvalidConstructQueryWithPropertyPathInConstructClause() {
1010+
String invalidSparqlQuery = " CONSTRUCT {\n" +
1011+
" ?s (!a)* ?p .\n" +
1012+
" }\n" +
1013+
" WHERE {\n" +
1014+
" ?s (!a)* ?p .\n" +
1015+
" }";
1016+
1017+
assertThrows(MalformedQueryException.class, () -> {
1018+
parser.parseQuery(invalidSparqlQuery, null);
1019+
});
1020+
1021+
}
1022+
1023+
@Test
1024+
public void testValidConstructQueryWithPropertyPathInWhereClause() {
1025+
String validSparqlQuery = "PREFIX foaf: <http://xmlns.com/foaf/0.1/> " +
1026+
"CONSTRUCT {\n" +
1027+
" ?person foaf:knows ?friend .\n" +
1028+
"}\n" +
1029+
"WHERE {\n" +
1030+
" ?person foaf:knows+ ?friend .\n" +
1031+
"}";
1032+
1033+
ParsedQuery parsedQuery = parser.parseQuery(validSparqlQuery, null);
1034+
assertThat(parsedQuery.getSourceString()).isEqualTo(validSparqlQuery);
1035+
1036+
}
1037+
1038+
@Test
1039+
public void testValidConstructQueryWithBlankNodeAsSubject() {
1040+
String validSparqlQuery = "PREFIX foaf: <http://xmlns.com/foaf/0.1/>\n"
1041+
+ "PREFIX site: <http://example.org/stats#>\n"
1042+
+ "\n"
1043+
+ "CONSTRUCT { [] foaf:name ?name }\n"
1044+
+ "WHERE\n"
1045+
+ "{ [] foaf:name ?name ;\n"
1046+
+ " site:hits ?hits .\n"
1047+
+ "}\n"
1048+
+ "ORDER BY desc(?hits)\n"
1049+
+ "LIMIT 2";
1050+
1051+
ParsedQuery parsedQuery = parser.parseQuery(validSparqlQuery, null);
1052+
assertThat(parsedQuery.getSourceString()).isEqualTo(validSparqlQuery);
1053+
1054+
}
1055+
1056+
@Test
1057+
public void testInvalidConstructQueryWithPropertyPathInPredicatePosition() {
1058+
String invalidSparqlQuery = "PREFIX foaf: <http://xmlns.com/foaf/0.1/> " +
1059+
"CONSTRUCT {\n" +
1060+
" ?person foaf:knows+ ?friend . " +
1061+
"}\n" +
1062+
"WHERE {\n" +
1063+
" ?person foaf:knows+ ?friend .\n" +
1064+
"}";
1065+
assertThrows(MalformedQueryException.class, () -> {
1066+
parser.parseQuery(invalidSparqlQuery, null);
1067+
});
1068+
1069+
}
1070+
1071+
@Test
1072+
public void testInvalidConstructQueryWithPropertyPathAlternationInPredicate() {
1073+
String invalidSparqlQuery = "PREFIX foaf: <http://xmlns.com/foaf/0.1/> " +
1074+
"CONSTRUCT {\n" +
1075+
" ?x (foaf:knows|foaf:friendOf) ?y . " +
1076+
"}\n" +
1077+
"WHERE {\n" +
1078+
" ?x (foaf:knows|foaf:friendOf) ?y .\n" +
1079+
"} ";
1080+
assertThrows(MalformedQueryException.class, () -> {
1081+
parser.parseQuery(invalidSparqlQuery, null);
1082+
});
1083+
1084+
}
1085+
1086+
@Test
1087+
public void testInvalidConstructQueryWithPathSequenceInPredicate() {
1088+
String invalidSparqlQuery = "PREFIX foaf: <http://xmlns.com/foaf/0.1/> " +
1089+
"CONSTRUCT {\n" +
1090+
" ?a foaf:knows/foaf:knows ?c . " +
1091+
"}\n" +
1092+
"WHERE {\n" +
1093+
" ?a foaf:knows/foaf:knows ?c . .\n" +
1094+
"} ";
1095+
assertThrows(MalformedQueryException.class, () -> {
1096+
parser.parseQuery(invalidSparqlQuery, null);
1097+
});
1098+
1099+
}
1100+
1101+
@Test
1102+
public void testInvalidConstructQueryWithNegatedPropertySet() {
1103+
String invalidSparqlQuery = "PREFIX foaf: <http://xmlns.com/foaf/0.1/> " +
1104+
"CONSTRUCT {\n" +
1105+
" ?s !foaf:knows ?o . " +
1106+
"}\n" +
1107+
"WHERE {\n" +
1108+
" ?s !foaf:knows ?o .\n" +
1109+
"} ";
1110+
assertThrows(MalformedQueryException.class, () -> {
1111+
parser.parseQuery(invalidSparqlQuery, null);
1112+
});
1113+
1114+
}
1115+
1116+
@Test
1117+
public void testInvalidConstructQueryWithZeroOrMorePathInPredicate() {
1118+
String invalidSparqlQuery = "PREFIX foaf: <http://xmlns.com/foaf/0.1/> " +
1119+
"CONSTRUCT {\n" +
1120+
" ?s foaf:knows* ?o . " +
1121+
"}\n" +
1122+
"WHERE {\n" +
1123+
" ?s foaf:knows* ?o .\n" +
1124+
"} ";
1125+
assertThrows(MalformedQueryException.class, () -> {
1126+
parser.parseQuery(invalidSparqlQuery, null);
1127+
});
1128+
}
1129+
1130+
@Test
1131+
public void testInvalidConstructQueryWithZeroOrMorePathInPredicates() {
1132+
String invalidSparqlQuery = "PREFIX foaf: <http://xmlns.com/foaf/0.1/> " +
1133+
"CONSTRUCT {\n" +
1134+
" ?s foaf:knows ?o . " +
1135+
" ?s foaf:name+ ?o . " +
1136+
"}\n" +
1137+
"WHERE {\n" +
1138+
" ?s foaf:knows* ?o .\n" +
1139+
" ?s foaf:name ?o .\n" +
1140+
"} ";
1141+
assertThrows(MalformedQueryException.class, () -> {
1142+
parser.parseQuery(invalidSparqlQuery, null);
1143+
});
1144+
}
1145+
1146+
@Test
1147+
public void testInvalidConstructQueryWithMalformedTriple() {
1148+
String invalidSparqlQuery = "PREFIX foaf: <http://xmlns.com/foaf/0.1/> " +
1149+
"CONSTRUCT {\n" +
1150+
" ?person foaf:name \n" +
1151+
"}\n" +
1152+
"WHERE {\n" +
1153+
" ?person foaf:name ?name .\n" +
1154+
"} ";
1155+
assertThrows(MalformedQueryException.class, () -> {
1156+
parser.parseQuery(invalidSparqlQuery, null);
1157+
});
1158+
}
1159+
10051160
private AggregateFunctionFactory buildDummyFactory() {
10061161
return new AggregateFunctionFactory() {
10071162
@Override

0 commit comments

Comments
 (0)