From b19f95495d1df437722e6a0c85ea5ca6f91cdd8b Mon Sep 17 00:00:00 2001 From: Tess Avitabile Date: Mon, 21 Aug 2017 15:31:06 -0400 Subject: SERVER-29840 Add allowed features bitmask to MatchExpressionParser::parse --- src/mongo/db/update/path_support_test.cpp | 5 +- src/mongo/db/update/pull_node.cpp | 9 +-- src/mongo/db/update/pull_node_test.cpp | 112 ++++++++++++++++++++++++++++++ src/mongo/db/update/update_driver.cpp | 8 ++- 4 files changed, 122 insertions(+), 12 deletions(-) (limited to 'src/mongo/db/update') diff --git a/src/mongo/db/update/path_support_test.cpp b/src/mongo/db/update/path_support_test.cpp index 339781699e1..3de5e8eff00 100644 --- a/src/mongo/db/update/path_support_test.cpp +++ b/src/mongo/db/update/path_support_test.cpp @@ -48,7 +48,6 @@ #include "mongo/db/matcher/expression.h" #include "mongo/db/matcher/expression_leaf.h" #include "mongo/db/matcher/expression_parser.h" -#include "mongo/db/matcher/extensions_callback_disallow_extensions.h" #include "mongo/stdx/memory.h" #include "mongo/unittest/unittest.h" #include "mongo/util/mongoutils/str.h" @@ -590,9 +589,7 @@ TEST_F(ArrayDoc, CreatePathAtFailsIfElemFoundIsArrayAndIdxFoundFieldIsNonNumeric static MatchExpression* makeExpr(const BSONObj& exprBSON) { const CollatorInterface* collator = nullptr; - return MatchExpressionParser::parse(exprBSON, ExtensionsCallbackDisallowExtensions(), collator) - .getValue() - .release(); + return MatchExpressionParser::parse(exprBSON, collator).getValue().release(); } static void assertContains(const EqualityMatches& equalities, const BSONObj& wrapped) { diff --git a/src/mongo/db/update/pull_node.cpp b/src/mongo/db/update/pull_node.cpp index 5842718c388..9ef4e02724b 100644 --- a/src/mongo/db/update/pull_node.cpp +++ b/src/mongo/db/update/pull_node.cpp @@ -31,7 +31,6 @@ #include "mongo/db/update/pull_node.h" #include "mongo/db/matcher/copyable_match_expression.h" -#include "mongo/db/matcher/extensions_callback_disallow_extensions.h" #include "mongo/db/query/collation/collator_interface.h" namespace mongo { @@ -43,9 +42,7 @@ namespace mongo { class PullNode::ObjectMatcher final : public PullNode::ElementMatcher { public: ObjectMatcher(BSONObj matchCondition, const CollatorInterface* collator) - : _matchExpr( - matchCondition, stdx::make_unique(), collator) { - } + : _matchExpr(matchCondition, collator) {} std::unique_ptr clone() const final { return stdx::make_unique(*this); @@ -77,9 +74,7 @@ private: class PullNode::WrappedObjectMatcher final : public PullNode::ElementMatcher { public: WrappedObjectMatcher(BSONElement matchCondition, const CollatorInterface* collator) - : _matchExpr(matchCondition.wrap(""), - stdx::make_unique(), - collator) {} + : _matchExpr(matchCondition.wrap(""), collator) {} std::unique_ptr clone() const final { return stdx::make_unique(*this); diff --git a/src/mongo/db/update/pull_node_test.cpp b/src/mongo/db/update/pull_node_test.cpp index d54640c0d1a..0fa4c83dcc0 100644 --- a/src/mongo/db/update/pull_node_test.cpp +++ b/src/mongo/db/update/pull_node_test.cpp @@ -63,6 +63,62 @@ TEST(PullNodeTest, InitWithBadTopLevelOperatorFails) { ASSERT_EQUALS(ErrorCodes::BadValue, status); } +TEST(PullNodeTest, InitWithTextFails) { + auto update = fromjson("{$pull: {a: {$text: {$search: 'str'}}}}"); + const CollatorInterface* collator = nullptr; + PullNode node; + auto status = node.init(update["$pull"]["a"], collator); + ASSERT_NOT_OK(status); + ASSERT_EQUALS(ErrorCodes::BadValue, status); +} + +TEST(PullNodeTest, InitWithWhereFails) { + auto update = fromjson("{$pull: {a: {$where: 'this.a == this.b'}}}"); + const CollatorInterface* collator = nullptr; + PullNode node; + auto status = node.init(update["$pull"]["a"], collator); + ASSERT_NOT_OK(status); + ASSERT_EQUALS(ErrorCodes::BadValue, status); +} + +TEST(PullNodeTest, InitWithGeoNearElemFails) { + auto update = + fromjson("{$pull: {a: {$nearSphere: {$geometry: {type: 'Point', coordinates: [0, 0]}}}}}"); + const CollatorInterface* collator = nullptr; + PullNode node; + auto status = node.init(update["$pull"]["a"], collator); + ASSERT_NOT_OK(status); + ASSERT_EQUALS(ErrorCodes::BadValue, status); +} + +TEST(PullNodeTest, InitWithGeoNearObjectFails) { + auto update = fromjson( + "{$pull: {a: {b: {$nearSphere: {$geometry: {type: 'Point', coordinates: [0, 0]}}}}}}"); + const CollatorInterface* collator = nullptr; + PullNode node; + auto status = node.init(update["$pull"]["a"], collator); + ASSERT_NOT_OK(status); + ASSERT_EQUALS(ErrorCodes::BadValue, status); +} + +TEST(PullNodeTest, InitWithExprElemFails) { + auto update = fromjson("{$pull: {a: {$expr: 5}}}"); + const CollatorInterface* collator = nullptr; + PullNode node; + auto status = node.init(update["$pull"]["a"], collator); + ASSERT_NOT_OK(status); + ASSERT_EQUALS(ErrorCodes::BadValue, status); +} + +TEST(PullNodeTest, InitWithExprObjectFails) { + auto update = fromjson("{$pull: {a: {b: {$expr: 5}}}}"); + const CollatorInterface* collator = nullptr; + PullNode node; + auto status = node.init(update["$pull"]["a"], collator); + ASSERT_NOT_OK(status); + ASSERT_EQUALS(ErrorCodes::BadValue, status); +} + TEST_F(PullNodeTest, TargetNotFound) { auto update = fromjson("{$pull : {a: {$lt: 1}}}"); const CollatorInterface* collator = nullptr; @@ -361,6 +417,62 @@ TEST_F(PullNodeTest, ApplyStringMatchAfterSetCollator) { ASSERT_FALSE(doc2.isInPlaceModeEnabled()); } +TEST_F(PullNodeTest, ApplyElementMatchAfterSetCollator) { + auto update = fromjson("{$pull : {a: {$gte: 'c'}}}"); + PullNode node; + const CollatorInterface* collator = nullptr; + ASSERT_OK(node.init(update["$pull"]["a"], collator)); + + // First without a collator. + mutablebson::Document doc(fromjson("{ a : ['a', 'b', 'c', 'd'] }")); + setPathTaken("a"); + auto result = node.apply(getApplyParams(doc.root()["a"])); + ASSERT_FALSE(result.noop); + ASSERT_FALSE(result.indexesAffected); + ASSERT_EQUALS(fromjson("{a: ['a', 'b']}"), doc); + ASSERT_FALSE(doc.isInPlaceModeEnabled()); + + // Now with a collator. + CollatorInterfaceMock mockCollator(CollatorInterfaceMock::MockType::kAlwaysEqual); + node.setCollator(&mockCollator); + mutablebson::Document doc2(fromjson("{ a : ['a', 'b', 'c', 'd'] }")); + resetApplyParams(); + setPathTaken("a"); + result = node.apply(getApplyParams(doc2.root()["a"])); + ASSERT_FALSE(result.noop); + ASSERT_FALSE(result.indexesAffected); + ASSERT_EQUALS(fromjson("{a: []}"), doc2); + ASSERT_FALSE(doc2.isInPlaceModeEnabled()); +} + +TEST_F(PullNodeTest, ApplyObjectMatchAfterSetCollator) { + auto update = fromjson("{$pull : {a: {b: 'y'}}}"); + PullNode node; + const CollatorInterface* collator = nullptr; + ASSERT_OK(node.init(update["$pull"]["a"], collator)); + + // First without a collator. + mutablebson::Document doc(fromjson("{a : [{b: 'w'}, {b: 'x'}, {b: 'y'}, {b: 'z'}]}")); + setPathTaken("a"); + auto result = node.apply(getApplyParams(doc.root()["a"])); + ASSERT_FALSE(result.noop); + ASSERT_FALSE(result.indexesAffected); + ASSERT_EQUALS(fromjson("{a : [{b: 'w'}, {b: 'x'}, {b: 'z'}]}"), doc); + ASSERT_FALSE(doc.isInPlaceModeEnabled()); + + // Now with a collator. + CollatorInterfaceMock mockCollator(CollatorInterfaceMock::MockType::kAlwaysEqual); + node.setCollator(&mockCollator); + mutablebson::Document doc2(fromjson("{a : [{b: 'w'}, {b: 'x'}, {b: 'y'}, {b: 'z'}]}")); + resetApplyParams(); + setPathTaken("a"); + result = node.apply(getApplyParams(doc2.root()["a"])); + ASSERT_FALSE(result.noop); + ASSERT_FALSE(result.indexesAffected); + ASSERT_EQUALS(fromjson("{a: []}"), doc2); + ASSERT_FALSE(doc2.isInPlaceModeEnabled()); +} + TEST_F(PullNodeTest, SetCollatorDoesNotAffectClone) { auto update = fromjson("{$pull : {a: 'c'}}"); PullNode node; diff --git a/src/mongo/db/update/update_driver.cpp b/src/mongo/db/update/update_driver.cpp index 6d1d89e7f77..52700a13f80 100644 --- a/src/mongo/db/update/update_driver.cpp +++ b/src/mongo/db/update/update_driver.cpp @@ -214,8 +214,14 @@ Status UpdateDriver::populateDocumentWithQueryFields(OperationContext* opCtx, // $where/$text clauses do not make sense, hence empty ExtensionsCallback. auto qr = stdx::make_unique(NamespaceString("")); qr->setFilter(query); + const boost::intrusive_ptr expCtx; auto statusWithCQ = - CanonicalQuery::canonicalize(opCtx, std::move(qr), ExtensionsCallbackNoop()); + CanonicalQuery::canonicalize(opCtx, + std::move(qr), + expCtx, + ExtensionsCallbackNoop(), + MatchExpressionParser::kAllowAllSpecialFeatures & + ~MatchExpressionParser::AllowedFeatures::kExpr); if (!statusWithCQ.isOK()) { return statusWithCQ.getStatus(); } -- cgit v1.2.1