From fc8197cd639c6b59c28d7683195d43298c0fb622 Mon Sep 17 00:00:00 2001 From: Iain Beeston Date: Tue, 14 Mar 2017 09:08:58 +0000 Subject: [PATCH] Made sure we report strict validation errors for draft1 and draft2 schemas We've had a bug for a while where strict validation didn't work for draft1 and draft2 schemas. Strict validation should raise errors if there are properties in the data that are not defined in the schema. However, this was only happening with draft3 and draft4 schemas. This error slipped through because the tests for strict validation were silently switching to either draft4 or the default schema version, and draft3 and draft4 did implement strict validation correctly. So in practice we were never really testing draft1 and draft2 with strict validation. I've fixed this by refactoring the code that draft3 and draft4 used for handling unrecognized properties (under strict validation) such that it could also be used by draft1 and draft2, and fixed up the strict validation tests so that they use the correct json-schema draft. --- CHANGELOG.md | 1 + lib/json-schema/attributes/properties.rb | 32 +---- .../attributes/properties_optional.rb | 34 +++++ test/draft1_test.rb | 5 +- test/draft2_test.rb | 5 +- test/draft3_test.rb | 6 +- test/draft4_test.rb | 6 +- test/support/strict_validation.rb | 129 +++++++++--------- 8 files changed, 118 insertions(+), 100 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1be4860a..6a4a70d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Rescue URI error when initializing a data string that contains a colon - Fragments with an odd number of components no longer raise an `undefined method `validate'` error +- Made strict validation report errors for draft1 and draft2 schemas ## [2.8.0] - 2017-02-07 diff --git a/lib/json-schema/attributes/properties.rb b/lib/json-schema/attributes/properties.rb index cbb81125..19689baa 100644 --- a/lib/json-schema/attributes/properties.rb +++ b/lib/json-schema/attributes/properties.rb @@ -1,8 +1,8 @@ -require 'json-schema/attribute' +require 'json-schema/attributes/properties_optional' module JSON class Schema - class PropertiesAttribute < Attribute + class PropertiesAttribute < PropertiesOptionalAttribute def self.required?(schema, options) schema.fetch('required') { options[:strict] } end @@ -33,33 +33,7 @@ def self.validate(current_schema, data, fragments, processor, validator, options end end - # When strict is true, ensure no undefined properties exist in the data - return unless options[:strict] == true && !schema.key?('additionalProperties') - - diff = data.select do |k, v| - k = k.to_s - - if schema.has_key?('patternProperties') - match = false - schema['patternProperties'].each do |property, property_schema| - regexp = Regexp.new(property) - if regexp.match(k) - match = true - break - end - end - - !schema['properties'].has_key?(k) && !match - else - !schema['properties'].has_key?(k) - end - end - - if diff.size > 0 - properties = diff.keys.join(', ') - message = "The property '#{build_fragment(fragments)}' contained undefined properties: '#{properties}'" - validation_error(processor, message, fragments, current_schema, self, options[:record_errors]) - end + validate_unrecognized_properties(current_schema, data, fragments, processor, validator, options) end end end diff --git a/lib/json-schema/attributes/properties_optional.rb b/lib/json-schema/attributes/properties_optional.rb index 0468676c..304e252d 100644 --- a/lib/json-schema/attributes/properties_optional.rb +++ b/lib/json-schema/attributes/properties_optional.rb @@ -19,6 +19,40 @@ def self.validate(current_schema, data, fragments, processor, validator, options expected_schema = JSON::Schema.new(property_schema, current_schema.uri, validator) expected_schema.validate(data[property], fragments + [property], processor, options) end + + validate_unrecognized_properties(current_schema, data, fragments, processor, validator, options) + end + end + + def self.validate_unrecognized_properties(current_schema, data, fragments, processor, validator, options = {}) + schema = current_schema.schema + + # When strict is true, ensure no undefined properties exist in the data + return unless options[:strict] == true && !schema.key?('additionalProperties') + + diff = data.select do |k, v| + k = k.to_s + + if schema.has_key?('patternProperties') + match = false + schema['patternProperties'].each do |property, property_schema| + regexp = Regexp.new(property) + if regexp.match(k) + match = true + break + end + end + + !schema['properties'].has_key?(k) && !match + else + !schema['properties'].has_key?(k) + end + end + + if diff.size > 0 + properties = diff.keys.join(', ') + message = "The property '#{build_fragment(fragments)}' contained undefined properties: '#{properties}'" + validation_error(processor, message, fragments, current_schema, self, options[:record_errors]) end end end diff --git a/test/draft1_test.rb b/test/draft1_test.rb index c15d7930..ec5ea8ec 100644 --- a/test/draft1_test.rb +++ b/test/draft1_test.rb @@ -2,7 +2,7 @@ class Draft1Test < Minitest::Test def validation_errors(schema, data, options) - super(schema, data, :version => :draft1) + super(schema, data, options.merge(:version => :draft1)) end def exclusive_minimum @@ -22,7 +22,8 @@ def exclusive_maximum include ObjectValidation::AdditionalPropertiesTests - include StrictValidation + include StrictValidation::PropertiesTests + include StrictValidation::AdditionalPropertiesTests include StringValidation::ValueTests include StringValidation::FormatTests diff --git a/test/draft2_test.rb b/test/draft2_test.rb index 7512be76..3dab383d 100644 --- a/test/draft2_test.rb +++ b/test/draft2_test.rb @@ -2,7 +2,7 @@ class Draft2Test < Minitest::Test def validation_errors(schema, data, options) - super(schema, data, :version => :draft2) + super(schema, data, options.merge(:version => :draft2)) end def exclusive_minimum @@ -28,7 +28,8 @@ def multiple_of include ObjectValidation::AdditionalPropertiesTests - include StrictValidation + include StrictValidation::PropertiesTests + include StrictValidation::AdditionalPropertiesTests include StringValidation::ValueTests include StringValidation::FormatTests diff --git a/test/draft3_test.rb b/test/draft3_test.rb index 9a7e58de..86732e2a 100644 --- a/test/draft3_test.rb +++ b/test/draft3_test.rb @@ -3,7 +3,7 @@ class Draft3Test < Minitest::Test def validation_errors(schema, data, options) - super(schema, data, :version => :draft3) + super(schema, data, options.merge(:version => :draft3)) end def exclusive_minimum @@ -31,7 +31,9 @@ def multiple_of include ObjectValidation::AdditionalPropertiesTests include ObjectValidation::PatternPropertiesTests - include StrictValidation + include StrictValidation::PropertiesTests + include StrictValidation::AdditionalPropertiesTests + include StrictValidation::PatternPropertiesTests include StringValidation::ValueTests include StringValidation::FormatTests diff --git a/test/draft4_test.rb b/test/draft4_test.rb index a93b4b12..2fd2f400 100644 --- a/test/draft4_test.rb +++ b/test/draft4_test.rb @@ -3,7 +3,7 @@ class Draft4Test < Minitest::Test def validation_errors(schema, data, options) - super(schema, data, :version => :draft4) + super(schema, data, options.merge(:version => :draft4)) end def exclusive_minimum @@ -31,7 +31,9 @@ def ipv4_format include ObjectValidation::AdditionalPropertiesTests include ObjectValidation::PatternPropertiesTests - include StrictValidation + include StrictValidation::PropertiesTests + include StrictValidation::AdditionalPropertiesTests + include StrictValidation::PatternPropertiesTests include StringValidation::ValueTests include StringValidation::FormatTests diff --git a/test/support/strict_validation.rb b/test/support/strict_validation.rb index 51472a19..e6d3036c 100644 --- a/test/support/strict_validation.rb +++ b/test/support/strict_validation.rb @@ -1,88 +1,91 @@ module StrictValidation - def test_strict_properties - schema = { - "$schema" => "http://json-schema.org/draft-04/schema#", - "properties" => { - "a" => {"type" => "string"}, - "b" => {"type" => "string"} + module PropertiesTests + def test_strict_properties + schema = { + "properties" => { + "a" => {"type" => "string"}, + "b" => {"type" => "string"} + } } - } - data = {"a" => "a"} - assert(!JSON::Validator.validate(schema,data,:strict => true)) + data = {"a" => "a"} + refute_valid schema,data,:strict => true - data = {"b" => "b"} - assert(!JSON::Validator.validate(schema,data,:strict => true)) + data = {"b" => "b"} + refute_valid schema,data,:strict => true - data = {"a" => "a", "b" => "b"} - assert(JSON::Validator.validate(schema,data,:strict => true)) + data = {"a" => "a", "b" => "b"} + assert_valid schema,data,:strict => true - data = {"a" => "a", "b" => "b", "c" => "c"} - assert(!JSON::Validator.validate(schema,data,:strict => true)) - end + data = {"a" => "a", "b" => "b", "c" => "c"} + refute_valid schema,data,:strict => true + end - def test_strict_error_message - schema = { :type => 'object', :properties => { :a => { :type => 'string' } } } - data = { :a => 'abc', :b => 'abc' } - errors = JSON::Validator.fully_validate(schema,data,:strict => true) - assert_match("The property '#/' contained undefined properties: 'b' in schema", errors[0]) + def test_strict_error_message + schema = { :type => 'object', :properties => { :a => { :type => 'string' } } } + data = { :a => 'abc', :b => 'abc' } + errors = JSON::Validator.fully_validate(schema,data,:strict => true) + assert_match("The property '#/' contained undefined properties: 'b' in schema", errors[0]) + end end - def test_strict_properties_additional_props - schema = { - "$schema" => "http://json-schema.org/draft-04/schema#", - "properties" => { - "a" => {"type" => "string"}, - "b" => {"type" => "string"} - }, - "additionalProperties" => {"type" => "integer"} - } + module AdditionalPropertiesTests + def test_strict_properties_additional_props + schema = { + "properties" => { + "a" => {"type" => "string"}, + "b" => {"type" => "string"} + }, + "additionalProperties" => {"type" => "integer"} + } - data = {"a" => "a"} - assert(!JSON::Validator.validate(schema,data,:strict => true)) + data = {"a" => "a"} + refute_valid schema,data,:strict => true - data = {"b" => "b"} - assert(!JSON::Validator.validate(schema,data,:strict => true)) + data = {"b" => "b"} + refute_valid schema,data,:strict => true - data = {"a" => "a", "b" => "b"} - assert(JSON::Validator.validate(schema,data,:strict => true)) + data = {"a" => "a", "b" => "b"} + assert_valid schema,data,:strict => true - data = {"a" => "a", "b" => "b", "c" => "c"} - assert(!JSON::Validator.validate(schema,data,:strict => true)) + data = {"a" => "a", "b" => "b", "c" => "c"} + refute_valid schema,data,:strict => true - data = {"a" => "a", "b" => "b", "c" => 3} - assert(JSON::Validator.validate(schema,data,:strict => true)) + data = {"a" => "a", "b" => "b", "c" => 3} + assert_valid schema,data,:strict => true + end end - def test_strict_properties_pattern_props - schema = { - "properties" => { - "a" => {"type" => "string"}, - "b" => {"type" => "string"} - }, - "patternProperties" => {"\\d+ taco" => {"type" => "integer"}} - } + module PatternPropertiesTests + def test_strict_properties_pattern_props + schema = { + "properties" => { + "a" => {"type" => "string"}, + "b" => {"type" => "string"} + }, + "patternProperties" => {"\\d+ taco" => {"type" => "integer"}} + } - data = {"a" => "a"} - assert(!JSON::Validator.validate(schema,data,:strict => true)) + data = {"a" => "a"} + refute_valid schema,data,:strict => true - data = {"b" => "b"} - assert(!JSON::Validator.validate(schema,data,:strict => true)) + data = {"b" => "b"} + refute_valid schema,data,:strict => true - data = {"a" => "a", "b" => "b"} - assert(JSON::Validator.validate(schema,data,:strict => true)) + data = {"a" => "a", "b" => "b"} + assert_valid schema,data,:strict => true - data = {"a" => "a", "b" => "b", "c" => "c"} - assert(!JSON::Validator.validate(schema,data,:strict => true)) + data = {"a" => "a", "b" => "b", "c" => "c"} + refute_valid schema,data,:strict => true - data = {"a" => "a", "b" => "b", "c" => 3} - assert(!JSON::Validator.validate(schema,data,:strict => true)) + data = {"a" => "a", "b" => "b", "c" => 3} + refute_valid schema,data,:strict => true - data = {"a" => "a", "b" => "b", "23 taco" => 3} - assert(JSON::Validator.validate(schema,data,:strict => true)) + data = {"a" => "a", "b" => "b", "23 taco" => 3} + assert_valid schema,data,:strict => true - data = {"a" => "a", "b" => "b", "23 taco" => "cheese"} - assert(!JSON::Validator.validate(schema,data,:strict => true)) + data = {"a" => "a", "b" => "b", "23 taco" => "cheese"} + refute_valid schema,data,:strict => true + end end - end