summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorAnand Thakker <anandthakker@users.noreply.github.com>2018-06-06 11:26:08 -0400
committerGitHub <noreply@github.com>2018-06-06 11:26:08 -0400
commit2da57bd1576c99ff4305cab7b6ad8c1a39ac2fa7 (patch)
tree5d2dfb87e69f0354d47a94acf54af40009932a04 /src
parent6eee8729554ed778491c52bda46cdcbec61b9c95 (diff)
downloadqtlocation-mapboxgl-2da57bd1576c99ff4305cab7b6ad8c1a39ac2fa7.tar.gz
[core] Fix crash due to mixing legacy filters and expressions (#12065)
* Fix crash due to mixing legacy filters and expressions In some cases, (invalid) nested filters that used a mix of legacy filter syntax and expression syntax caused a crash due to a failure to propagate parsing errors from deeper within the filter expression. These errors went undetected in part because these conversion functions returned unique_ptr<Expression> values (or vectors thereof), using {nullptr} to represent a parsing error, but the core expression classes expect unique_ptr<Expression> that are never null. This changes over to using expression::ParseResult (aka optional<unique_ptr<Expression>>), to represent conversion failure the same way we do in the rest of the expression system. * Fix clang 3.8 / gcc 4.9 issue
Diffstat (limited to 'src')
-rw-r--r--src/mbgl/style/conversion/filter.cpp84
1 files changed, 51 insertions, 33 deletions
diff --git a/src/mbgl/style/conversion/filter.cpp b/src/mbgl/style/conversion/filter.cpp
index 386d85e921..ad13470bca 100644
--- a/src/mbgl/style/conversion/filter.cpp
+++ b/src/mbgl/style/conversion/filter.cpp
@@ -13,7 +13,7 @@ namespace conversion {
using namespace mbgl::style::expression;
static bool isExpression(const Convertible& filter);
-std::unique_ptr<Expression> convertLegacyFilter(const Convertible& values, Error& error);
+ParseResult convertLegacyFilter(const Convertible& values, Error& error);
optional<Filter> Converter<Filter>::operator()(const Convertible& value, Error& error) const {
if (isExpression(value)) {
@@ -26,9 +26,12 @@ optional<Filter> Converter<Filter>::operator()(const Convertible& value, Error&
return { Filter(std::move(parseResult)) };
}
} else {
- std::unique_ptr<Expression> expression = convertLegacyFilter(value, error);
- if (!expression) return {};
- return Filter(optional<std::unique_ptr<Expression>>(std::move(expression)));
+ ParseResult expression = convertLegacyFilter(value, error);
+ if (!expression) {
+ assert(error.message.size() > 0);
+ return {};
+ }
+ return Filter(optional<std::unique_ptr<Expression>>(std::move(*expression)));
}
}
@@ -68,51 +71,62 @@ bool isExpression(const Convertible& filter) {
}
}
-std::unique_ptr<Expression> createExpression(std::string op, std::vector<std::unique_ptr<Expression>> args, Error& error) {
+ParseResult createExpression(std::string op, optional<std::vector<std::unique_ptr<Expression>>> args, Error& error) {
+ if (!args) return {};
+ assert(std::all_of(args->begin(), args->end(), [](const std::unique_ptr<Expression> &e) {
+ return bool(e.get());
+ }));
+
if (op == "any") {
- return std::make_unique<Any>(std::move(args));
-
+ return {std::make_unique<Any>(std::move(*args))};
} else if (op == "all") {
- return std::make_unique<All>(std::move(args));
-
+ return {std::make_unique<All>(std::move(*args))};
} else {
ParsingContext parsingContext(type::Boolean);
- ParseResult parseResult = createCompoundExpression(op, std::move(args), parsingContext);
+ ParseResult parseResult = createCompoundExpression(op, std::move(*args), parsingContext);
if (!parseResult) {
error = { parsingContext.getCombinedErrors() };
return {};
} else {
- return std::move(*parseResult);
+ return parseResult;
}
}
}
-std::unique_ptr<Expression> createExpression(std::string op, std::unique_ptr<Expression> expression, Error& error) {
+ParseResult createExpression(std::string op, ParseResult arg, Error& error) {
+ if (!arg) {
+ return {};
+ }
+
std::vector<std::unique_ptr<Expression>> args;
- args.push_back(std::move(expression));
+ args.push_back(std::move(*arg));
return createExpression(op, std::move(args), error);
}
-std::unique_ptr<Expression> convertLiteral(const Convertible& convertible, Error& error) {
+ParseResult convertLiteral(const Convertible& convertible, Error& error) {
ParsingContext parsingContext;
ParseResult parseResult = Literal::parse(convertible, parsingContext);
if (parseResult) {
- return std::move(*parseResult);
+ return parseResult;
} else {
error = { parsingContext.getCombinedErrors() };
return {};
}
}
-std::vector<std::unique_ptr<Expression>> convertLiteralArray(const Convertible &input, Error& error, std::size_t startIndex = 0) {
+optional<std::vector<std::unique_ptr<Expression>>> convertLiteralArray(const Convertible &input, Error& error, std::size_t startIndex = 0) {
std::vector<std::unique_ptr<Expression>> output;
for (std::size_t i = startIndex; i < arrayLength(input); i++) {
- output.push_back(convertLiteral(arrayMember(input, i), error));
+ ParseResult literal = convertLiteral(arrayMember(input, i), error);
+ if (!literal) {
+ return {};
+ }
+ output.push_back(std::move(*literal));
}
- return output;
+ return {std::move(output)};
}
-std::unique_ptr<Expression> convertLegacyComparisonFilter(const Convertible& values, Error& error, optional<std::string> opOverride = {}) {
+ParseResult convertLegacyComparisonFilter(const Convertible& values, Error& error, optional<std::string> opOverride = {}) {
optional<std::string> op = opOverride ? opOverride : toString(arrayMember(values, 0));
optional<std::string> property = toString(arrayMember(values, 1));
@@ -128,29 +142,29 @@ std::unique_ptr<Expression> convertLegacyComparisonFilter(const Convertible& val
}
}
-std::unique_ptr<Expression> convertLegacyHasFilter(const Convertible& values, Error& error) {
+ParseResult convertLegacyHasFilter(const Convertible& values, Error& error) {
optional<std::string> property = toString(arrayMember(values, 1));
if (!property) {
error = { "filter property must be a string" };
return {};
} else if (*property == "$type") {
- return std::make_unique<Literal>(true);
+ return {std::make_unique<Literal>(true)};
} else if (*property == "$id") {
return createExpression("filter-has-id", std::vector<std::unique_ptr<Expression>>(), error);
} else {
- return createExpression("filter-has", std::make_unique<Literal>(*property), error);
+ return createExpression("filter-has", {std::make_unique<Literal>(*property)}, error);
}
}
-std::unique_ptr<Expression> convertLegacyInFilter(const Convertible& values, Error& error) {
+ParseResult convertLegacyInFilter(const Convertible& values, Error& error) {
optional<std::string> property = toString(arrayMember(values, 1));
if (!property) {
error = { "filter property must be a string" };
return {};
} else if (arrayLength(values) == 0) {
- return std::make_unique<Literal>(false);
+ return {std::make_unique<Literal>(false)};
} else if (*property == "$type") {
return createExpression("filter-type-in", convertLiteralArray(values, error, 2), error);
} else if (*property == "$id") {
@@ -160,17 +174,21 @@ std::unique_ptr<Expression> convertLegacyInFilter(const Convertible& values, Err
}
}
-std::vector<std::unique_ptr<Expression>> convertLegacyFilterArray(const Convertible &input, Error& error, std::size_t startIndex = 0) {
+optional<std::vector<std::unique_ptr<Expression>>> convertLegacyFilterArray(const Convertible &input, Error& error, std::size_t startIndex = 0) {
std::vector<std::unique_ptr<Expression>> output;
for (std::size_t i = startIndex; i < arrayLength(input); i++) {
- output.push_back(convertLegacyFilter(arrayMember(input, i), error));
+ optional<std::unique_ptr<Expression>> child = convertLegacyFilter(arrayMember(input, i), error);
+ if (!child) {
+ return {};
+ }
+ output.push_back(std::move(*child));
}
- return output;
+ return {std::move(output)};
}
-std::unique_ptr<Expression> convertLegacyFilter(const Convertible& values, Error& error) {
+ParseResult convertLegacyFilter(const Convertible& values, Error& error) {
if (isUndefined(values)) {
- return std::make_unique<Literal>(true);
+ return {std::make_unique<Literal>(true)};
}
optional<std::string> op = toString(arrayMember(values, 0));
@@ -179,9 +197,9 @@ std::unique_ptr<Expression> convertLegacyFilter(const Convertible& values, Error
error = { "filter operator must be a string" };
return {};
} else if (arrayLength(values) <= 1) {
- return std::make_unique<Literal>(*op != "any");
+ return {std::make_unique<Literal>(*op != "any")};
} else {
- return (
+ return {
*op == "==" ||
*op == "<" ||
*op == ">" ||
@@ -195,8 +213,8 @@ std::unique_ptr<Expression> convertLegacyFilter(const Convertible& values, Error
*op == "!in" ? createExpression("!", convertLegacyInFilter(values, error), error) :
*op == "has" ? convertLegacyHasFilter(values, error) :
*op == "!has" ? createExpression("!", convertLegacyHasFilter(values, error), error) :
- std::make_unique<Literal>(true)
- );
+ ParseResult(std::make_unique<Literal>(true))
+ };
}
}