SERVER-54482 Fix crash if collation.backwards param is undefined/null

5 files changed, 26 insertions, 11 deletions

diff --git a/jstests/core/collation.js b/jstests/core/collation.js
index 15142a7659f..ff856ba8ea8 100644
--- a/jstests/core/collation.js
+++ b/jstests/core/collation.js
@@ -1676,6 +1676,21 @@ if (db.getMongo().writeMode() === "commands") {
     });
 }
 
+// updateOne with undefined/null collation.backwards parameter (SERVER-54482).
+if (db.getMongo().writeMode() === "commands") {
+    for (let backwards of [undefined, null]) {
+        assert.throws(function() {
+            coll.bulkWrite([{
+                updateOne: {
+                    filter: {str: 'foo'},
+                    update: {$set: {str: 'bar'}},
+                    collation: {locale: 'en_US', backwards: backwards}
+                }
+            }]);
+        });
+    }
+}
+
 // updateMany with bulkWrite().
 coll.drop();
 assert.commandWorked(coll.insert({_id: 1, str: "foo"}));
diff --git a/src/mongo/db/query/canonical_query_encoder.cpp b/src/mongo/db/query/canonical_query_encoder.cpp
index f5c610004d0..e8ef4308158 100644
--- a/src/mongo/db/query/canonical_query_encoder.cpp
+++ b/src/mongo/db/query/canonical_query_encoder.cpp
@@ -367,7 +367,7 @@ void encodeCollation(const CollatorInterface* collation, StringBuilder* keyBuild
     *keyBuilder << encodeEnum(spec.getAlternate());
     *keyBuilder << encodeEnum(spec.getMaxVariable());
     *keyBuilder << spec.getNormalization();
-    *keyBuilder << spec.getBackwards().value_or(false);
+    *keyBuilder << spec.getBackwards();
 
     // We do not encode 'spec.version' because query shape strings are never persisted, and need
     // not be stable between versions.
diff --git a/src/mongo/db/query/collation/collator_factory_icu.cpp b/src/mongo/db/query/collation/collator_factory_icu.cpp
index b2d90d1b3d0..499f19384d3 100644
--- a/src/mongo/db/query/collation/collator_factory_icu.cpp
+++ b/src/mongo/db/query/collation/collator_factory_icu.cpp
@@ -396,9 +396,9 @@ Status updateCollationSpecFromICUCollator(const BSONObj& spec,
     } else {
         UErrorCode status = U_ZERO_ERROR;
         // collation->getBackwards should be engaged if spec has a "backwards" field.
-        invariant(collation->getBackwards().is_initialized());
+        invariant(collation->getBackwards().has_value());
         icuCollator->setAttribute(
-            UCOL_FRENCH_COLLATION, boolToAttribute(*collation->getBackwards()), status);
+            UCOL_FRENCH_COLLATION, boolToAttribute(collation->getBackwards()), status);
         if (U_FAILURE(status)) {
             icu::ErrorCode icuError;
             icuError.set(status);
@@ -466,7 +466,7 @@ Status validateLocaleID(const BSONObj& spec, StringData originalID, const icu::C
 Status validateCollationSpec(const Collation& collation, const BSONObj& spec) {
     // The backwards option specifically means backwards secondary weighting, and therefore only
     // affects the secondary comparison level. It has no effect at strength 1.
-    if (collation.getBackwards().value_or(false) &&
+    if (collation.getBackwards() &&
         static_cast<CollationStrength>(collation.getStrength()) == CollationStrength::kPrimary) {
         return {ErrorCodes::BadValue,
                 str::stream() << "'" << Collation::kBackwardsFieldName << "' is invalid with '"
diff --git a/src/mongo/db/query/collation/collator_factory_icu_test.cpp b/src/mongo/db/query/collation/collator_factory_icu_test.cpp
index f0a08fdf97a..bb62deaf241 100644
--- a/src/mongo/db/query/collation/collator_factory_icu_test.cpp
+++ b/src/mongo/db/query/collation/collator_factory_icu_test.cpp
@@ -418,7 +418,7 @@ TEST(CollatorFactoryICUTest, DefaultsSetSuccessfully) {
     ASSERT_EQ(static_cast<int>(CollationMaxVariableEnum::kPunct),
               static_cast<int>(collator.getValue()->getSpec().getMaxVariable()));
     ASSERT_FALSE(collator.getValue()->getSpec().getNormalization());
-    ASSERT_FALSE(*collator.getValue()->getSpec().getBackwards());
+    ASSERT_FALSE(collator.getValue()->getSpec().getBackwards());
 }
 
 TEST(CollatorFactoryICUTest, LanguageDependentDefaultsSetSuccessfully) {
@@ -426,7 +426,7 @@ TEST(CollatorFactoryICUTest, LanguageDependentDefaultsSetSuccessfully) {
     auto collator = factory.makeFromBSON(BSON("locale"
                                               << "fr_CA"));
     ASSERT_OK(collator.getStatus());
-    ASSERT_TRUE(*collator.getValue()->getSpec().getBackwards());
+    ASSERT_TRUE(collator.getValue()->getSpec().getBackwards());
 }
 
 TEST(CollatorFactoryICUTest, CaseLevelFalseParsesSuccessfully) {
@@ -616,7 +616,7 @@ TEST(CollatorFactoryICUTest, BackwardsFalseParsesSuccessfully) {
                                               << "en_US"
                                               << "backwards" << false));
     ASSERT_OK(collator.getStatus());
-    ASSERT_FALSE(*collator.getValue()->getSpec().getBackwards());
+    ASSERT_FALSE(collator.getValue()->getSpec().getBackwards());
 }
 
 TEST(CollatorFactoryICUTest, BackwardsTrueParsesSuccessfully) {
@@ -625,7 +625,7 @@ TEST(CollatorFactoryICUTest, BackwardsTrueParsesSuccessfully) {
                                               << "en_US"
                                               << "backwards" << true));
     ASSERT_OK(collator.getStatus());
-    ASSERT_TRUE(*collator.getValue()->getSpec().getBackwards());
+    ASSERT_TRUE(collator.getValue()->getSpec().getBackwards());
 }
 
 TEST(CollatorFactoryICUTest, LongStrengthFieldParsesSuccessfully) {
diff --git a/src/mongo/idl/basic_types.idl b/src/mongo/idl/basic_types.idl
index c82abccf4f7..1bac48374f5 100644
--- a/src/mongo/idl/basic_types.idl
+++ b/src/mongo/idl/basic_types.idl
@@ -384,10 +384,10 @@ structs:
                 type: bool
                 default: false
             # Causes accent differences to be considered in reverse order, as it is done in the
-            # French language.
+            # French language. Note: the optionalBool parser rejects null/undefined inputs, which
+            # preserves an invariant in updateCollationSpecFromICUCollator.
             backwards:
-                type: bool
-                optional: true
+                type: optionalBool
             # Indicates the version of the collator. It is used to ensure that we do not mix
             # versions by, for example, constructing an index with one version of ICU and then
             # attempting to use this index with a server that is built against a newer ICU version.