SERVER-35247 Undefined is allowed for required fields in IDL

2 files changed, 88 insertions, 11 deletions

diff --git a/buildscripts/idl/idl/generator.py b/buildscripts/idl/idl/generator.py
index 82c22b45b8f..d2eccf54c9c 100644
--- a/buildscripts/idl/idl/generator.py
+++ b/buildscripts/idl/idl/generator.py
@@ -845,10 +845,21 @@ class _CppSourceFileWriter(_CppFileWriterBase):
         else:
             self._writer.write_line('%s = std::move(values);' % (_get_field_member_name(field)))
 
-    def gen_field_deserializer(self, field, bson_object, bson_element):
-        # type: (ast.Field, unicode, unicode) -> None
+    def _gen_usage_check(self, field, bson_element, field_usage_check):
+        # type: (ast.Field, unicode, _FieldUsageCheckerBase) -> None
+        """Generate the field usage check and insert the required field check."""
+        if field_usage_check:
+            field_usage_check.add(field, bson_element)
+
+            if _is_required_serializer_field(field):
+                self._writer.write_line('%s = true;' % (_get_has_field_member_name(field)))
+
+    def gen_field_deserializer(self, field, bson_object, bson_element, field_usage_check):
+        # type: (ast.Field, unicode, unicode, _FieldUsageCheckerBase) -> None
         """Generate the C++ deserializer piece for a field."""
         if field.array:
+            self._gen_usage_check(field, bson_element, field_usage_check)
+
             self._gen_array_deserializer(field, bson_element)
             return
 
@@ -864,13 +875,17 @@ class _CppSourceFileWriter(_CppFileWriterBase):
                 method_name = writer.get_method_name_from_qualified_method_name(field.deserializer)
                 expression = "%s(%s)" % (method_name, bson_object)
 
+            self._gen_usage_check(field, bson_element, field_usage_check)
+
             self._writer.write_line('%s = %s;' % (_get_field_member_name(field), expression))
         else:
-            # May be an empty block if the type is 'any'
             predicate = _get_bson_type_check(bson_element, 'ctxt', field)
             if predicate:
                 predicate = "MONGO_likely(%s)" % (predicate)
             with self._predicate(predicate):
