diff options
-rw-r--r-- | src/mongo/bson/bsonelement.cpp | 18 | ||||
-rw-r--r-- | src/mongo/bson/bsonelement.h | 13 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_parser.cpp | 83 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_parser_array_test.cpp | 7 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_parser_test.cpp | 58 |
5 files changed, 105 insertions, 74 deletions
diff --git a/src/mongo/bson/bsonelement.cpp b/src/mongo/bson/bsonelement.cpp index d93f928861c..e02dfd07f33 100644 --- a/src/mongo/bson/bsonelement.cpp +++ b/src/mongo/bson/bsonelement.cpp @@ -515,7 +515,8 @@ StatusWith<long long> BSONElement::parseIntegerElementToNonNegativeLong() const if (number.getValue() < 0) { return Status(ErrorCodes::FailedToParse, - str::stream() << "Expected a positive number in: " << toString(true, true)); + str::stream() + << "Expected a non-negative number in: " << toString(true, true)); } return number; @@ -585,6 +586,21 @@ StatusWith<int> BSONElement::parseIntegerElementToInt() const { return static_cast<int>(valueLong); } +StatusWith<int> BSONElement::parseIntegerElementToNonNegativeInt() const { + auto number = parseIntegerElementToInt(); + if (!number.isOK()) { + return number; + } + + if (number.getValue() < 0) { + return Status(ErrorCodes::FailedToParse, + str::stream() + << "Expected a non-negative number in: " << toString(true, true)); + } + + return number; +} + BSONObj BSONElement::embeddedObjectUserCheck() const { if (MONGO_likely(isABSONObj())) return BSONObj(value(), BSONObj::LargeSizeTrait{}); diff --git a/src/mongo/bson/bsonelement.h b/src/mongo/bson/bsonelement.h index 11a8ad6994a..c99623c7e80 100644 --- a/src/mongo/bson/bsonelement.h +++ b/src/mongo/bson/bsonelement.h @@ -412,7 +412,7 @@ public: long long exactNumberLong() const; /** - * Parses a BSONElement of any numeric type into a positive long long, failing if the value + * Parses a BSONElement of any numeric type into a non-negative long long, failing if the value * is any of the following: * * - NaN. @@ -433,6 +433,17 @@ public: StatusWith<long long> parseIntegerElementToLong() const; /** + * Parses a BSONElement of any numeric type into a non-negative int, failing if the value + * is any of the following: + * + * - NaN + * - Negative + * - a non-integral number + * - too large in the positive or negative direction to fit in an int + */ + StatusWith<int> parseIntegerElementToNonNegativeInt() const; + + /** * Parses a BSONElement of any numeric type into an integer, failing if the value is: * * - NaN diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp index 887caf45535..e5dd4c3816f 100644 --- a/src/mongo/db/matcher/expression_parser.cpp +++ b/src/mongo/db/matcher/expression_parser.cpp @@ -696,58 +696,13 @@ StatusWith<std::vector<uint32_t>> parseBitPositionsArray(const BSONObj& theArray // Fill temporary bit position array with integers read from the BSON array. for (auto e : theArray) { - if (!e.isNumber()) { - return Status(ErrorCodes::BadValue, - str::stream() << "bit positions must be an integer but got: " << e); - } - - if (e.type() == BSONType::NumberDouble) { - auto eDouble = e.numberDouble(); - - // NaN doubles are rejected. - if (std::isnan(eDouble)) { - return Status(ErrorCodes::BadValue, - str::stream() << "bit positions cannot take a NaN: " << e); - } - - // This makes sure e does not overflow a 32-bit integer container. - if (eDouble > std::numeric_limits<int>::max() || - eDouble < std::numeric_limits<int>::min()) { - return Status( - ErrorCodes::BadValue, - str::stream() - << "bit positions cannot be represented as a 32-bit signed integer: " << e); - } - - // This checks if e is integral. - if (eDouble != static_cast<double>(static_cast<long long>(eDouble))) { - return Status(ErrorCodes::BadValue, - str::stream() << "bit positions must be an integer but got: " << e); - } - } - - if (e.type() == BSONType::NumberLong) { - auto eLong = e.numberLong(); - - // This makes sure e does not overflow a 32-bit integer container. - if (eLong > std::numeric_limits<int>::max() || - eLong < std::numeric_limits<int>::min()) { - return Status( - ErrorCodes::BadValue, - str::stream() - << "bit positions cannot be represented as a 32-bit signed integer: " << e); - } - } - - auto eValue = e.numberInt(); - - // No negatives. - if (eValue < 0) { + auto status = e.parseIntegerElementToNonNegativeInt(); + if (!status.isOK()) { return Status(ErrorCodes::BadValue, - str::stream() << "bit positions must be >= 0 but got: " << e); + str::stream() + << "Failed to parse bit position. " << status.getStatus().reason()); } - - bitPositions.push_back(eValue); + bitPositions.push_back(status.getValue()); } return bitPositions; @@ -1724,29 +1679,13 @@ StatusWithMatchExpression parseSubField(const BSONObj& context, } case PathAcceptingKeyword::SIZE: { - int size = 0; - if (e.type() == BSONType::NumberInt) { - size = e.numberInt(); - } else if (e.type() == BSONType::NumberLong) { - if (e.numberInt() == e.numberLong()) { - size = e.numberInt(); - } else { - return {Status(ErrorCodes::BadValue, - "$size must be representable as a 32-bit integer")}; - } - } else if (e.type() == BSONType::NumberDouble) { - if (e.numberInt() == e.numberDouble()) { - size = e.numberInt(); - } else { - return {Status(ErrorCodes::BadValue, "$size must be a whole number")}; - } - } else { - return {Status(ErrorCodes::BadValue, "$size needs a number")}; - } - - if (size < 0) { - return {Status(ErrorCodes::BadValue, "$size may not be negative")}; + auto status = e.parseIntegerElementToNonNegativeInt(); + if (!status.isOK()) { + return Status(ErrorCodes::BadValue, + str::stream() + << "Failed to parse $size. " << status.getStatus().reason()); } + int size = status.getValue(); return {std::make_unique<SizeMatchExpression>( name, size, diff --git a/src/mongo/db/matcher/expression_parser_array_test.cpp b/src/mongo/db/matcher/expression_parser_array_test.cpp index c519ee2b374..243bed553f0 100644 --- a/src/mongo/db/matcher/expression_parser_array_test.cpp +++ b/src/mongo/db/matcher/expression_parser_array_test.cpp @@ -109,6 +109,13 @@ TEST(MatchExpressionParserArrayTest, SizeWithNegativeIntegralDouble) { ASSERT_FALSE(result.isOK()); } +TEST(MatchExpressionParserArrayTest, SizeWithTooLargeDouble) { + BSONObj query = BSON("x" << BSON("$size" << 1e12)); + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + StatusWithMatchExpression result = MatchExpressionParser::parse(query, expCtx); + ASSERT_FALSE(result.isOK()); +} + TEST(MatchExpressionParserArrayTest, SizeWithDouble) { BSONObj query = BSON("x" << BSON("$size" << 2.5)); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); diff --git a/src/mongo/db/matcher/expression_parser_test.cpp b/src/mongo/db/matcher/expression_parser_test.cpp index a37e25c9542..af6e1ff997a 100644 --- a/src/mongo/db/matcher/expression_parser_test.cpp +++ b/src/mongo/db/matcher/expression_parser_test.cpp @@ -519,6 +519,64 @@ TEST(MatchExpressionParserTest, SampleRateFailureCases) { ASSERT_NOT_OK(result.getStatus()); } +TEST(MatchExpressionParserTest, BitwiseOperators) { + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + std::vector<std::string> bitwiseOperators{ + "$bitsAllClear", "$bitsAllSet", "$bitsAnyClear", "$bitsAnySet"}; + for (auto& bitwiseOperator : bitwiseOperators) { + // Test accepting valid type coercion + auto result = MatchExpressionParser::parse( + BSON("x" << BSON(bitwiseOperator << BSON_ARRAY(1))), expCtx); + ASSERT_TRUE(result.isOK()); + result = MatchExpressionParser::parse(BSON("x" << BSON(bitwiseOperator << BSON_ARRAY(1LL))), + expCtx); + ASSERT_TRUE(result.isOK()); + result = MatchExpressionParser::parse(BSON("x" << BSON(bitwiseOperator << BSON_ARRAY(1.0))), + expCtx); + ASSERT_TRUE(result.isOK()); + + // Test rejecting overflow values. + result = MatchExpressionParser::parse( + BSON("x" << BSON(bitwiseOperator << BSON_ARRAY(std::numeric_limits<long long>::min()))), + expCtx); + ASSERT_EQ(result.getStatus(), ErrorCodes::BadValue); + result = MatchExpressionParser::parse( + BSON("x" << BSON(bitwiseOperator << BSON_ARRAY(std::numeric_limits<long long>::max()))), + expCtx); + ASSERT_EQ(result.getStatus(), ErrorCodes::BadValue); + result = MatchExpressionParser::parse( + BSON("x" << BSON(bitwiseOperator << BSON_ARRAY(1e30))), expCtx); + ASSERT_EQ(result.getStatus(), ErrorCodes::BadValue); + result = MatchExpressionParser::parse( + BSON("x" << BSON(bitwiseOperator << BSON_ARRAY(-1e30))), expCtx); + ASSERT_EQ(result.getStatus(), ErrorCodes::BadValue); + + // Test rejecting non-integral values. + result = MatchExpressionParser::parse(BSON("x" << BSON(bitwiseOperator << BSON_ARRAY(1.5))), + expCtx); + ASSERT_EQ(result.getStatus(), ErrorCodes::BadValue); + + // Test rejecting negative values. + result = MatchExpressionParser::parse(BSON("x" << BSON(bitwiseOperator << BSON_ARRAY(-1))), + expCtx); + ASSERT_EQ(result.getStatus(), ErrorCodes::BadValue); + result = MatchExpressionParser::parse( + BSON("x" << BSON(bitwiseOperator << BSON_ARRAY(-1LL))), expCtx); + ASSERT_EQ(result.getStatus(), ErrorCodes::BadValue); + result = MatchExpressionParser::parse( + BSON("x" << BSON(bitwiseOperator << BSON_ARRAY(-1.0))), expCtx); + ASSERT_EQ(result.getStatus(), ErrorCodes::BadValue); + + // Test rejecting non-numeric values. + result = MatchExpressionParser::parse( + BSON("x" << BSON(bitwiseOperator << BSON_ARRAY("string"))), expCtx); + ASSERT_EQ(result.getStatus(), ErrorCodes::BadValue); + result = MatchExpressionParser::parse(BSON("x" << BSON(bitwiseOperator << BSON_ARRAY(NAN))), + expCtx); + ASSERT_EQ(result.getStatus(), ErrorCodes::BadValue); + } +} + TEST(InternalBinDataSubTypeMatchExpressionTest, SubTypeParsesCorrectly) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto query = BSON("a" << BSON("$_internalSchemaBinDataSubType" << BinDataType::bdtCustom)); |