SERVER-23664 $mod match expression should raise an error when the remainder is not a number

2 files changed, 19 insertions, 24 deletions

diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp
index fac9967cf2e..327d00262fe 100644
--- a/src/mongo/db/matcher/expression_parser.cpp
+++ b/src/mongo/db/matcher/expression_parser.cpp
@@ -432,28 +432,28 @@ StatusWithMatchExpression parseExpr(StringData name,
     return {std::make_unique<ExprMatchExpression>(std::move(elem), expCtx)};
 }
 
-StatusWithMatchExpression parseMOD(StringData name, BSONElement e) {
-    if (e.type() != BSONType::Array)
+StatusWithMatchExpression parseMOD(StringData name, BSONElement elem) {
+    if (elem.type() != BSONType::Array)
         return {Status(ErrorCodes::BadValue, "malformed mod, needs to be an array")};
 
-    BSONObjIterator i(e.Obj());
+    BSONObjIterator iter(elem.Obj());
 
-    if (!i.more())
+    if (!iter.more())
         return {Status(ErrorCodes::BadValue, "malformed mod, not enough elements")};
-    auto d = i.next();
-    if (!d.isNumber())
+    auto divisor = iter.next();
+    if (!divisor.isNumber())
         return {Status(ErrorCodes::BadValue, "malformed mod, divisor not a number")};
 
-    if (!i.more())
+    if (!iter.more())
         return {Status(ErrorCodes::BadValue, "malformed mod, not enough elements")};
-    auto r = i.next();
-    if (!d.isNumber())
+    auto remainder = iter.next();
+    if (!remainder.isNumber())
         return {Status(ErrorCodes::BadValue, "malformed mod, remainder not a number")};
 
-    if (i.more())
+    if (iter.more())
         return {Status(ErrorCodes::BadValue, "malformed mod, too many elements")};
 
-    return {std::make_unique<ModMatchExpression>(name, d.numberInt(), r.numberInt())};
+    return {std::make_unique<ModMatchExpression>(name, divisor.numberInt(), remainder.numberInt())};
 }
 
 StatusWithMatchExpression parseRegexDocument(StringData name, const BSONObj& doc) {
diff --git a/src/mongo/db/matcher/expression_parser_leaf_test.cpp b/src/mongo/db/matcher/expression_parser_leaf_test.cpp
index ad76f13ff92..b14ba97988d 100644
--- a/src/mongo/db/matcher/expression_parser_leaf_test.cpp
+++ b/src/mongo/db/matcher/expression_parser_leaf_test.cpp
@@ -322,6 +322,14 @@ TEST(MatchExpressionParserLeafTest, SimpleModBad1) {
     query = BSON("x" << BSON("$mod" << BSON("a" << 1 << "b" << 2)));
     result = MatchExpressionParser::parse(query, expCtx);
     ASSERT_NOT_OK(result.getStatus());
+
+    query = BSON("x" << BSON("$mod" << BSON_ARRAY(5 << "r")));
+    result = MatchExpressionParser::parse(query, expCtx);
+    ASSERT_NOT_OK(result.getStatus());
+
+    query = BSON("x" << BSON("$mod" << BSON_ARRAY(5 << BSONNULL)));
+    result = MatchExpressionParser::parse(query, expCtx);
+    ASSERT_NOT_OK(result.getStatus());
 }
 
 TEST(MatchExpressionParserLeafTest, SimpleMod1) {
@@ -335,19 +343,6 @@ TEST(MatchExpressionParserLeafTest, SimpleMod1) {
     ASSERT(result.getValue()->matchesBSON(BSON("x" << 8)));
 }
 
-TEST(MatchExpressionParserLeafTest, SimpleModNotNumber) {
-    BSONObj query = BSON("x" << BSON("$mod" << BSON_ARRAY(2 << "r")));
-    boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
-    StatusWithMatchExpression result = MatchExpressionParser::parse(query, expCtx);
-    ASSERT_OK(result.getStatus());
-
-    ASSERT(result.getValue()->matchesBSON(BSON("x" << 2)));
-    ASSERT(result.getValue()->matchesBSON(BSON("x" << 4)));
-    ASSERT(!result.getValue()->matchesBSON(BSON("x" << 5)));
-    ASSERT(!result.getValue()->matchesBSON(BSON("x"
-                                                << "a")));
-}
-
 TEST(MatchExpressionParserLeafTest, IdCollation) {
     BSONObj query = BSON("$id"
                          << "string");