Fix bug when ref_schema doesn't have a URI.

[lukeschlather]

I'm not quite sure this is the right thing to do. It seems better than the current behavior, which blindly trys to chomp schema_ref.uri.

[brandur]

Hi @lukeschlather,

Thanks a lot for the contribution here, and sorry about the crazy delay in response.

So just looking over this, it kind of looks like if your proposed change will just drop through to try and resolve by JSON pointer only, which doesn't really seem right either. Do you think it might be a good idea to try and error out under this condition?

[alexleigh]

I have a different fix at https://github.com/alexleigh/json_schema/commit/07064ad180b564894532105bf170864b4ac6473b where I try to avoid this error by just assigning an uri to reference schema. I didn't submit a PR because I'm not sure if my solution is any more correct.

[lukeschlather]

I think it is appropriate to error in this case. (I ran into this when hand-managing a schema and I made a mistake.)

I believe this should fall through and error out at https://github.com/lukeschlather/json_schema/blob/6d3fd7eab0911eb612ba5c42e8db144feaec8dc9/lib/json_schema/reference_expander.rb#L138

My intention was just to get rid of the chomp backtrace and provide a friendlier error. @alexleigh is your thing essentially doing the same or do you want this to work (not error out.)

[alexleigh]

My intention was to try to get this to work. My use case is to make reference expander work to resolve relative JSON references such as "other-schema.json#" even when none of the schema files have an "id". But I soon realized with the way this library relies on URIs to resolve references that a lot more work needs to be done to make local file references work in the absence of IDs. So my fix doesn't get far enough to do what I wanted it to work, but I also don't think this case should trigger an exception as attempting to resolve a JSON reference when the schema doesn't have an ID should be allowed.

[brandur]

Thanks for the quick responses here @lukeschlather and @alexleigh.

@lukeschlather I put a slightly different fix into #68 that does the same as what you're trying to do here, but without attempted pointer dereference, and adds a test case. Does it look okay to you?

@alexleigh Interesting. Admittedly relative URIs might be quite useful here. Do you happen to know whether that approach is kosher according to the JSON reference spec? If so, we should consider trying to port your fix back into master.

[brandur]

Think about this a little more, I'm not 100% sure that relative URIs can/should even have meaning the way that this project is currently implemented because it's not clear what they'd be referring to. It's possible that the relative path should always return a reference expansion error :$

[alexleigh]

Based on my reading of the RFC, using a relative JSON reference in a JSON schema when that schema doesn't have an "id" attribute is valid in JSON Schema draft 4. A JSON reference such as "other-file.json#" is itself a valid relative JSON reference, and since in draft 4 the "id" attribute is used to alter the URI resolution scope, in the absence of the "id" attribute a relative JSON reference is resolved based on the unaltered URI of the containing schema. I agree that this doesn't work with this library as written, since the library doesn't know how a JSON schema is loaded and thus can't construct the schema URI when the schema doesn't have an "id" attribute. To make something like this work we need at the very least a variant of JsonSchema.parse! that takes a URI or path. Both https://github.com/daveclayton/json-schema-validator and https://github.com/ruby-json-schema/json-schema rely on this style of loading schema files to resolve relative JSON references when IDs are absent.

[brandur]

@alexleigh Thanks!

To make something like this work we need at the very least a variant of JsonSchema.parse! that takes a URI or path.

I'd mostly prefer for the library to stay is in that it never tries to actually resolve new schemas based on a file system location or HTTP address, and instead relies on everything being registered in advance. Such an approach carries potential security and performance considerations that scare me a bit.

That said, I can imagine something like you suggest here in that all we really have to do is load a path worth of schemas and then assign any that are missing a URI one that's generated based off where we found it on the filesystem. Schemas could then use that relative path to reference each other.

[brandur]

I'm going to close this pull for now because I ended up pulling in #68 just so we don't have a nil error that occurs, but I'd certainly consider other improvements to how this should work.