From a025f927e1d8e5983603a57edc47950d1b5b628d Mon Sep 17 00:00:00 2001 From: "jie.wang" <38901892+jievince@users.noreply.github.com> Date: Thu, 12 Jan 2023 13:59:56 +0800 Subject: [PATCH] forbid invalid prop expr used in cypher (#5242) --- .../executor/algo/BFSShortestPathExecutor.cpp | 16 +++--- .../executor/algo/ProduceAllPathsExecutor.cpp | 24 ++++---- src/graph/executor/maintain/EdgeExecutor.cpp | 7 +-- .../executor/maintain/EdgeIndexExecutor.cpp | 14 ++--- src/graph/validator/MatchValidator.cpp | 26 ++++++--- tests/tck/features/match/Base.feature | 55 +++++++++++++++++++ 6 files changed, 97 insertions(+), 45 deletions(-) diff --git a/src/graph/executor/algo/BFSShortestPathExecutor.cpp b/src/graph/executor/algo/BFSShortestPathExecutor.cpp index cf9faafa0d9..098ad840df5 100644 --- a/src/graph/executor/algo/BFSShortestPathExecutor.cpp +++ b/src/graph/executor/algo/BFSShortestPathExecutor.cpp @@ -167,15 +167,13 @@ folly::Future 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& meetVids, diff --git a/src/graph/executor/algo/ProduceAllPathsExecutor.cpp b/src/graph/executor/algo/ProduceAllPathsExecutor.cpp index f2b709a000f..0011caa5638 100644 --- a/src/graph/executor/algo/ProduceAllPathsExecutor.cpp +++ b/src/graph/executor/algo/ProduceAllPathsExecutor.cpp @@ -194,19 +194,17 @@ folly::Future 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 index 1c8a3151f44..d790e085f1d 100644 --- a/src/graph/executor/maintain/EdgeExecutor.cpp +++ b/src/graph/executor/maintain/EdgeExecutor.cpp @@ -86,11 +86,8 @@ folly::Future ShowEdgesExecutor::execute() { SCOPED_TIMER(&execTime_); auto spaceId = qctx()->rctx()->session()->space().id; - return qctx() - ->getMetaClient() - ->listEdgeSchemas(spaceId) - .via(runner()) - .thenValue([this, spaceId](StatusOr> resp) { + return qctx()->getMetaClient()->listEdgeSchemas(spaceId).via(runner()).thenValue( + [this, spaceId](StatusOr> 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 index 00f1f2e9d20..8fc1da1114a 100644 --- a/src/graph/executor/maintain/EdgeIndexExecutor.cpp +++ b/src/graph/executor/maintain/EdgeIndexExecutor.cpp @@ -118,11 +118,8 @@ folly::Future ShowEdgeIndexesExecutor::execute() { auto *iNode = asNode(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> resp) { + return qctx()->getMetaClient()->listEdgeIndexes(spaceId).via(runner()).thenValue( + [this, spaceId, bySchema](StatusOr> resp) { memory::MemoryCheckGuard guard; if (!resp.ok()) { LOG(WARNING) << "SpaceId: " << spaceId << ", Show edge indexes failed" << resp.status(); @@ -174,11 +171,8 @@ folly::Future ShowEdgeIndexStatusExecutor::execute() { SCOPED_TIMER(&execTime_); auto spaceId = qctx()->rctx()->session()->space().id; - return qctx() - ->getMetaClient() - ->listEdgeIndexStatus(spaceId) - .via(runner()) - .thenValue([this, spaceId](StatusOr> resp) { + return qctx()->getMetaClient()->listEdgeIndexStatus(spaceId).via(runner()).thenValue( + [this, spaceId](StatusOr> 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 index 8c3e0ed7dd5..18d6f09754f 100644 --- 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 &exprs, const std::unordered_map &aliasesAvailable) const { - static const std::unordered_set 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 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 index f8c0f8c07b1..9355b90eb8a 100644 --- 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: