Disallow traversal patterns outside of the appropriate clauses (#1431)

* Disallow traversal patterns outside of the appropriate clauses

* Address PR comments

Co-authored-by: Roi Lipman <swilly22@users.noreply.github.com>

src/ast/ast_validations.c

@@ -299,6 +299,32 @@ static AST_Validation _ValidateFunctionCalls(const cypher_astnode_t *node,
 	return res;
 }
 
+/* While Cypher allows paths to appear in a number of places, RedisGraph
+ * only supports them in the appropriate clauses and in path filters. */
+static AST_Validation _Validate_Path_Locations(const cypher_astnode_t *root) {
+	uint nchildren = cypher_astnode_nchildren(root);
+	for(uint i = 0; i < nchildren; i ++) {
+		const cypher_astnode_t *child = cypher_astnode_get_child(root, i);
+		const cypher_astnode_type_t child_type = cypher_astnode_type(child);
+		if(child_type == CYPHER_AST_PATTERN_PATH) {
+			const cypher_astnode_type_t root_type = cypher_astnode_type(root);
+			if(root_type != CYPHER_AST_PATTERN &&
+					root_type != CYPHER_AST_MATCH &&
+					root_type != CYPHER_AST_MERGE &&
+					root_type != CYPHER_AST_WITH &&
+					root_type != CYPHER_AST_NAMED_PATH &&
+					root_type != CYPHER_AST_UNARY_OPERATOR &&
+					root_type != CYPHER_AST_BINARY_OPERATOR) {
+				QueryCtx_SetError("Encountered path traversal in unsupported location '%s'",
+						cypher_astnode_typestr(child_type));
+				return AST_INVALID;
+			}
+		}
+		if (_Validate_Path_Locations(child) != AST_VALID) return AST_INVALID;
+	}
+	return AST_VALID;
+}
+
 static inline bool _AliasIsReturned(rax *projections, const char *identifier) {
 	return raxFind(projections, (unsigned char *)identifier, strlen(identifier)) != raxNotFound;
 }
@@ -1672,6 +1698,9 @@ AST_Validation AST_Validate_Query(const cypher_parse_result_t *result) {
 	AST mock_ast; // Build a fake AST with the correct AST root
 	mock_ast.root = body;
 
+	// Check for path traversals in unsupported locations.
+	if(_Validate_Path_Locations(mock_ast.root) != AST_VALID) return AST_INVALID;
+
 	// Check for invalid queries not captured by libcypher-parser
 	AST_Validation res;
 	if(AST_ContainsClause(&mock_ast, CYPHER_AST_UNION)) {

tests/flow/test_query_validation.py

@@ -418,7 +418,6 @@ def test29_invalid_filter_non_boolean_constant(self):
             redis_graph.query(query)
             assert(False)
         except redis.exceptions.ResponseError as e:
-            # Expecting an error.
             assert("expected Boolean but was Node" in e.message)
             pass
 
@@ -427,7 +426,6 @@ def test29_invalid_filter_non_boolean_constant(self):
             redis_graph.query(query)
             assert(False)
         except redis.exceptions.ResponseError as e:
-            # Expecting an error.
             assert("expected Boolean but was Float" in e.message)
             pass
 
@@ -436,7 +434,6 @@ def test29_invalid_filter_non_boolean_constant(self):
             redis_graph.query(query)
             assert(False)
         except redis.exceptions.ResponseError as e:
-            # Expecting an error.
             assert("expected Boolean but was Integer" in e.message)
             pass
 
@@ -448,6 +445,19 @@ def test29_invalid_filter_non_boolean_constant(self):
         query = """MATCH (a) WHERE a.fakeprop RETURN a"""
         redis_graph.query(query)
 
+    # Encountering traversals as property values or ORDER BY expressions should raise compile-time errors.
+    def test30_unexpected_traversals(self):
+        queries = ["""MATCH (a {prop: ()-[]->()}) RETURN a""",
+                   """MATCH (a) RETURN a ORDER BY (a)-[]->()""",
+                   """MATCH (a) RETURN (a)-[]->()"""]
+        for query in queries:
+            try:
+                redis_graph.query(query)
+                assert(False)
+            except redis.exceptions.ResponseError as e:
+                # Expecting an error.
+                assert("Encountered path traversal" in e.message)
+
     def test31_set_invalid_property_type(self):
         queries = ["""MATCH (a) CREATE (:L {v: a})""",
                    """MATCH (a), (b) WHERE b.age IS NOT NULL SET b.age = a"""]