Skip to content

Commit d5d5900

Browse files
davishmcclurgdavishmcclurg
committed
Limit anyOf/oneOf discriminator to listed refs
This addresses an issue where `resolve_ref` gets called for refs that aren't explicitly listed in `anyOf` or `oneOf`. The [specification][0] says: > In both the oneOf and anyOf use cases, all possible schemas MUST be listed explicitly. So for anyOf/oneOf discriminators, this uses the provided refs to build a mapping of property values to schema objects for lookup during validation. Explicit mappings are found by schema name or full ref to support: ```yaml discriminator: propertyName: petType mapping: cat: Cat dog: '#/components/schemas/Dog' ``` Impicit mappings are only created for refs under `#/components/schemas/` and are overridden by any explicit mappings that point to the same schema. `allOf` discriminators still resolve refs because there isn't an explicit list of allowed refs. Switched to `Schema#ref` now that it calls `root.resolve_ref` itself. `FIXED_FIELD_REGEX` comes from the [spec][1]: > All the fixed fields declared above are objects that MUST use keys that match the regular expression: ^[a-zA-Z0-9\.\-_]+$. It's used in all cases to make sure schemas are only looked up by name if the property value is a valid schema name. Closes: #144 [0]: https://spec.openapis.org/oas/v3.1.0#discriminator-object [1]: https://spec.openapis.org/oas/v3.1.0#components-object
1 parent e5ca2b1 commit d5d5900

File tree

3 files changed

+170
-32
lines changed

3 files changed

+170
-32
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
### Bug Fixes
66

7+
- Limit anyOf/oneOf discriminator to listed refs
78
- Require discriminator `propertyName` property
89
- Support `Schema#ref` in subschemas
910

‎lib/json_schemer/openapi31/vocab/base.rb‎

Lines changed: 65 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -34,52 +34,89 @@ def validate(*)
3434
end
3535

3636
class Discriminator < Keyword
37-
include Format::JSONPointer
37+
# https://spec.openapis.org/oas/v3.1.0#components-object
38+
FIXED_FIELD_REGEX = /\A[a-zA-Z0-9\.\-_]+$\z/
3839

3940
attr_accessor :skip_ref_once
4041

4142
def error(formatted_instance_location:, **)
4243
"value at #{formatted_instance_location} does not match `discriminator` schema"
4344
end
4445

45-
def validate(instance, instance_location, keyword_location, context)
46-
property_name = value.fetch('propertyName')
47-
mapping = value['mapping'] || {}
46+
def mapping
47+
@mapping ||= value['mapping'] || {}
48+
end
4849

49-
return result(instance, instance_location, keyword_location, true) unless instance.is_a?(Hash)
50-
return result(instance, instance_location, keyword_location, false) unless instance.key?(property_name)
50+
def subschemas_by_property_value
51+
@subschemas_by_property_value ||= if schema.parsed.key?('anyOf') || schema.parsed.key?('oneOf')
52+
subschemas = schema.parsed['anyOf']&.parsed || []
53+
subschemas += schema.parsed['oneOf']&.parsed || []
5154

52-
property = instance.fetch(property_name)
53-
ref = mapping.fetch(property, property)
55+
subschemas_by_ref = {}
56+
subschemas_by_schema_name = {}
57+
58+
subschemas.each do |subschema|
59+
subschema_ref = subschema.parsed.fetch('$ref').parsed
60+
subschemas_by_ref[subschema_ref] = subschema
5461

55-
ref_schema = nil
56-
unless ref.start_with?('#') && valid_json_pointer?(ref.delete_prefix('#'))
57-
ref_schema = begin
58-
root.resolve_ref(URI.join(schema.base_uri, "#/components/schemas/#{ref}"))
59-
rescue InvalidRefPointer
60-
nil
62+
if subschema_ref.start_with?('#/components/schemas/')
63+
schema_name = subschema_ref.delete_prefix('#/components/schemas/')
64+
subschemas_by_schema_name[schema_name] = subschema if FIXED_FIELD_REGEX.match?(schema_name)
65+
end
6166
end
62-
end
63-
ref_schema ||= root.resolve_ref(URI.join(schema.base_uri, ref))
6467

65-
return if skip_ref_once == ref_schema.absolute_keyword_location
68+
explicit_mapping = mapping.transform_values do |schema_name_or_ref|
69+
subschemas_by_schema_name.fetch(schema_name_or_ref) { subschemas_by_ref.fetch(schema_name_or_ref) }
70+
end
6671

67-
nested = []
72+
implicit_mapping = subschemas_by_schema_name.reject do |_schema_name, subschema|
73+
explicit_mapping.value?(subschema)
74+
end
6875

