Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uri reference is invalid #77

Closed
casualjim opened this issue Jan 25, 2015 · 7 comments
Closed

uri reference is invalid #77

casualjim opened this issue Jan 25, 2015 · 7 comments

Comments

@casualjim
Copy link

why is a uri reference invalid? The rfc says they are valid uris
and the json schema spec does not forbid them afaict: http://json-schema.org/latest/json-schema-validation.html#anchor123

but this test exists:
https://github.com/json-schema/JSON-Schema-Test-Suite/blob/468fa788ca2053a69815c43abc500e044086120c/tests/draft4/optional/format.json#L39

so I'm curious to find out why

@fge
Copy link
Contributor

fge commented Jan 25, 2015

This is indeed a bug; this test should be removed.

Or at least it should be modified so that this test passes.

@Julian
Copy link
Member

Julian commented Jan 25, 2015

Uh, yeah, I remember this -- I forgot if we got to a conclusion (obviously not if we didn't change the test back again) but I think you're right -- the draft 3 and draft 4 specs are very confusing, but they allow this (unfortunately). We need to fix it in draft 5 though, having URI references be valid is extremely surprising to people.

@hasselmm
Copy link

See http://tools.ietf.org/html/rfc3986#section-5.4.1 or just verify with any well tested library code, e.g. Qt's QUrl in strict parsing mode. The test is plain wrong.

@Julian
Copy link
Member

Julian commented Oct 11, 2015

I'm not sure what you're pointing out with that (that's not a relevant section) or what you mean by checking Qt. The spec is contradictory, we'll address it in draft 5. I'm not sure what to do about the test in the meantime, I have no issue either dropping or leaving it but we've flip flopped twice now, so I'm inclined to leave it given how long it's been in, until draft 5 is out.

@sgpinkus
Copy link

sgpinkus commented Jun 14, 2016

👍 . So change to Absolute URI? This is not an "absolute URI" either -

"description": "a valid protocol-relative URI",
. That should be update too.

@handrews
Copy link
Contributor

@casualjim Note: this was fixed in draft 5 by adding the "uriref" format and reserving "uri" for non-references only.

@Julian
Copy link
Member

Julian commented Jan 26, 2017

Closing this, should be addressed properly now in Draft 6

@Julian Julian closed this as completed Jan 26, 2017
Julian added a commit that referenced this issue Feb 3, 2017
Plenty of confusion still about Draft 3/4's URI, but
that's already being addressed in Draft 6.

After this PR, the tests here are at least internally
consistent (at this point they all consider only URIs
not URI references).

Refs: #58, #77, #156

* awwright/master:
  Flip protocol-relative URI Reference (not a URI)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants