Skip to content

Commit fc8197c

Browse files
iainbeestoniainbeeston
committed
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.
1 parent 2f95d53 commit fc8197c

File tree

8 files changed

+118
-100
lines changed

8 files changed

+118
-100
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
1414
- Rescue URI error when initializing a data string that contains a colon
1515
- Fragments with an odd number of components no longer raise an `undefined method `validate'`
1616
error
17+
- Made strict validation report errors for draft1 and draft2 schemas
1718

1819
## [2.8.0] - 2017-02-07
1920

‎lib/json-schema/attributes/properties.rb‎

Lines changed: 3 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
require 'json-schema/attribute'
1+
require 'json-schema/attributes/properties_optional'
22

33
module JSON
44
class Schema
5-
class PropertiesAttribute < Attribute
5+
class PropertiesAttribute < PropertiesOptionalAttribute
66
def self.required?(schema, options)
77
schema.fetch('required') { options[:strict] }
88
end
@@ -33,33 +33,7 @@ def self.validate(current_schema, data, fragments, processor, validator, options
3333
end
3434
end
3535

36-
# When strict is true, ensure no undefined properties exist in the data
37-
return unless options[:strict] == true && !schema.key?('additionalProperties')
38-
39-
diff = data.select do |k, v|
40-
k = k.to_s
41-
42-
if schema.has_key?('patternProperties')
43-
match = false
44-
schema['patternProperties'].each do |property, property_schema|
45-
regexp = Regexp.new(property)
46-
if regexp.match(k)
47-
match = true
48-
break
49-
end
50-
end
51-
52-
!schema['properties'].has_key?(k) && !match
53-
else
54-
!schema['properties'].has_key?(k)
55-
end
56-
end
57-
58-
if diff.size > 0
59-
properties = diff.keys.join(', ')
60-
message = "The property '#{build_fragment(fragments)}' contained undefined properties: '#{properties}'"
61-
validation_error(processor, message, fragments, current_schema, self, options[:record_errors])
62-
end
36+
validate_unrecognized_properties(current_schema, data, fragments, processor, validator, options)
6337
end
6438
end
6539
end

‎lib/json-schema/attributes/properties_optional.rb‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,40 @@ def self.validate(current_schema, data, fragments, processor, validator, options
1919
expected_schema = JSON::Schema.new(property_schema, current_schema.uri, validator)
2020
expected_schema.validate(data[property], fragments + [property], processor, options)
2121
end
22+
23+
validate_unrecognized_properties(current_schema, data, fragments, processor, validator, options)
24+
end
25+
end
26+
27+
def self.validate_unrecognized_properties(current_schema, data, fragments, processor, validator, options = {})
28+
schema = current_schema.schema
29+
30+
# When strict is true, ensure no undefined properties exist in the data
31+
return unless options[:strict] == true && !schema.key?('additionalProperties')
32+
33+
diff = data.select do |k, v|
34+
k = k.to_s
35+
36+
if schema.has_key?('patternProperties')
37+
match = false
38+
schema['patternProperties'].each do |property, property_schema|
39+
regexp = Regexp.new(property)
40+
if regexp.match(k)
41+
match = true
42+
break
43+
end
44+
end
45+
46+
!schema['properties'].has_key?(k) && !match
47+
else
48+
!schema['properties'].has_key?(k)
49+
end
50+
end
51+
52+
if diff.size > 0
53+
properties = diff.keys.join(', ')
54+
message = "The property '#{build_fragment(fragments)}' contained undefined properties: '#{properties}'"
55+
validation_error(processor, message, fragments, current_schema, self, options[:record_errors])
2256
end
2357
end
2458
end

‎test/draft1_test.rb‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
class Draft1Test < Minitest::Test
44
def validation_errors(schema, data, options)
5-
super(schema, data, :version => :draft1)
5+
super(schema, data, options.merge(:version => :draft1))
66
end
77

88
def exclusive_minimum
@@ -22,7 +22,8 @@ def exclusive_maximum
2222

2323
include ObjectValidation::AdditionalPropertiesTests
2424

25-
include StrictValidation
25+
include StrictValidation::PropertiesTests
26+
include StrictValidation::AdditionalPropertiesTests
2627

2728
include StringValidation::ValueTests
2829
include StringValidation::FormatTests

‎test/draft2_test.rb‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
class Draft2Test < Minitest::Test
44
def validation_errors(schema, data, options)
5-
super(schema, data, :version => :draft2)
5+
super(schema, data, options.merge(:version => :draft2))
66
end
77

88
def exclusive_minimum
@@ -28,7 +28,8 @@ def multiple_of
2828

2929
include ObjectValidation::AdditionalPropertiesTests
3030

31-
include StrictValidation
31+
include StrictValidation::PropertiesTests
32+
include StrictValidation::AdditionalPropertiesTests
3233

3334
include StringValidation::ValueTests
3435
include StringValidation::FormatTests

‎test/draft3_test.rb‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
class Draft3Test < Minitest::Test
55
def validation_errors(schema, data, options)
6-
super(schema, data, :version => :draft3)
6+
super(schema, data, options.merge(:version => :draft3))
77
end
88

99
def exclusive_minimum
@@ -31,7 +31,9 @@ def multiple_of
3131
include ObjectValidation::AdditionalPropertiesTests
3232
include ObjectValidation::PatternPropertiesTests
3333

34-
include StrictValidation
34+
include StrictValidation::PropertiesTests
35+
include StrictValidation::AdditionalPropertiesTests
36+
include StrictValidation::PatternPropertiesTests
3537

3638
include StringValidation::ValueTests
3739
include StringValidation::FormatTests

‎test/draft4_test.rb‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
class Draft4Test < Minitest::Test
55
def validation_errors(schema, data, options)
6-
super(schema, data, :version => :draft4)
6+
super(schema, data, options.merge(:version => :draft4))
77
end
88

99
def exclusive_minimum
@@ -31,7 +31,9 @@ def ipv4_format
3131
include ObjectValidation::AdditionalPropertiesTests
3232
include ObjectValidation::PatternPropertiesTests
3333

34-
include StrictValidation
34+
include StrictValidation::PropertiesTests
35+
include StrictValidation::AdditionalPropertiesTests
36+
include StrictValidation::PatternPropertiesTests
3537

3638
include StringValidation::ValueTests
3739
include StringValidation::FormatTests

‎test/support/strict_validation.rb‎

Lines changed: 66 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,88 +1,91 @@
11
module StrictValidation
2-
def test_strict_properties
3-
schema = {
4-
"$schema" => "http://json-schema.org/draft-04/schema#",
5-
"properties" => {
6-
"a" => {"type" => "string"},
7-
"b" => {"type" => "string"}
2+
module PropertiesTests
3+
def test_strict_properties
4+
schema = {
5+
"properties" => {
6+
"a" => {"type" => "string"},
7+
"b" => {"type" => "string"}
8+
}
89
}
9-
}
1010

11-
data = {"a" => "a"}
12-
assert(!JSON::Validator.validate(schema,data,:strict => true))
11+
data = {"a" => "a"}
12+
refute_valid schema,data,:strict => true
1313

14-
data = {"b" => "b"}
15-
assert(!JSON::Validator.validate(schema,data,:strict => true))
14+
data = {"b" => "b"}
15+
refute_valid schema,data,:strict => true
1616

17-
data = {"a" => "a", "b" => "b"}
18-
assert(JSON::Validator.validate(schema,data,:strict => true))
17+
data = {"a" => "a", "b" => "b"}
18+
assert_valid schema,data,:strict => true
1919

20-
data = {"a" => "a", "b" => "b", "c" => "c"}
21-
assert(!JSON::Validator.validate(schema,data,:strict => true))
22-
end
20+
data = {"a" => "a", "b" => "b", "c" => "c"}
21+
refute_valid schema,data,:strict => true
22+
end
2323

24-
def test_strict_error_message
25-
schema = { :type => 'object', :properties => { :a => { :type => 'string' } } }
26-
data = { :a => 'abc', :b => 'abc' }
27-
errors = JSON::Validator.fully_validate(schema,data,:strict => true)
28-
assert_match("The property '#/' contained undefined properties: 'b' in schema", errors[0])
24+
def test_strict_error_message
25+
schema = { :type => 'object', :properties => { :a => { :type => 'string' } } }
26+
data = { :a => 'abc', :b => 'abc' }
27+
errors = JSON::Validator.fully_validate(schema,data,:strict => true)
28+
assert_match("The property '#/' contained undefined properties: 'b' in schema", errors[0])
29+
end
2930
end
3031

31-
def test_strict_properties_additional_props
32-
schema = {
33-
"$schema" => "http://json-schema.org/draft-04/schema#",
34-
"properties" => {
35-
"a" => {"type" => "string"},
36-
"b" => {"type" => "string"}
37-
},
38-
"additionalProperties" => {"type" => "integer"}
39-
}
32+
module AdditionalPropertiesTests
33+
def test_strict_properties_additional_props
34+
schema = {
35+
"properties" => {
36+
"a" => {"type" => "string"},
37+
"b" => {"type" => "string"}
38+
},
39+
"additionalProperties" => {"type" => "integer"}
40+
}
4041

41-
data = {"a" => "a"}
42-
assert(!JSON::Validator.validate(schema,data,:strict => true))
42+
data = {"a" => "a"}
43+
refute_valid schema,data,:strict => true
4344

44-
data = {"b" => "b"}
45-
assert(!JSON::Validator.validate(schema,data,:strict => true))
45+
data = {"b" => "b"}
46+
refute_valid schema,data,:strict => true
4647

47-
data = {"a" => "a", "b" => "b"}
48-
assert(JSON::Validator.validate(schema,data,:strict => true))
48+
data = {"a" => "a", "b" => "b"}
49+
assert_valid schema,data,:strict => true
4950

50-
data = {"a" => "a", "b" => "b", "c" => "c"}
51-
assert(!JSON::Validator.validate(schema,data,:strict => true))
51+
data = {"a" => "a", "b" => "b", "c" => "c"}
52+
refute_valid schema,data,:strict => true
5253

53-
data = {"a" => "a", "b" => "b", "c" => 3}
54-
assert(JSON::Validator.validate(schema,data,:strict => true))
54+
data = {"a" => "a", "b" => "b", "c" => 3}
55+
assert_valid schema,data,:strict => true
56+
end
5557
end
5658

57-
def test_strict_properties_pattern_props
58-
schema = {
59-
"properties" => {
60-
"a" => {"type" => "string"},
61-
"b" => {"type" => "string"}
62-
},
63-
"patternProperties" => {"\\d+ taco" => {"type" => "integer"}}
64-
}
59+
module PatternPropertiesTests
60+
def test_strict_properties_pattern_props
61+
schema = {
62+
"properties" => {
63+
"a" => {"type" => "string"},
64+
"b" => {"type" => "string"}
65+
},
66+
"patternProperties" => {"\\d+ taco" => {"type" => "integer"}}
67+
}
6568

66-
data = {"a" => "a"}
67-
assert(!JSON::Validator.validate(schema,data,:strict => true))
69+
data = {"a" => "a"}
70+
refute_valid schema,data,:strict => true
6871

69-
data = {"b" => "b"}
70-
assert(!JSON::Validator.validate(schema,data,:strict => true))
72+
data = {"b" => "b"}
73+
refute_valid schema,data,:strict => true
7174

72-
data = {"a" => "a", "b" => "b"}
73-
assert(JSON::Validator.validate(schema,data,:strict => true))
75+
data = {"a" => "a", "b" => "b"}
76+
assert_valid schema,data,:strict => true
7477

75-
data = {"a" => "a", "b" => "b", "c" => "c"}
76-
assert(!JSON::Validator.validate(schema,data,:strict => true))
78+
data = {"a" => "a", "b" => "b", "c" => "c"}
79+
refute_valid schema,data,:strict => true
7780

78-
data = {"a" => "a", "b" => "b", "c" => 3}
79-
assert(!JSON::Validator.validate(schema,data,:strict => true))
81+
data = {"a" => "a", "b" => "b", "c" => 3}
82+
refute_valid schema,data,:strict => true
8083

81-
data = {"a" => "a", "b" => "b", "23 taco" => 3}
82-
assert(JSON::Validator.validate(schema,data,:strict => true))
84+
data = {"a" => "a", "b" => "b", "23 taco" => 3}
85+
assert_valid schema,data,:strict => true
8386

84-
data = {"a" => "a", "b" => "b", "23 taco" => "cheese"}
85-
assert(!JSON::Validator.validate(schema,data,:strict => true))
87+
data = {"a" => "a", "b" => "b", "23 taco" => "cheese"}
88+
refute_valid schema,data,:strict => true
89+
end
8690
end
87-
8891
end

0 commit comments

Comments
 (0)