forbid invalid prop expr used in cypher (#5242)

vesoft-inc · Jan 12, 2023 · a025f92 · a025f92
1 parent 3c0cce4
commit a025f92
Show file tree

Hide file tree

Showing 6 changed files with 97 additions and 45 deletions.
diff --git a/src/graph/executor/algo/BFSShortestPathExecutor.cpp b/src/graph/executor/algo/BFSShortestPathExecutor.cpp
@@ -167,15 +167,13 @@ folly::Future<Status> BFSShortestPathExecutor::conjunctPath() {
     }
   }
 
-  return folly::collect(futures)
-      .via(runner())
-      .thenValue([this](auto&& resps) {
-        memory::MemoryCheckGuard guard;
-        for (auto& resp : resps) {
-          currentDs_.append(std::move(resp));
-        }
-        return Status::OK();
-      });
+  return folly::collect(futures).via(runner()).thenValue([this](auto&& resps) {
+    memory::MemoryCheckGuard guard;
+    for (auto& resp : resps) {
+      currentDs_.append(std::move(resp));
+    }
+    return Status::OK();
+  });
 }
 
 DataSet BFSShortestPathExecutor::doConjunct(const std::vector<Value>& meetVids,

diff --git a/src/graph/executor/algo/ProduceAllPathsExecutor.cpp b/src/graph/executor/algo/ProduceAllPathsExecutor.cpp
@@ -194,19 +194,17 @@ folly::Future<Status> ProduceAllPathsExecutor::conjunctPath() {
     }
   }
 
