brandur / json_schema

A JSON Schema V4 and Hyperschema V4 parser and validator.
MIT License
230 stars 45 forks source link

Update from `0.20.3` -> `0.20.4`+ results in infinite loop #115

Closed jessieay closed 5 years ago

jessieay commented 5 years ago

This is very similar to the issue seen / fixed here: https://github.com/brandur/json_schema/issues/112

I've written a test that reproduces what I am seeing locally:

# test/json_schema/reference_expander_test.rb

  it "is not broken" do
    sample1 = {
      "id" => "http://json-schema.org/draft-04/schema#",
      "$schema" => "http://json-schema.org/draft-04/schema#",
      "properties" => {
        "additionalItems" => {
          "anyOf" => [ { "$ref" => "#" } ]
        }
      }
    }
    schema1 = JsonSchema::Parser.new.parse!(sample1)
    sample2 = {
      "$schema" => "http://json-schema.org/draft-04/hyper-schema#",
      "id" => "http://json-schema.org/draft-04/hyper-schema#",
      "allOf" => [
        { "$ref" => "http://json-schema.org/draft-04/schema#" }
      ]
    }
    schema2 = JsonSchema::Parser.new.parse!(sample2)

    store = JsonSchema::DocumentStore.new
    expander = JsonSchema::ReferenceExpander.new

    store.add_schema(schema1)
    store.add_schema(schema2)

    expander.expand!(schema2, store: store)

    assert schema1.expanded?
    assert schema2.expanded?
  end

learned via trial and error that if I change the value of ref inside "allOf" to "#" this spec passes. So, similar to the other fix, it appears that this infinite loop is happening somewhere within `dereference.

kytrinyx commented 5 years ago

I've got this on my todo list to debug. I have a suspicion about what is going on, but I need to actually trace through!

jessieay commented 5 years ago

Hi @kytrinyx! I was wondering if you might be able to drop me a hint on what you think the issue is here? I would love to help debug but it would be very helpful to have some guidance, especially if you already have an inkling of what's going on.

kytrinyx commented 5 years ago

So sorry @jessieay. I have no recollection of whatever my suspicion was, so I'm debugging this from scratch. My approach is to stick a binding pry in the dereference method and run only the target test, and then eyeball things until something jumps out at me.

Eyeballing this, I think that the problem is that we don't recurse into subschema's schema children in this particular case. There's a point in the dereferencing where the new_schema points to the first schema in your test, and ref is a global reference. So we start manually expanding references here:

https://github.com/brandur/json_schema/blob/e534277c6426290e4243e24e26bc72500ea38f7a/lib/json_schema/reference_expander.rb#L128

But the subschema has a subschema and yet when we call dereference here, it doesn't recurse into it:

https://github.com/brandur/json_schema/blob/e534277c6426290e4243e24e26bc72500ea38f7a/lib/json_schema/reference_expander.rb#L166

So I think that what we have to do is some extra work here in the case where parent_ref is not nil, because it calls dereference on the subschema, and then we end up in the infinite loop.

https://github.com/brandur/json_schema/blob/e534277c6426290e4243e24e26bc72500ea38f7a/lib/json_schema/reference_expander.rb#L94-L103

kytrinyx commented 5 years ago

Thinking out loud...

At that spot, ref_schema.uri is the same as subschema.reference.uri.to_s.... I wonder why one is a URI object and one is a String. Whatever the case, though, I think that when they are equivalent, we need to stop dereferencing.