extended tests for json-pointer format

[CroniD]

See also json-schema-org/json-schema-spec#328

[CroniD]

Well, there are 2 tests failing, as expected. ajv supports "JSON String representation" and "URI Fragment Identifier Representation", but the last one shouldn't be valid.

[epoberezkin]

@CroniD I will have a look

[epoberezkin]

@CroniD Given that RFC6901 defines both JSON string and URI fragment formats for JSON pointer and that the JSON-Schema spec doesn't limit the format to JSON string, I don't think we should have tests here that require that URI fragment format is invalid - I believe that the tests should either be changed to make them valid or just removed to leave this question to the implementations discretion. I don't have any preference which way this is changed in this PR - "not a valid JSON-pointer (URI Fragment Identifier Representation) #1/2" should be either removed or changed to "valid".

We can change it this way or another in the next version of JSON schema spec (i.e. to explicitly specify URI fragment JSON pointer to be valid/invalid according to "json-pointer" format in the schema). I don't have particularly strong opinion how it should be in draft-07, but in draft-06 it should stay the way it is formulated in the spec.

Also, can you please change the order so that the tests that were not changed do not show as both removed and added, so it is easier to see what is added?

[handrews]

@epoberezkin I am asserting that a bug in the spec (careless language) produced incorrect guidance. Having a test suite that enforces the correct, intended behavior, will help us keep that behavior going forwards.

Or do you suggest inverting the minimum/maximum test cases because of the copy-paste errors in those keywords in draft-06?

It's fine to stay with odd behavior if that odd behavior was the intent of a spec, but a bug is a bug and we should not write tests that re-enforce the bug.

[handrews]

The example given to justify adding the json-pointer format is quite clearly NOT talking about using it as a URI fragment: json-schema-org/json-schema-spec#109

[handrews]

I would also argue that, given that the ABNF for JSON Pointer in RFC 9601 has nothing to do wth URI syntax, and the format is specified to apply to JSON Strings, that the JSON String representation is pretty clearly intended to be used. There was ZERO discussion of using URI fragments with this format that I can find.

Any such usage is incorrect and should be caught by the test suite. We will also improve the language.

[epoberezkin]

we should not write tests that re-enforce the bug.

As I wrote, it's ok to remove those tests and leave the decision to implementations until the point when we agree in which way it should be clarified.
I'm not sure why a wider interpretation that is part RFC6901 as well should be seen as a bug. What is wrong with allowing URI fragment format given that it's in RFC6901? Why should JSON schema spec be inconsistent with RFC6901? I can understand that your intention may have been to limit it, but 1) it is not how it is written 2) I am not sure I agree, it has to be discussed.

Or do you suggest inverting the minimum/maximum test cases because of the copy-paste errors in those keywords in draft-06?

Do we have a copy paste error in the spec or is it just a sarcasm? In any case, the analogy is completely invalid - allowing URI fragments makes sense. Inverting test cases for minimum/maximum makes no sense.

Also, the spec is primary (normative), the tests are secondary (informative), using tests to clarify the spec is not a valid approach. @awwright would agree I think.

[epoberezkin]

@handrews I think that the argument in the second paragraph of https://tools.ietf.org/html/rfc6901#section-6 is supportive of what you write.

@CroniD I suggest a couple of options then - 1) remove the tests in question, merge this PR, then make another PR with these two tests and merge when I update Ajv. 2) wait till I update the Ajv (a week or so) and then merge. 3) update json-pointer format that Ajv uses in the test here: ajv.addFormat('json-pointer', /.../)

I don't think skipping the tests completely would be a good idea.

In any case, could you please change the order so it's clear what's added.

[CroniD]

Hi all,

@epoberezkin I know that most people think "JSON String representation" and "URI Fragment Identifier Representation" of Json Pointer are interchangeable when talking about what is "a valid Json Pointer", but it really isn't (strictly speaking). "valid" usually refers to something that one can verify based on rules. RFC6901 defines syntax rules. In that way, a Json Pointer in it's "URI Fragment Identifier Representation" isn't syntactical valid. Tbh, this applies to "JSON String representation" as well, because the json pointer syntax doesn't define any json-specific escape mechanism, but since a json schema is something like a json text, the escaping mechanism is inherently defined. That's why the "JSON String representation" is close to what a "valid json pointer" means in a json schema, a valid json pointer in a json text. As @handrews mentioned, it's a bug. Well, strictly speaking it's a bug.
More so, as implementor of an validator for json schema one must define how the "URI Fragment Identifier Representation" is handled in a Json text. Does it need to be percent-encoded or not? RFC6901 doesn't define this, either does draft-06. As far as I know (or believe), RFC3986 does link the need to percent-encode an uri to the term "produced uri". Whatever that means for uri's in a structured text as data, not as part of the text or document type structure (I mean something like json, xml, yaml etc.
Meaning: if one wants to support both and conform to as it's stated in draft-06, one have to support percent-encoded and non-percent-encoded uri fragments. That's tough to implement.

BTW: Is ajv considered to be the current reference implementation of a json schema validator? If not, why execute the tests automatically specifically with ajv? Json syntax checks are fine, but test execution is odd.

Nevertheless, I think option 2 is good for me. :)

About the order of the tests: I re-worked the hole section, but it's kinda strange in the diff view. I'll try to change it, so it's more clear what's changed. :)

Best regards

[handrews]

@CroniD ajv is not the reference implementation (there is no reference implementation), but of the two implementation owners actively involved in the process, @epoberezkin has had more time recently to implement draft-06 and extend the test suite for it. AFAIK ajv is still the only full draft-06 implementation, although I know @Julian is working on it in Python and others have at least started looking at it.

[handrews]

@epoberezkin I think the key wording in that section you linked is here (bolded emphasis mine):

A JSON Pointer can be represented in a URI fragment identifier

Saying that it can be represented in a URI fragment identifier is not the same thing as saying that it can be represented as a URI fragment identifier. The previous section uses the same wording about JSON strings, but since this standard is about working with JSON we're already working with the contents of JSON strings in all cases.

So in a JSON document, a string "#/foo/bar" is a JSON string containing a fragment identifier #/foo/bar which itself contains a JSON Pointer /foo/bar. Therefore in terms of format, "#/foo/bar" is a uri-reference, not a json-pointer on its own.

[epoberezkin]

It is merged via command line. Usually github understands it...