SERVER-27197 fix BSONType::Code comparison to not use collator

4 files changed, 152 insertions, 9 deletions

diff --git a/src/mongo/bson/bsonelement.cpp b/src/mongo/bson/bsonelement.cpp
index 5131d5943ed..4417821f816 100644
--- a/src/mongo/bson/bsonelement.cpp
+++ b/src/mongo/bson/bsonelement.cpp
@@ -336,6 +336,19 @@ const StringMap<BSONObj::MatchType> queryOperatorMap{
     {"bitsAnyClear", BSONObj::opBITS_ANY_CLEAR},
 };
 
+// Compares two string elements using a simple binary compare.
+int compareElementStringValues(const BSONElement& leftStr, const BSONElement& rightStr) {
+    // we use memcmp as we allow zeros in UTF8 strings
+    int lsz = leftStr.valuestrsize();
+    int rsz = rightStr.valuestrsize();
+    int common = std::min(lsz, rsz);
+    int res = memcmp(leftStr.valuestr(), rightStr.valuestr(), common);
+    if (res)
+        return res;
+    // longer std::string is the greater one
+    return lsz - rsz;
+}
+
 }  // namespace
 
 int BSONElement::getGtLtOp(int def) const {
@@ -943,20 +956,13 @@ int compareElementValues(const BSONElement& l,
         case jstOID:
             return memcmp(l.value(), r.value(), OID::kOIDSize);
         case Code:
+            return compareElementStringValues(l, r);
         case Symbol:
         case String: {
             if (comparator) {
                 return comparator->compare(l.valueStringData(), r.valueStringData());
             } else {
-                // we use memcmp as we allow zeros in UTF8 strings
-                int lsz = l.valuestrsize();
-                int rsz = r.valuestrsize();
-                int common = std::min(lsz, rsz);
-                int res = memcmp(l.valuestr(), r.valuestr(), common);
-                if (res)
-                    return res;
-                // longer std::string is the greater one
-                return lsz - rsz;
+                return compareElementStringValues(l, r);
             }
         }
         case Object:
diff --git a/src/mongo/db/pipeline/document_comparator_test.cpp b/src/mongo/db/pipeline/document_comparator_test.cpp
index 27e73f7edd6..6122e0fe51f 100644
--- a/src/mongo/db/pipeline/document_comparator_test.cpp
+++ b/src/mongo/db/pipeline/document_comparator_test.cpp
@@ -197,5 +197,23 @@ TEST(DocumentComparatorTest, HashingCodeWScopeShouldNotRespectCollation) {
     ASSERT_NE(seed1, seed2);
 }
 
+TEST(DocumentComparatorTest, ComparingCodeShouldNotRespectCollation) {
+    const CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    const DocumentComparator comparator(&collator);
+    const Document doc1{{"a", BSONCode("js code")}};
+    const Document doc2{{"a", BSONCode("other js code")}};
+    ASSERT_TRUE(comparator.evaluate(doc1 != doc2));
+}
+
+TEST(DocumentComparatorTest, HashingCodeShouldNotRespectCollation) {
+    const CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    const Document doc1{{"a", BSONCode("js code")}};
+    const Document doc2{{"a", BSONCode("other js code")}};
+    size_t seed1, seed2 = 0;
+    doc1.hash_combine(seed1, &collator);
+    doc2.hash_combine(seed2, &collator);
+    ASSERT_NE(seed1, seed2);
+}
+
 }  // namespace
 }  // namespace mongo
diff --git a/src/mongo/db/pipeline/value_comparator_test.cpp b/src/mongo/db/pipeline/value_comparator_test.cpp
index cbfb5eb36bd..3a1fe7abe75 100644
--- a/src/mongo/db/pipeline/value_comparator_test.cpp
+++ b/src/mongo/db/pipeline/value_comparator_test.cpp
@@ -295,5 +295,21 @@ TEST(ValueComparatorTest, HashingCodeWScopeShouldNotRespectCollation) {
     ASSERT_NE(comparator.hash(val1), comparator.hash(val2));
 }
 
+TEST(ValueComparatorTest, ComparingCodeShouldNotRespectCollation) {
+    const CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    const ValueComparator comparator(&collator);
+    const Value val1{BSONCode("js code")};
+    const Value val2{BSONCode("other js code")};
+    ASSERT_TRUE(comparator.evaluate(val1 != val2));
+}
+
+TEST(ValueComparatorTest, HashingCodeShouldNotRespectCollation) {
+    const CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    const ValueComparator comparator(&collator);
+    const Value val1{BSONCode("js code")};
+    const Value val2{BSONCode("other js code")};
+    ASSERT_NE(comparator.hash(val1), comparator.hash(val2));
+}
+
 }  // namespace
 }  // namespace mongo
diff --git a/src/mongo/db/query/collation/collation_bson_comparison_test.cpp b/src/mongo/db/query/collation/collation_bson_comparison_test.cpp
index fc07f1ac244..1521af1f146 100644
--- a/src/mongo/db/query/collation/collation_bson_comparison_test.cpp
+++ b/src/mongo/db/query/collation/collation_bson_comparison_test.cpp
@@ -102,5 +102,108 @@ TEST(CollationBSONComparisonTest, HashingCodeWScopeObjWithCollationShouldNotResp
     ASSERT_NE(comparator.hash(obj1), comparator.hash(obj2));
 }
 