-  return folly::collect(futures)
-      .via(runner())
-      .thenValue([this](auto&& resps) {
-        memory::MemoryCheckGuard guard;
-        for (auto& resp : resps) {
-          currentDs_.append(std::move(resp));
-        }
-        preLeftPaths_.swap(leftPaths_);
-        preRightPaths_.swap(rightPaths_);
-        leftPaths_.clear();
-        rightPaths_.clear();
-        return Status::OK();
-      });
+  return folly::collect(futures).via(runner()).thenValue([this](auto&& resps) {
+    memory::MemoryCheckGuard guard;
+    for (auto& resp : resps) {
+      currentDs_.append(std::move(resp));
+    }
+    preLeftPaths_.swap(leftPaths_);
+    preRightPaths_.swap(rightPaths_);
+    leftPaths_.clear();
+    rightPaths_.clear();
+    return Status::OK();
+  });
 }
 
 void ProduceAllPathsExecutor::setNextStepVid(Interims& paths, const string& var) {

diff --git a/src/graph/executor/maintain/EdgeExecutor.cpp b/src/graph/executor/maintain/EdgeExecutor.cpp
@@ -86,11 +86,8 @@ folly::Future<Status> ShowEdgesExecutor::execute() {
   SCOPED_TIMER(&execTime_);
 
   auto spaceId = qctx()->rctx()->session()->space().id;
-  return qctx()
-      ->getMetaClient()
-      ->listEdgeSchemas(spaceId)
-      .via(runner())
-      .thenValue([this, spaceId](StatusOr<std::vector<meta::cpp2::EdgeItem>> resp) {
+  return qctx()->getMetaClient()->listEdgeSchemas(spaceId).via(runner()).thenValue(
+      [this, spaceId](StatusOr<std::vector<meta::cpp2::EdgeItem>> resp) {
         memory::MemoryCheckGuard guard;
         if (!resp.ok()) {
           LOG(WARNING) << "SpaceId: " << spaceId << ", Show edges failed: " << resp.status();

diff --git a/src/graph/executor/maintain/EdgeIndexExecutor.cpp b/src/graph/executor/maintain/EdgeIndexExecutor.cpp
@@ -118,11 +118,8 @@ folly::Future<Status> ShowEdgeIndexesExecutor::execute() {
   auto *iNode = asNode<ShowEdgeIndexes>(node());
   const auto &bySchema = iNode->name();
   auto spaceId = qctx()->rctx()->session()->space().id;
-  return qctx()
-      ->getMetaClient()
-      ->listEdgeIndexes(spaceId)
-      .via(runner())
-      .thenValue([this, spaceId, bySchema](StatusOr<std::vector<meta::cpp2::IndexItem>> resp) {
+  return qctx()->getMetaClient()->listEdgeIndexes(spaceId).via(runner()).thenValue(
+      [this, spaceId, bySchema](StatusOr<std::vector<meta::cpp2::IndexItem>> resp) {
         memory::MemoryCheckGuard guard;
         if (!resp.ok()) {
           LOG(WARNING) << "SpaceId: " << spaceId << ", Show edge indexes failed" << resp.status();
@@ -174,11 +171,8 @@ folly::Future<Status> ShowEdgeIndexStatusExecutor::execute() {
   SCOPED_TIMER(&execTime_);
 
   auto spaceId = qctx()->rctx()->session()->space().id;
-  return qctx()
-      ->getMetaClient()
-      ->listEdgeIndexStatus(spaceId)
-      .via(runner())
-      .thenValue([this, spaceId](StatusOr<std::vector<meta::cpp2::IndexStatus>> resp) {
+  return qctx()->getMetaClient()->listEdgeIndexStatus(spaceId).via(runner()).thenValue(
+      [this, spaceId](StatusOr<std::vector<meta::cpp2::IndexStatus>> resp) {
         memory::MemoryCheckGuard guard;
         if (!resp.ok()) {
           LOG(WARNING) << "SpaceId: " << spaceId << ", Show edge index status failed"

diff --git a/src/graph/validator/MatchValidator.cpp b/src/graph/validator/MatchValidator.cpp
@@ -509,14 +509,19 @@ Status MatchValidator::validateReturn(MatchReturn *ret,
 Status MatchValidator::validateAliases(
     const std::vector<const Expression *> &exprs,
     const std::unordered_map<std::string, AliasType> &aliasesAvailable) const {
-  static const std::unordered_set<Expression::Kind> kinds = {Expression::Kind::kLabel,
-                                                             Expression::Kind::kLabelAttribute,
-                                                             Expression::Kind::kLabelTagProperty,
-                                                             // primitive props
-                                                             Expression::Kind::kEdgeSrc,
-                                                             Expression::Kind::kEdgeDst,
-                                                             Expression::Kind::kEdgeRank,
-                                                             Expression::Kind::kEdgeType};
+  static const std::unordered_set<Expression::Kind> kinds = {
+      Expression::Kind::kLabel,
+      Expression::Kind::kLabelAttribute,
+      Expression::Kind::kLabelTagProperty,
+      // primitive props
+      Expression::Kind::kEdgeSrc,
+      Expression::Kind::kEdgeDst,
+      Expression::Kind::kEdgeRank,
+      Expression::Kind::kEdgeType,
+      // invalid prop exprs
+      Expression::Kind::kSrcProperty,
+      Expression::Kind::kDstProperty,
+  };
 
   for (auto *expr : exprs) {
     auto refExprs = ExpressionUtils::collectAll(expr, kinds);
@@ -1058,6 +1063,11 @@ Status MatchValidator::checkAlias(
                                        name.c_str());
       }
     }
+    case Expression::Kind::kSrcProperty:
+    case Expression::Kind::kDstProperty: {
+      return Status::SemanticError("Expression %s is not allowed to use in cypher",
+                                   refExpr->toString().c_str());
+    }
     default:  // refExpr must satisfy one of cases and should never hit this branch
       break;
   }

diff --git a/tests/tck/features/match/Base.feature b/tests/tck/features/match/Base.feature
@@ -1072,6 +1072,61 @@ Feature: Basic match
       MATCH (v{name: "Tim Duncan"}) return v
       """
     Then a SemanticError should be raised at runtime: `name:"Tim Duncan"': No tag found for property.
+    When executing query:
+      """
+      MATCH (v:player) RETURN $$.player.age AS a
+      """
+    Then a SemanticError should be raised at runtime: Expression $$.player.age is not allowed to use in cypher
+    When executing query:
+      """
+      MATCH (v:player) RETURN $^.player.age AS a
+      """
+    Then a SemanticError should be raised at runtime: Expression $^.player.age is not allowed to use in cypher
+    When executing query:
+      """
+      MATCH (v:player) RETURN $-.player.age AS a
+      """
+    Then a SemanticError should be raised at runtime: `$-.player', not exist prop `player'
+    When executing query:
+      """
+      MATCH (v:player) WHERE $var.player > 0 RETURN v
+      """
+    Then a SemanticError should be raised at runtime: `$var.player', not exist variable `var'
+    When executing query:
+      """
+      MATCH (v:player) WHERE $$.player.age > 0 RETURN v
+      """
+    Then a SemanticError should be raised at runtime: Expression $$.player.age is not allowed to use in cypher
+    When executing query:
+      """
+      MATCH (v:player) WHERE $^.player.age > 0 RETURN v
+      """
+    Then a SemanticError should be raised at runtime: Expression $^.player.age is not allowed to use in cypher
+    When executing query:
+      """
+      MATCH (v:player) WHERE $-.player.age > 0 RETURN v
+      """
+    Then a SemanticError should be raised at runtime: `$-.player', not exist prop `player'
+    When executing query:
+      """
+      MATCH (v:player) WHERE $var.player > 0 RETURN v
+      """
+    Then a SemanticError should be raised at runtime: `$var.player', not exist variable `var'
+    When executing query:
+      """
+      MATCH (v:player) WITH {k: $^} AS x RETUR x.k.player.name
+      """
+    Then a SyntaxError should be raised at runtime: syntax error near `} AS x R'
+    When executing query:
+      """
+      MATCH (v:player) WITH {k: $^.player.age} AS x RETURN x.k
+      """
+    Then a SemanticError should be raised at runtime: Expression $^.player.age is not allowed to use in cypher
+    When executing query:
+      """
+      MATCH (v:player) WITH {k: $var} AS x RETUR x.k.player.name
+      """
+    Then a SyntaxError should be raised at runtime: Direct output of variable is prohibited near `{k: $var}'
 
   Scenario: match with tag filter
     When executing query: