Internally tagged enums don't honor deny_unknown_fields as precisely as

they might.

flatten doesn't act quite as intended with regard to
additional_properties

schemars/src/flatten.rs

@@ -38,6 +38,31 @@ macro_rules! impl_merge {
     };
 }
 
+// For ObjectValidation::additional_properties.
+impl Merge for Option<Box<Schema>> {
+    fn merge(self, other: Self) -> Self {
+        match (self.map(|x| *x), other.map(|x| *x)) {
+            // Perfer permissive schemas.
+            (Some(Schema::Bool(true)), _) => Some(Box::new(true.into())),
+            (_, Some(Schema::Bool(true))) => Some(Box::new(true.into())),
+            (None, _) => None,
+            (_, None) => None,
+
+            // Merge if we have two non-trivial schemas.
+            (Some(Schema::Object(s1)), Some(Schema::Object(s2))) => {
+                Some(Box::new(Schema::Object(s1.merge(s2))))
+            }
+
+            // Perfer the more permissive schema.
+            (Some(s1 @ Schema::Object(_)), Some(Schema::Bool(false))) => Some(Box::new(s1)),
+            (Some(Schema::Bool(false)), Some(s2 @ Schema::Object(_))) => Some(Box::new(s2)),
+
+            // Default to the null schema.
+            (Some(Schema::Bool(false)), Some(Schema::Bool(false))) => Some(Box::new(false.into())),
+        }
+    }
+}
+
 impl_merge!(SchemaObject {
     merge: extensions instance_type enum_values
         metadata subschemas number string array object,
@@ -76,8 +101,8 @@ impl_merge!(ArrayValidation {
 });
 
 impl_merge!(ObjectValidation {
-    merge: required properties pattern_properties,
+    merge: required properties pattern_properties additional_properties,
-    or: max_properties min_properties additional_properties property_names,
+    or: max_properties min_properties property_names,
 });
 
 impl<T: Merge> Merge for Option<T> {

schemars/tests/enum.rs

@@ -99,3 +99,16 @@ pub enum Adjacent {
 fn enum_adjacent_tagged() -> TestResult {
     test_default_generated_schema::<Adjacent>("enum-adjacent-tagged")
 }
+
+#[derive(Debug, JsonSchema)]
+#[schemars(tag = "typeProperty")]
+pub enum SimpleInternal {
+    A,
+    B,
+    C,
+}
+
+#[test]
+fn enum_simple_internal_tag() -> TestResult {
+    test_default_generated_schema::<SimpleInternal>("enum-simple-internal")
+}

schemars/tests/enum_deny_unknown_fields.rs

@@ -104,3 +104,16 @@ pub enum Adjacent {
 fn enum_adjacent_tagged() -> TestResult {
     test_default_generated_schema::<Adjacent>("enum-adjacent-tagged-duf")
 }
+
+#[derive(Debug, JsonSchema)]
+#[schemars(tag = "typeProperty", deny_unknown_fields)]
+pub enum SimpleInternal {
+    A,
+    B,
+    C,
+}
+
+#[test]
+fn enum_simple_internal_tag() -> TestResult {
+    test_default_generated_schema::<SimpleInternal>("enum-simple-internal-duf")
+}

schemars/tests/expected/enum-internal-duf.json

@@ -14,7 +14,8 @@
             "UnitOne"
           ]
         }
-      }
+      },
+      "additionalProperties": false
     },
     {
       "type": "object",
@@ -45,7 +46,8 @@
             "UnitStructNewType"
           ]
         }
-      }
+      },
+      "additionalProperties": false
     },
     {
       "type": "object",
@@ -106,7 +108,8 @@
             "UnitTwo"
           ]
         }
-      }
+      },
+      "additionalProperties": false
     },
     {
       "type": [
@@ -124,7 +127,8 @@
             "WithInt"
           ]
         }
-      }
+      },
+      "additionalProperties": false
     }
   ]
 }

schemars/tests/expected/enum-internal.json

@@ -28,9 +28,6 @@
             "StringMap"
           ]
         }
-      },
-      "additionalProperties": {
-        "type": "string"
       }
     },
     {

schemars/tests/expected/enum-simple-internal-duf.json

@@ -0,0 +1,51 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "title": "SimpleInternal",
+  "oneOf": [
+    {
+      "type": "object",
+      "required": [
+        "typeProperty"
+      ],
+      "properties": {
+        "typeProperty": {
+          "type": "string",
+          "enum": [
+            "A"
+          ]
+        }
+      },
+      "additionalProperties": false
+    },
+    {
+      "type": "object",
+      "required": [
+        "typeProperty"
+      ],
+      "properties": {
+        "typeProperty": {
+          "type": "string",
+          "enum": [
+            "B"
+          ]
+        }
+      },
+      "additionalProperties": false
+    },
+    {
+      "type": "object",
+      "required": [
+        "typeProperty"
+      ],
+      "properties": {
+        "typeProperty": {
+          "type": "string",
+          "enum": [
+            "C"
+          ]
+        }
+      },
+      "additionalProperties": false
+    }
+  ]
+}

schemars/tests/expected/enum-simple-internal.json

@@ -0,0 +1,48 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "title": "SimpleInternal",
+  "oneOf": [
+    {
+      "type": "object",
+      "required": [
+        "typeProperty"
+      ],
+      "properties": {
+        "typeProperty": {
+          "type": "string",
+          "enum": [
+            "A"
+          ]
+        }
+      }
+    },
+    {
+      "type": "object",
+      "required": [
+        "typeProperty"
+      ],
+      "properties": {
+        "typeProperty": {
+          "type": "string",
+          "enum": [
+            "B"
+          ]
+        }
+      }
+    },
+    {
+      "type": "object",
+      "required": [
+        "typeProperty"
+      ],
+      "properties": {
+        "typeProperty": {
+          "type": "string",
+          "enum": [
+            "C"
+          ]
+        }
+      }
+    }
+  ]
+}

schemars_derive/src/schema_exprs.rs

@@ -183,6 +183,12 @@ fn expr_for_external_tagged_enum<'a>(
                     required.insert(#name.to_owned());
                     required
                 },
+                // Externally tagged variants must prohibit additional
+                // properties irrespective of the disposition of
+                // `deny_unknown_fields`. If additional properties were allowed
+                // one could easily construct an object that validated against
+                // multiple variants since here it's the properties rather than
+                // the values of a property that distingish between variants.
                 additional_properties: Some(Box::new(false.into())),
                 ..Default::default()
             })),
@@ -206,6 +212,13 @@ fn expr_for_internal_tagged_enum<'a>(
 ) -> TokenStream {
     let mut unique_names = HashSet::new();
     let mut count = 0;
+    let set_additional_properties = if deny_unknown_fields {
+        quote! {
+            additional_properties: Some(Box::new(false.into())),
+        }
+    } else {
+        TokenStream::new()
+    };
     let variant_schemas = variants
         .map(|variant| {
             unique_names.insert(variant.name());
@@ -230,6 +243,9 @@ fn expr_for_internal_tagged_enum<'a>(
                         required.insert(#tag_name.to_owned());
                         required
                     },
+                    // As we're creating a "wrapper" object, we can honor the
+                    // disposition of deny_unknown_fields.
+                    #set_additional_properties
                     ..Default::default()
                 })),
             });
@@ -328,6 +344,8 @@ fn expr_for_adjacent_tagged_enum<'a>(
                         #add_content_to_required
                         required
                     },
+                    // As we're creating a "wrapper" object, we can honor the
+                    // disposition of deny_unknown_fields.
                     #set_additional_properties
                     ..Default::default()
                 })),