69-
if schema.parsed.key?('anyOf') || schema.parsed.key?('oneOf')
70-
subschemas = schema.parsed['anyOf']&.parsed || []
71-
subschemas += schema.parsed['oneOf']&.parsed || []
72-
subschemas.each do |subschema|
73-
if subschema.parsed.fetch('$ref').ref_schema.absolute_keyword_location == ref_schema.absolute_keyword_location
74-
nested << subschema.validate_instance(instance, instance_location, keyword_location, context)
76+
implicit_mapping.merge(explicit_mapping)
77+
else
78+
Hash.new do |hash, property_value|
79+
schema_name_or_ref = mapping.fetch(property_value, property_value)
80+
81+
subschema = nil
82+
83+
if FIXED_FIELD_REGEX.match?(schema_name_or_ref)
84+
subschema = begin
85+
schema.ref("#/components/schemas/#{schema_name_or_ref}")
86+
rescue InvalidRefPointer
87+
nil
88+
end
7589
end
90+
91+
subschema ||= begin
92+
schema.ref(schema_name_or_ref)
93+
rescue InvalidRefResolution, UnknownRef
94+
nil
95+
end
96+
97+
hash[property_value] = subschema
7698
end
77-
else
78-
ref_schema.parsed['allOf']&.skip_ref_once = schema.absolute_keyword_location
79-
nested << ref_schema.validate_instance(instance, instance_location, keyword_location, context)
8099
end
100+
end
101+
102+
def validate(instance, instance_location, keyword_location, context)
103+
return result(instance, instance_location, keyword_location, true) unless instance.is_a?(Hash)
104+
105+
property_name = value.fetch('propertyName')
106+
107+
return result(instance, instance_location, keyword_location, false) unless instance.key?(property_name)
108+
109+
property_value = instance.fetch(property_name)
110+
subschema = subschemas_by_property_value[property_value]
111+
112+
return result(instance, instance_location, keyword_location, false) unless subschema
113+
114+
return if skip_ref_once == subschema.absolute_keyword_location
115+
subschema.parsed['allOf']&.skip_ref_once = schema.absolute_keyword_location
116+
117+
subschema_result = subschema.validate_instance(instance, instance_location, keyword_location, context)
81118

82-
result(instance, instance_location, keyword_location, (nested.any? && nested.all?(&:valid)), nested)
119+
result(instance, instance_location, keyword_location, subschema_result.valid, subschema_result.nested)
83120
ensure
84121
self.skip_ref_once = nil
85122
end

‎test/open_api_test.rb‎

Lines changed: 104 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ def test_discriminator_specification_example
207207
assert_equal([['format', '/components/schemas/Dog/allOf/1/properties/packSize']], schemer.validate(invalid_pack_size).map { |error| error.values_at('type', 'schema_pointer') })
208208
assert_equal([['required', '/components/schemas/Pet'], ['discriminator', '/components/schemas/Pet']], schemer.validate(missing_pet_type).map { |error| error.values_at('type', 'schema_pointer') })
209209
assert_equal([['required', '/components/schemas/Pet'], ['required', '/components/schemas/Cat/allOf/1']], schemer.validate(missing_name).map { |error| error.values_at('type', 'schema_pointer') })
210-
assert_raises(JSONSchemer::UnknownRef) { schemer.validate(invalid_pet_type) }
210+
assert_equal([['discriminator', '/components/schemas/Pet']], schemer.validate(invalid_pet_type).map { |error| error.values_at('type', 'schema_pointer') })
211211
end
212212

213213
def test_all_of_discriminator
@@ -447,6 +447,51 @@ def test_all_of_discriminator_with_non_discriminator_ref
447447
assert_equal([['required', '/components/schemas/Pet'], ['discriminator', '/components/schemas/Pet'], ['required', '/components/schemas/Other']], schemer.validate({}).map { |error| error.values_at('type', 'schema_pointer') })
448448
end
449449

