Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

forbid invalid prop expr used in cypher #5242

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
jievince marked this conversation as resolved.
Show resolved Hide resolved
"""
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