voxpupuli / json-schema

Ruby JSON Schema Validator
MIT License
1.52k stars 242 forks source link

additionalProperties is tested and implemented incorrectly #321

Closed dpatti closed 7 years ago

dpatti commented 8 years ago

This schema is being used by these tests. This schema doesn't make sense to me. My understanding is that additionalProperties is supposed to be a boolean value false or a schema describing what each additional property's value should validate against.

The reason these tests are concerning is because they fail when I tried to fix the following bit in AdditionalPropertiesAttribute, which reads:

JSON::Schema.new(addprop[key] || addprop, current_schema.uri, validator)

What I believe this line should do is instantiate a new schema using the value of the additionalProperties object, but we're trying to access [key] on it. This demonstrates why this is a problem:

schema = {
  properties: {
    foo: {
      type: "string",
    }
  },
  additionalProperties: {
    type: "number",
  }
}

p JSON::Validator.validate(schema, { foo: "one", two: 2, three: 3 }) # => true
p JSON::Validator.validate(schema, { foo: "one", two: 2, three: 3, type: 4 }) # => NoMethodError

With backtrace:

/opt/rubies/2.2.4/lib/ruby/gems/2.2.0/gems/json-schema-2.5.0/lib/json-schema/schema/validator.rb:22:in `validate': undefined method `each' for "number":String (NoMethodError)
  from /opt/rubies/2.2.4/lib/ruby/gems/2.2.0/gems/json-schema-2.5.0/lib/json-schema/schema.rb:33:in `validate'
  from /opt/rubies/2.2.4/lib/ruby/gems/2.2.0/gems/json-schema-2.5.0/lib/json-schema/attributes/additionalproperties.rb:18:in `block in validate'
  from /opt/rubies/2.2.4/lib/ruby/gems/2.2.0/gems/json-schema-2.5.0/lib/json-schema/attributes/additionalproperties.rb:16:in `each'

I want to say this is wrong, but I'm not exactly sure how I should go about fixing something like this. It seems like it would change behavior if people relied on it working the way it is tested. Here's the commit I blamed this back to: https://github.com/ruby-json-schema/json-schema/commit/7fba5303

iainbeeston commented 8 years ago

I've started looking at this. The issue occurs whenever you have a property name in the data that matches a property defined on your additionalProperties object (in this case having a property with the name type in both additionalProperties and in your json data is causing the issue). I haven't got a fix yet but I'm working on it

dpatti commented 8 years ago

@iainbeeston Unless I'm mistaken, you should never be defining properties on additionalProperties. It's either a boolean or a schema. https://github.com/ruby-json-schema/json-schema/blob/master/resources/draft-04.json#L93-L99

iainbeeston commented 7 years ago

Hi @dpatti I believe I've fixed this in #360 and released the fix in json-schema 2.7.0. Using that version of the gem, the example above works without errors.