450+
def test_all_of_discriminator_with_remote_ref
451+
schema = {
452+
'$id' => 'http://example.com/schema',
453+
'discriminator' => {
454+
'propertyName' => 'petType',
455+
'mapping' => {
456+
'Dog' => 'http://example.com/dog'
457+
}
458+
}
459+
}
460+
schemer = JSONSchemer.schema(
461+
schema,
462+
:meta_schema => JSONSchemer.openapi31,
463+
:ref_resolver => {
464+
URI('http://example.com/schema') => schema,
465+
URI('http://example.com/cat') => {
466+
'allOf' => [
467+
{ '$ref' => 'http://example.com/schema' },
468+
CAT_SCHEMA
469+
]
470+
},
471+
URI('http://example.com/dog') => {
472+
'allOf' => [
473+
{ '$ref' => 'http://example.com/schema' },
474+
DOG_SCHEMA
475+
]
476+
}
477+
}.to_proc
478+
)
479+
480+
assert(schemer.valid_schema?)
481+
refute(schemer.valid?(CAT))
482+
assert(schemer.valid?(CAT.merge('petType' => 'http://example.com/cat')))
483+
assert(schemer.valid?(DOG))
484+
485+
invalid_cat = INVALID_CAT.merge('petType' => 'http://example.com/cat')
486+
invalid_cat_result = schemer.validate(invalid_cat, output_format: 'basic', resolve_enumerators: true)
487+
assert_equal('/discriminator/allOf/1/properties/name/type', invalid_cat_result.dig('errors', 0, 'keywordLocation'))
488+
assert_equal('http://example.com/cat#/allOf/1/properties/name/type', invalid_cat_result.dig('errors', 0, 'absoluteKeywordLocation'))
489+
490+
invalid_dog_result = schemer.validate(INVALID_DOG, output_format: 'basic', resolve_enumerators: true)
491+
assert_equal('/discriminator/allOf/1/properties/bark/type', invalid_dog_result.dig('errors', 0, 'keywordLocation'))
492+
assert_equal('http://example.com/dog#/allOf/1/properties/bark/type', invalid_dog_result.dig('errors', 0, 'absoluteKeywordLocation'))
493+
end
494+
450495
def test_any_of_discriminator_without_matching_schema
451496
openapi = {
452497
'openapi' => '3.1.0',
@@ -513,6 +558,60 @@ def test_one_of_discriminator_without_matching_schema
513558
assert_equal([['discriminator', '/components/schemas/MyResponseType']], schemer.validate(INVALID_LIZARD).map { |error| error.values_at('type', 'schema_pointer') })
514559
end
515560

561+
def test_any_of_discriminator_ignores_nested_schemas
562+
openapi = {
563+
'openapi' => '3.1.0',
564+
'components' => {
565+
'schemas' => {
566+
'MyResponseType' => {
567+
'anyOf' => [
568+
{ '$ref' => '#/components/schemas/Cat' },
569+
{ '$ref' => '#/components/schemas/Cat/$defs/nah' }
570+
],
571+
'discriminator' => {
572+
'propertyName' => 'petType'
573+
}
574+
},
575+
'Cat' => CAT_SCHEMA.merge('$defs' => { 'nah' => {} })
576+
}
577+
}
578+
}
579+
580+
schemer = JSONSchemer.openapi(openapi).schema('MyResponseType')
581+
582+
assert(schemer.valid_schema?)
583+
assert(schemer.valid?(CAT))
584+
refute(schemer.valid?(CAT.merge('petType' => 'nah')))
585+
refute(schemer.valid?(CAT.merge('petType' => 'Cat/$defs/nah')))
586+
end
587+
588+
def test_one_of_discriminator_ignores_nested_schemas
589+
openapi = {
590+
'openapi' => '3.1.0',
591+
'components' => {
592+
'schemas' => {
593+
'MyResponseType' => {
594+
'oneOf' => [
595+
{ '$ref' => '#/components/schemas/Cat' },
596+
{ '$ref' => '#/components/schemas/Cat/$defs/nah' }
597+
],
598+
'discriminator' => {
599+
'propertyName' => 'petType'
600+
}
601+
},
602+
'Cat' => CAT_SCHEMA.merge('$defs' => { 'nah' => {} })
603+
}
604+
}
605+
}
606+
607+
schemer = JSONSchemer.openapi(openapi).schema('MyResponseType')
608+
609+
assert(schemer.valid_schema?)
610+
assert(schemer.valid?(CAT))
611+
refute(schemer.valid?(CAT.merge('petType' => 'nah')))
612+
refute(schemer.valid?(CAT.merge('petType' => 'Cat/$defs/nah')))
613+
end
614+
516615
def test_discrimator_mapping
517616
openapi = {
518617
'openapi' => '3.1.0',
@@ -542,7 +641,7 @@ def test_discrimator_mapping
542641

543642
assert(schemer.valid_schema?)
544643
assert(schemer.valid?(CAT.merge('petType' => 'c')))
545-
assert(schemer.valid?(MISTY.merge('petType' => 'Cat')))
644+
refute(schemer.valid?(MISTY.merge('petType' => 'Cat')))
546645
assert_equal(['/components/schemas/Cat/properties/name'], schemer.validate(INVALID_CAT.merge('petType' => 'c')).map { |error| error.fetch('schema_pointer') })
547646
assert(schemer.valid?(DOG.merge('petType' => 'd')))
548647
assert_equal(['/components/schemas/Dog/properties/bark'], schemer.validate(INVALID_DOG.merge('petType' => 'dog')).map { |error| error.fetch('schema_pointer') })
@@ -585,8 +684,9 @@ def test_non_json_pointer_discriminator
585684
assert(schemer.valid?(CAT))
586685
assert(schemer.valid?(MISTY))
587686
assert_equal(['/components/schemas/Cat/properties/name'], schemer.validate(INVALID_CAT).map { |error| error.fetch('schema_pointer') })
588-
assert(schemer.valid?(DOG))
589-
assert_equal(['/components/schemas/Dog/properties/bark'], schemer.validate(INVALID_DOG).map { |error| error.fetch('schema_pointer') })
687+
refute(schemer.valid?(DOG))
688+
assert_equal(['/components/schemas/MyResponseType'], schemer.validate(INVALID_DOG).map { |error| error.fetch('schema_pointer') })
689+
assert_equal(['/components/schemas/Dog/properties/bark'], schemer.validate(INVALID_DOG.merge('petType' => 'dog')).map { |error| error.fetch('schema_pointer') })
590690
assert(schemer.valid?(LIZARD))
591691
assert_equal(['/components/schemas/Lizard/properties/lovesRocks'], schemer.validate(INVALID_LIZARD).map { |error| error.fetch('schema_pointer') })
592692
assert(schemer.valid?(MONSTER))

0 commit comments

Comments
 (0)