Skip to content

Commit

Permalink
Internally tagged enums don't honor deny_unknown_fields as precisely as
Browse files Browse the repository at this point in the history
they might.

flatten doesn't act quite as intended with regard to
additional_properties
  • Loading branch information
ahl committed Oct 8, 2021
1 parent 515a87a commit c543c15
Show file tree
Hide file tree
Showing 8 changed files with 176 additions and 9 deletions.
27 changes: 25 additions & 2 deletions schemars/src/flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,29 @@ 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.
(None | Some(Schema::Bool(true)), _) => None,
(_, None | Some(Schema::Bool(true))) => 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,
Expand Down Expand Up @@ -76,8 +99,8 @@ impl_merge!(ArrayValidation {
});

impl_merge!(ObjectValidation {
merge: required properties pattern_properties,
or: max_properties min_properties additional_properties property_names,
merge: required properties pattern_properties additional_properties,
or: max_properties min_properties property_names,
});

impl<T: Merge> Merge for Option<T> {
Expand Down
13 changes: 13 additions & 0 deletions schemars/tests/enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
13 changes: 13 additions & 0 deletions schemars/tests/enum_deny_unknown_fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
12 changes: 8 additions & 4 deletions schemars/tests/expected/enum-internal-duf.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
"UnitOne"
]
}
}
},
"additionalProperties": false
},
{
"type": "object",
Expand Down Expand Up @@ -45,7 +46,8 @@
"UnitStructNewType"
]
}
}
},
"additionalProperties": false
},
{
"type": "object",
Expand Down Expand Up @@ -106,7 +108,8 @@
"UnitTwo"
]
}
}
},
"additionalProperties": false
},
{
"type": [
Expand All @@ -124,7 +127,8 @@
"WithInt"
]
}
}
},
"additionalProperties": false
}
]
}
3 changes: 0 additions & 3 deletions schemars/tests/expected/enum-internal.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@
"StringMap"
]
}
},
"additionalProperties": {
"type": "string"
}
},
{
Expand Down
51 changes: 51 additions & 0 deletions schemars/tests/expected/enum-simple-internal-duf.json
Original file line number Diff line number Diff line change
@@ -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
}
]
}
48 changes: 48 additions & 0 deletions schemars/tests/expected/enum-simple-internal.json
Original file line number Diff line number Diff line change
@@ -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"
]
}
}
}
]
}
18 changes: 18 additions & 0 deletions schemars_derive/src/schema_exprs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})),
Expand All @@ -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());
Expand All @@ -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()
})),
});
Expand Down Expand Up @@ -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()
})),
Expand Down

0 comments on commit c543c15

Please sign in to comment.