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

JSON pointer query, trailing slashes and empty keys #410

Closed
mfriedemann opened this issue Mar 15, 2018 · 4 comments · Fixed by #411
Closed

JSON pointer query, trailing slashes and empty keys #410

mfriedemann opened this issue Mar 15, 2018 · 4 comments · Fixed by #411

Comments

@mfriedemann
Copy link

Hey,

I found an inconsistency, or potentially a bug, with the handling of JSON pointers in JSONObject.query().

Specifically, the handling of trailing slashes differs between the root element and elements further down.

From reading the RFC and a discussion about it, I got the impression that a trailing slash is supposed to refer to an empty key, but this only works with the root element.
For elements further down, a trailing slash will not select the empty key below the element, but the element itself instead.

example code

public void testPointerQuery() {
	JSONObject none = new JSONObject();
	JSONObject emptyKeyRoot = new JSONObject("{'':'empty key at root'}");
	JSONObject emptyKeyBelow = new JSONObject("{'foo':{'':'empty key below foo'}}");
	
	System.out.println("json=" + none + " ptr='' query -> " + none.query(""));
	System.out.println("json=" + none + " ptr='/' query -> " + none.query("/"));
	
	System.out.println("json=" + emptyKeyRoot + " ptr='' query -> " + emptyKeyRoot.query(""));
	System.out.println("json=" + emptyKeyRoot + " ptr='/' query -> " + emptyKeyRoot.query("/"));
	
	System.out.println("json=" + emptyKeyBelow + " ptr='' query -> " + emptyKeyBelow.query(""));
	System.out.println("json=" + emptyKeyBelow + " ptr='/' query -> " + emptyKeyBelow.query("/"));
	System.out.println("json=" + emptyKeyBelow + " ptr='/foo' query -> " + emptyKeyBelow.query("/foo"));
	System.out.println("json=" + emptyKeyBelow + " ptr='/foo/' query -> " + emptyKeyBelow.query("/foo/"));
}

output for example code above

json={} ptr='' query -> {}
json={} ptr='/' query -> null
json={"":"empty key at root"} ptr='' query -> {"":"empty key at root"}
json={"":"empty key at root"} ptr='/' query -> empty key at root
json={"foo":{"":"empty key below foo"}} ptr='' query -> {"foo":{"":"empty key below foo"}}
json={"foo":{"":"empty key below foo"}} ptr='/' query -> null
json={"foo":{"":"empty key below foo"}} ptr='/foo' query -> {"":"empty key below foo"}
json={"foo":{"":"empty key below foo"}} ptr='/foo/' query -> {"":"empty key below foo"}

expected output (last line)

json={"foo":{"":"empty key below foo"}} ptr='/foo/' query -> empty key below foo

This happens with both the string and URI fragment representations, using the 20180130 maven artifact.

Thanks,
M.

@stleary
Copy link
Owner

stleary commented Mar 16, 2018

Investigating...

@stleary
Copy link
Owner

stleary commented Mar 17, 2018

Please reference the RFC you read?
Are you asserting that JSONPointer strings ending with '/' are valid?
Given your test case, what, in your opinion, would correct output look like?
Nice work, by the way.

@mfriedemann
Copy link
Author

mfriedemann commented Mar 19, 2018

I am referring to RFC 6901, and a discussion I found about implementation details.
Specifically, RFC 6901, Section 3 states:

A JSON Pointer is a Unicode string (see [RFC4627], Section 3)
containing a sequence of zero or more reference tokens, each prefixed
by a '/' (%x2F) character.

Additionally, the list of examples contains the mapping of pointer (left) to evaluation result (right)

"/" 0

for an example document

{
"" : 0
}

Unfortunately, the RFC does not specify this in greater depth, but given the wording cited above ("prefixed by'/'), I would assert that trailing slashes should be valid and that they should reference empty keys (or fail, if none exists at that level).

I won't be able to check the proposed fix today, will try and look at it tomorrow.

Thanks,
M.

@mfriedemann
Copy link
Author

As per the comment to #411, I can confirm that it fixes #410.

Thanks,
M.

@stleary stleary closed this as completed Mar 22, 2018
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

Successfully merging a pull request may close this issue.

2 participants