JsonPointer syntax errors should not be valid

[ssilverman]

jsonp-test-suite/src/main/java/org/leadpony/jsonp/testsuite/tests/JsonPointerTest.java

Lines 68 to 70 in 6dca31b

	TILDE_FOLLOWED_BY_ILLEGAL_CHARACTER("/a~2b", true),
	TILDE_FOLLOWED_BY_SLASH("/a~/", true),
	ENDING_WITH_TILDE("/a~", true),

These should be false since their syntax is invalid.

[leadpony]

@ssilverman
Thank you for reporting.
I will check them this weekend.
Are you implementing a new JSON-P compliant parser/generator?

[ssilverman]

Yes. I’ve also got a new validator and was glad to have found yours too a while back.

[leadpony]

@ssilverman
Now I remember why I marked these cases as valid.

The purpose of this test suite is to test implementations of JSR 374 (JSON Processing) API,
which is now superseded by Jakarta JSON Processing API.

The JSON-P specification tells us that JSON Pointer in JSON-P should conform to
RFC 6901. However the RFC does not define how applications handle
such illegal tildes found in JSON Pointer, as below.

This specification does not define how errors are handled. An
application of JSON Pointer SHOULD specify the impact and handling of
each type of error.
For example, some applications might stop pointer processing upon an
error, while others may attempt to recover from missing values by
inserting default ones.

Therefore we should respect of the behavior of the Reference Implementation.
I believe the Reference Implementation ignores such syntax error and treats the tildes as normal character.

You are almost correct. I once implemented my parser to throw exceptions in such cases, but I changed it in order to conform to the Reference Implementation.

[leadpony]

@ssilverman
If you really think JsonExceptions should be thrown in these cases (I agree with you), I recommend that you draw an issue in eclipse-ee4j/jsonp.

[ssilverman]

I hear you on the point about the tests not being to test JSON nor to test the JSON-P API, it's to test the implementation of the JSON-P API. However, what happens if the implementation disagrees with its own documentation? JSON-P specifically states, in JsonProvider.createPointer:

@throws JsonException if {@code jsonPointer} is not a valid JSON Pointer

(See https://github.com/eclipse-ee4j/jsonp/blob/dcef07f088197eb7f44829a3ccf4f6a9b99d29ff/api/src/main/java/jakarta/json/spi/JsonProvider.java#L289)

It also states in RFC 6901 (https://www.rfc-editor.org/rfc/rfc6901.html#section-3)

It is an error condition if a JSON Pointer value does not conform to this syntax 

While I agree that the RFC leaves it up to the application to handle errors, the API itself states that a JsonException is thrown for invalid JSON Pointers. I think we can agree that "/a~2b" is not a valid JSON Pointer, per the Section 3 statement above.

So back to the first question in this post: What happens if the implementation disagrees with its own documentation? In this case the JSON-P API itself stating an exception should be thrown?

[leadpony]

@ssilverman
Personally I agree with you.
The Reference Implementation throws an exception if a given JSON Pointer does not start with a slash. So I feel it is natural that the implementation throws an exception when it detects an illegal tilde in the JSON Pointer. I prefer such strict error checking.

But I cannot say this is a bug with 100% confidence. Apache Johnzon also does not throw an exception in this case. So I judged these test cases are valid and I modified Joy (my own implementation) in order to be compatible with other implementations.

If you are sure that this is a bug in the Reference Implementation, I recommend you consult this problem with JSON-P team.

[leadpony]

@ssilverman
Are you developing JSON-P implementation by wrapping other JSON library, such as Jackson? Could you tell me your purpose for building yet another implementation?

[ssilverman]

I built my own parser (I'm not wrapping anything) and thought I'd code it to a well-known Java spec because I like it when things are compatible; I found javax.json. I like to implement specs, so you could say I did this for sport, but I've been going down a JSON rabbit hole. I've also implemented a JSON Schema validator too (it's called "Snow"; you'll soon find a PR in your inbox for this: https://github.com/leadpony/json-schema-conformance-test).

It all started when I was tasked with doing some RDM standards work (DMX related; lighting control and such) related to JSON schemas (rabbit hole # 1). Because no Java implementation yet implemented Draft 2019-09, I wrote one (rabbit hole # 2). Then, I went down another rabbit hole with how to effectively do proper errors and annotations such that they're not only clear, but organized well and only giving information instead of too much when there's just simple errors in a schema instance (rabbit hole # 3). I needed a break from that, remembered that I like writing manual DFA-based parsers (deterministic finite automaton), so I wrote a DFA (in truth a PDA (pushdown automaton) because it kind of needs a stack) for all the character transition states in JSON (actually, I implemented complete JSON5 support) thinking it would be as fast as possible (basically one table lookup per character read, plus another table lookup for an associated action, plus a little additional logic) (rabbit hole # 4). As I said, I like standards and compatible interfaces, so I completed a javax.json layer for that (rabbit hole # 5). Sadly, lo and behold, GSON is still about 30% faster than mine when building objects 🤦 (mine is still very fast when just using the parser for events and not building an object tree, thankfully). I like completeness and I couldn't just leave it (GSON is faster when building the object tree, so I'm sure Jackson is too), so I completed my javax.json layer by validating against all these tests, and here we are.

So, in summary: an attempt at a faster JSON parser, and "completeness".

[leadpony]

@ssilverman
If you really love to implement faster software, try this benchmark.
https://github.com/leadpony/jsonp-benchmark
Your implementation is faster than the Reference Implementation?

[ssilverman]

<sigh> My implementation looks like it's 4x ops/s slower than the reference implementation. I must be doing something that's not very JIT friendly.

[leadpony]

@ssilverman
I posted an issue in eclipse-ee4j/jsonp#266 for this problem.