Skip to content

Commit

Permalink
forbid invalid prop expr used in cypher (#5242)
Browse files Browse the repository at this point in the history
  • Loading branch information
jievince authored Jan 12, 2023
1 parent 3c0cce4 commit a025f92
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 45 deletions.
16 changes: 7 additions & 9 deletions src/graph/executor/algo/BFSShortestPathExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
24 changes: 11 additions & 13 deletions src/graph/executor/algo/ProduceAllPathsExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
7 changes: 2 additions & 5 deletions src/graph/executor/maintain/EdgeExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
14 changes: 4 additions & 10 deletions src/graph/executor/maintain/EdgeIndexExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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"
Expand Down
26 changes: 18 additions & 8 deletions src/graph/validator/MatchValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
55 changes: 55 additions & 0 deletions tests/tck/features/match/Base.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit a025f92

Please sign in to comment.