JSONPointer: Encoding quotes not required?

[jimblackler]

JSONPointer encodes quotes as \"

https://github.com/stleary/JSON-java/blob/7abd89b4bcc321152c178af5504da7ea4ac66693/src/main/java/org/json/JSONPointer.java#L271

However I can't find a requirement for that in https://tools.ietf.org/html/rfc6901#section-3 or elsewhere.

It causes a conflict in an application that expects the canonical form of the URL not to be escaped in this way. For use in fragment identifiers RFC 3986 allows %22 to encode ".

[stleary]

Please see https://tools.ietf.org/html/rfc6901#section-5. Does this address your concern?

[jimblackler]

Is that saying that those chracters should be escaped when represented as a JSON string literal? (That would be something handled by clients not by JsonPointer()). Does the requirement apply otherwise?

If the code at https://github.com/stleary/JSON-java/blob/7abd89b4bcc321152c178af5504da7ea4ac66693/src/main/java/org/json/JSONPointer.java#L271 is executed and I JSON encode the resulting string I'll get two sets of encoding on the string characters.

[jimblackler]

The motivation for the query is an apparent clash between JSON Schema offical test suite requirements and org.json.JSONPointer implementation.

https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/master/tests/draft7/ref.json#L363

The test "refs with quote" creates a reference to the value at definitions/foo"bar; in JSON Schema references are required to by JSON Pointers, first escaped to the JSON Pointer requirements and then escaped to URI requirements to represent them as fragments (after the #). Accordingly, the test requires the form foo%22bar. The fact then that the reference is stored as a JSON String will cover the requirements in https://tools.ietf.org/html/rfc6901#section-5.

https://tools.ietf.org/html/draft-handrews-json-schema-01#section-8.3 https://tools.ietf.org/html/draft-handrews-json-schema-01#section-8.3.2

If org.json.JSONPointer is used to contruct this reference it first will escape the " to \" which will then result in a reference foo\"bar, then URI encoded to foo%5C%22bar

[stleary]

Are you requesting a change to the code?

[jimblackler]

Yes; if the exising maintainers agree with me that the current behavior is incorrect.

[johnjaylward]

I haven't had any time to look into this or read through the spec. Work has been been very busy for me lately. At the moment I don't think I have enough understanding of the problem to comment.

[stleary]

@jimblackler Thanks for being patient. I don't see a compelling case for a bug that must be fixed. At best, it may be an unusual corner case. However, if the current behavior is causing problems with your application, feel free to submit a PR that converts JSONPointer string escaped double quotes with the corresponding URI-encoded value. Otherwise, we will probably leave the code as-is.

[jimblackler]

Thank you for the reply Sean. I don't agree that corner case is a good description of the probem since anything that uses a quotation mark in tokens will encounter this. I believe that the library is applying transformations that are not required by rfc6901 and this sadly is actively preventing the library from interoperating with programs that expect the standard (as per the example in comment 4).

I accept I might be the first to report this, but it is also possible I am the first to both test it with rigour while also taking the time to log an issue with the aim of improving the library.

The PR would be to remove the final two replace calls in escape().

I volunteer to do some work to make https://github.com/stleary/JSON-java/blob/master/src/main/java/org/json/JSONPointer.java more strictly obey https://tools.ietf.org/html/rfc6901 which I think is important for adoption of JSONPointer since the org.json namespace implies it is the reference implentation. This work would include more tests cases and a resassment of the existing cases; for example, I believe that org.json.junit.JSONPointerTest.quotationEscaping() enforces incorrect behavior.

[stleary]

OK to proceed, but please review the FAQ. Changes that break backward compatibility are rarely accepted.

[jimblackler]

Well, I believe it would fall under https://github.com/stleary/JSON-java/wiki/FAQ#what-kind-of-changes-will-be-accepted-for-this-project

"Changes to existing behavior that has been in place for a substantial period of time will only be accepted in the case of bugs where the existing behavior is clearly worse than the fix."

This behavior has beeen present for 4.5 years. (Added in https://github.com/stleary/JSON-java/commit/d833c2d8de86be697bbc1e9a416736cefafcf320)

[fossterer]

@jimblackler Have opinions on https://github.com/stleary/JSON-java/pull/588?

I didn't get " Linked pull requests" and " Linked issues" to reflect the availability of PR

[stleary]

Closed due to lack of activity.