+
+                self._gen_usage_check(field, bson_element, field_usage_check)
+
                 object_value = self._gen_field_deserializer_expression(bson_element, field)
                 if field.chained_struct_field:
                     self._writer.write_line('%s.%s(%s);' %
@@ -976,7 +991,8 @@ class _CppSourceFileWriter(_CppFileWriterBase):
 
         if isinstance(struct, ast.Command) and struct.command_field:
             with self._block('{', '}'):
-                self.gen_field_deserializer(struct.command_field, bson_object, "commandElement")
+                self.gen_field_deserializer(struct.command_field, bson_object, "commandElement",
+                                            None)
         else:
             struct_type_info = struct_types.get_struct_info(struct)
 
@@ -1021,16 +1037,14 @@ class _CppSourceFileWriter(_CppFileWriterBase):
                 field_predicate = 'fieldName == %s' % (_get_field_constant_name(field))
 
                 with self._predicate(field_predicate, not first_field):
-                    field_usage_check.add(field, "element")
 
                     if field.ignore:
+                        field_usage_check.add(field, "element")
+
                         self._writer.write_line('// ignore field')
                     else:
-                        if _is_required_serializer_field(field):
-                            self._writer.write_line('%s = true;' %
-                                                    (_get_has_field_member_name(field)))
-
-                        self.gen_field_deserializer(field, bson_object, "element")
+                        self.gen_field_deserializer(field, bson_object, "element",
+                                                    field_usage_check)
 
                 if first_field:
                     first_field = False
@@ -1056,7 +1070,7 @@ class _CppSourceFileWriter(_CppFileWriterBase):
                 continue
 
             # Simply generate deserializers since these are all 'any' types
-            self.gen_field_deserializer(field, bson_object, "element")
+            self.gen_field_deserializer(field, bson_object, "element", None)
             self._writer.write_empty_line()
 
         self._writer.write_empty_line()
diff --git a/src/mongo/idl/idl_test.cpp b/src/mongo/idl/idl_test.cpp
index 8818132c65e..ed5ea3dc6ab 100644
--- a/src/mongo/idl/idl_test.cpp
+++ b/src/mongo/idl/idl_test.cpp
@@ -274,6 +274,31 @@ TEST(IDLOneTypeTests, TestNegativeWrongTypes) {
     TestParsers<One_timestamp, bsonTimestamp>();
 }
 
+// Negative: document with wrong types for required field
+TEST(IDLOneTypeTests, TestNegativeRequiredNullTypes) {
+    TestParse<One_string, String, NullLabeler, jstNULL>(BSONNULL);
+    TestParse<One_int, NumberInt, NullLabeler, jstNULL>(BSONNULL);
+    TestParse<One_long, NumberLong, NullLabeler, jstNULL>(BSONNULL);
+    TestParse<One_double, NumberDouble, NullLabeler, jstNULL>(BSONNULL);
+    TestParse<One_bool, Bool, NullLabeler, jstNULL>(BSONNULL);
+    TestParse<One_objectid, jstOID, NullLabeler, jstNULL>(BSONNULL);
+    TestParse<One_date, Date, NullLabeler, jstNULL>(BSONNULL);
+    TestParse<One_timestamp, bsonTimestamp, NullLabeler, jstNULL>(BSONNULL);
+}
+
+// Negative: document with wrong types for required field
+TEST(IDLOneTypeTests, TestNegativeRequiredUndefinedTypes) {
+    TestParse<One_string, String, UndefinedLabeler, Undefined>(BSONUndefined);
+    TestParse<One_int, NumberInt, UndefinedLabeler, Undefined>(BSONUndefined);
+    TestParse<One_long, NumberLong, UndefinedLabeler, Undefined>(BSONUndefined);
+    TestParse<One_double, NumberDouble, UndefinedLabeler, Undefined>(BSONUndefined);
+    TestParse<One_bool, Bool, UndefinedLabeler, Undefined>(BSONUndefined);
+    TestParse<One_objectid, jstOID, UndefinedLabeler, Undefined>(BSONUndefined);
+    TestParse<One_date, Date, UndefinedLabeler, Undefined>(BSONUndefined);
+    TestParse<One_timestamp, bsonTimestamp, UndefinedLabeler, Undefined>(BSONUndefined);
+}
+
+
 // Mixed: test a type that accepts multiple bson types
 TEST(IDLOneTypeTests, TestSafeInt32) {
     TestParse<One_safeint32, NumberInt, StringData, String>("test_value");
@@ -600,6 +625,19 @@ TEST(IDLFieldTests, TestStrictStructIgnoredField) {
     }
 }
 
+// Negative: check duplicate ignored fields fail
+TEST(IDLFieldTests, TestStrictDuplicateIgnoredFields) {
+    IDLParserErrorContext ctxt("root");
+
+    // Negative: Test duplicate ignored fields fail
+    {
+        auto testDoc =
+            BSON("required_field" << 12 << "ignored_field" << 123 << "ignored_field" << 456);
+        ASSERT_THROWS(IgnoredField::parse(ctxt, testDoc), AssertionException);
+    }
+}
+
+
 // First test: test an empty document and the default value
 // Second test: test a non-empty document and that we do not get the default value
 #define TEST_DEFAULT_VALUES(field_name, default_value, new_value)   \
@@ -685,6 +723,31 @@ TEST(IDLFieldTests, TestOptionalFields) {
     }
 }
 
+template <typename TestT>
+void TestWeakType(TestT test_value) {
+    IDLParserErrorContext ctxt("root");
+    auto testDoc =
+        BSON("field1" << test_value << "field2" << test_value << "field3" << test_value << "field4"
+                      << test_value
+                      << "field5"
+                      << test_value);
+    auto testStruct = Optional_field::parse(ctxt, testDoc);
+
+    ASSERT_FALSE(testStruct.getField1().is_initialized());
+    ASSERT_FALSE(testStruct.getField2().is_initialized());
+    ASSERT_FALSE(testStruct.getField3().is_initialized());
+    ASSERT_FALSE(testStruct.getField4().is_initialized());
+    ASSERT_FALSE(testStruct.getField5().is_initialized());
+}
+
+// Positive: struct strict, and optional field works
+TEST(IDLFieldTests, TestOptionalFieldsWithNullAndUndefined) {
+
+    TestWeakType<NullLabeler>(BSONNULL);
+
+    TestWeakType<UndefinedLabeler>(BSONUndefined);
+}
+
 // Positive: Test a nested struct
 TEST(IDLNestedStruct, TestDuplicateTypes) {
     IDLParserErrorContext ctxt("root");