Skip to content

Commit

Permalink
Made sure we report strict validation errors for draft1 and draft2
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
iainbeeston committed Jul 6, 2017
1 parent 2f95d53 commit fc8197c
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 100 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
32 changes: 3 additions & 29 deletions lib/json-schema/attributes/properties.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions lib/json-schema/attributes/properties_optional.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions test/draft1_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -22,7 +22,8 @@ def exclusive_maximum

include ObjectValidation::AdditionalPropertiesTests

include StrictValidation
include StrictValidation::PropertiesTests
include StrictValidation::AdditionalPropertiesTests

include StringValidation::ValueTests
include StringValidation::FormatTests
Expand Down
5 changes: 3 additions & 2 deletions test/draft2_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -28,7 +28,8 @@ def multiple_of

include ObjectValidation::AdditionalPropertiesTests

include StrictValidation
include StrictValidation::PropertiesTests
include StrictValidation::AdditionalPropertiesTests

include StringValidation::ValueTests
include StringValidation::FormatTests
Expand Down
6 changes: 4 additions & 2 deletions test/draft3_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions test/draft4_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
129 changes: 66 additions & 63 deletions test/support/strict_validation.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit fc8197c

Please sign in to comment.