+TEST(CollationBSONComparisonTest, ElementStringComparisonShouldRespectCollation) {
+    const CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    const BSONElementComparator comparator(BSONElementComparator::FieldNamesMode::kConsider,
+                                           &collator);
+    BSONObj obj1 = BSON("a"
+                        << "foo");
+    BSONObj obj2 = BSON("a"
+                        << "not foo");
+    ASSERT(comparator.evaluate(obj1["a"] == obj2["a"]));
+}
+
+TEST(CollationBSONComparisonTest, ElementStringHashShouldRespectCollation) {
+    const CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    const BSONElementComparator comparator(BSONElementComparator::FieldNamesMode::kConsider,
+                                           &collator);
+    BSONObj obj1 = BSON("a"
+                        << "foo");
+    BSONObj obj2 = BSON("a"
+                        << "not foo");
+    ASSERT_EQ(comparator.hash(obj1["a"]), comparator.hash(obj2["a"]));
+}
+
+TEST(CollationBSONComparisonTest, ObjStringComparisonShouldRespectCollation) {
+    const CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    const BSONObjComparator comparator(
+        BSONObj(), BSONObjComparator::FieldNamesMode::kConsider, &collator);
+    BSONObj obj1 = BSON("a"
+                        << "foo");
+    BSONObj obj2 = BSON("a"
+                        << "not foo");
+    ASSERT(comparator.evaluate(obj1 == obj2));
+}
+
+TEST(CollationBSONComparisonTest, ObjStringHashShouldRespectCollation) {
+    const CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    const BSONObjComparator comparator(
+        BSONObj(), BSONObjComparator::FieldNamesMode::kConsider, &collator);
+    BSONObj obj1 = BSON("a"
+                        << "foo");
+    BSONObj obj2 = BSON("a"
+                        << "not foo");
+    ASSERT_EQ(comparator.hash(obj1), comparator.hash(obj2));
+}
+
+TEST(CollationBSONComparisonTest, ElementCodeComparisonShouldNotRespectCollation) {
+    const CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    const BSONElementComparator comparator(BSONElementComparator::FieldNamesMode::kConsider,
+                                           &collator);
+    BSONObj obj1 = BSON("a" << BSONCode("foo"));
+    BSONObj obj2 = BSON("a" << BSONCode("not foo"));
+    ASSERT(comparator.evaluate(obj1["a"] != obj2["a"]));
+}
+
+TEST(CollationBSONComparisonTest, ElementCodeHashShouldNotRespectCollation) {
+    const CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    const BSONElementComparator comparator(BSONElementComparator::FieldNamesMode::kConsider,
+                                           &collator);
+    BSONObj obj1 = BSON("a" << BSONCode("foo"));
+    BSONObj obj2 = BSON("a" << BSONCode("not foo"));
+    ASSERT_NE(comparator.hash(obj1["a"]), comparator.hash(obj2["a"]));
+}
+
+TEST(CollationBSONComparisonTest, ObjCodeComparisonShouldNotRespectCollation) {
+    const CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    const BSONObjComparator comparator(
+        BSONObj(), BSONObjComparator::FieldNamesMode::kConsider, &collator);
+    BSONObj obj1 = BSON("a" << BSONCode("foo"));
+    BSONObj obj2 = BSON("a" << BSONCode("not foo"));
+    ASSERT(comparator.evaluate(obj1 != obj2));
+}
+
+TEST(CollationBSONComparisonTest, ObjCodeHashShouldNotRespectCollation) {
+    const CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    const BSONObjComparator comparator(
+        BSONObj(), BSONObjComparator::FieldNamesMode::kConsider, &collator);
+    BSONObj obj1 = BSON("a" << BSONCode("foo"));
+    BSONObj obj2 = BSON("a" << BSONCode("not foo"));
+    ASSERT_NE(comparator.hash(obj1), comparator.hash(obj2));
+}
+
+TEST(CollationBSONComparisonTest, IdenticalCodeAndStringValuesAreNotEqual) {
+    const CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    const BSONObjComparator comparator(
+        BSONObj(), BSONObjComparator::FieldNamesMode::kConsider, &collator);
+    BSONObj obj1 = BSON("a"
+                        << "foo");
+    BSONObj obj2 = BSON("a" << BSONCode("foo"));
+    ASSERT_BSONOBJ_NE(obj1, obj2);
+    ASSERT(comparator.evaluate(obj1 != obj2));
+}
+
+TEST(CollationBSONComparisonTest, IdenticalCodeAndStringValuesDoNotHashEqually) {
+    const CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual);
+    const BSONObjComparator comparator(
+        BSONObj(), BSONObjComparator::FieldNamesMode::kConsider, &collator);
+    BSONObj obj1 = BSON("a"
+                        << "foo");
+    BSONObj obj2 = BSON("a" << BSONCode("foo"));
+    ASSERT_NE(comparator.hash(obj1), comparator.hash(obj2));
+    ASSERT_NE(SimpleBSONObjComparator::kInstance.hash(obj1),
+              SimpleBSONObjComparator::kInstance.hash(obj2));
+}
+
 }  // namespace
 }  // namespace mongo