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

Alignment with spec in its current state (a report) #127

Open
gregsdennis opened this issue Dec 24, 2022 · 7 comments
Open

Alignment with spec in its current state (a report) #127

gregsdennis opened this issue Dec 24, 2022 · 7 comments

Comments

@gregsdennis
Copy link
Collaborator

gregsdennis commented Dec 24, 2022

I've recently updated my implementation (JsonPath.Net) to conform with the spec. I haven't published it yet, but I wanted to share a finding.

These tests are not in alignment with the spec. I realize that spec alignment isn't a goal of this project, but I wanted to share my results.

// dashes are not allowed in shorthand property names
// discussion: https://github.com/ietf-wg-jsonpath/draft-ietf-jsonpath-base/issues/16
$.key-dash
$[?(@.key-dash == 'value')]

// shorthand property names must start with a letter
$.2",
$[?(@.2 == 'second')]
$[?(@.2 == 'third')]
$.-1
$.$
$.'key'
$."key"
$.."key"
$..'key'
$..'key'
$.'some.key'

// leading zeroes are not allowed for numeric literals
// reason: disallowed for JSON, so copied here
$[?(@.key==010)]
$[010:024:010]

// JSON literals are not expression results
$[?(@.key>0 && false)]
$[?(@.key>0 && true)]
$[?(@.key>0 || false)]
$[?(@.key>0 || true)]
$[?((@.key<44)==false)]
$[?(true)]
$[?(false)]
$[?(null)]

// leading and trailing whitespace is disallowed
// https://github.com/jsonpath-standard/jsonpath-compliance-test-suite/issues/13
"$. a "

// regex operator transitioned to functions
// discussion: https://github.com/ietf-wg-jsonpath/draft-ietf-jsonpath-base/issues/70
// discussion: https://github.com/ietf-wg-jsonpath/iregexp/issues/15
$[?(@.name=~/hello.*/)]
$[?(@.name=~/@.pattern/)]

// functions are only valid in expressions and are not extensions on paths
// discussion (length): https://github.com/ietf-wg-jsonpath/draft-ietf-jsonpath-base/issues/154
// discussion (functions): https://github.com/ietf-wg-jsonpath/draft-ietf-jsonpath-base/issues/203 
$.data.sum()
$[?(@.length() == 4)]

// relative paths were excluded
// discussion: https://github.com/ietf-wg-jsonpath/draft-ietf-jsonpath-base/issues/59
@.a

// 'in' operator was excluded
// discussion: https://github.com/ietf-wg-jsonpath/draft-ietf-jsonpath-base/issues/64
$[?(@.d in [2, 3])]
$[?(2 in @.d)]

// only JSON primitives are supported in expressions
$[?(@.d==['v1','v2'])]

// only singular paths are allowed in comparisons
$[?(@[0:1]==[1])]  // also array literals are disallowed
$[?(@.*==[1,2])]   // also array literals are disallowed
$[?(@[0:1]==1)]
$[?(@[*]==2)]
$[?(@.*==2)]
$[?(@[*]>=4)]
$.x[?(@[*]>=$.y[*])]

// other invalid syntaxes
$...key
$.['key']
$.[\"key\"]
$..
$.key..
.key
key
      // empty string
$[?(@.key===42)]

// big numbers not supported
$[2:-113667776004:-1]
$[113667776004:2:-1]
@gregsdennis
Copy link
Collaborator Author

Looks like some new tests were added, including some spec-invalid cases. I've updated the set above to include the new invalid ones.

@danielaparker
Copy link
Collaborator

I've recently updated my implementation (JsonPath.Net) to conform with the spec. I haven't published it yet, but I wanted to share a finding.

These tests are not in alignment with the spec.

That reflects the fact that the IETF spec is incompatible in important ways with all existing implementations to 2022, which in turn are incompatible in important ways with themselves. And important implementations such as Jayway can't change very much, because they have thousands of artifacts depending on them, are incorporated into enterprise APIs, and have large existing user communities.

It does raise the suggestion that the IETF spec is not so much about standardizing existing practice, as it is about making something new. Perhaps it should be called something new, much as what started off as an IETF effort to standardize MessagePack became CBOR. The IETF spec is certainly as different from Goessner JSONPath and Jayway JSONPath as IETF CBOR is from MessagePack.

Daniel

@gregsdennis
Copy link
Collaborator Author

I think Jayway's approach, if they want to implement the spec, would need to be to have a "spec-compliant" mode. Maybe even make that the default after a major version bump.

I'll update the above with reasons why some of these aren't included.

@danielaparker
Copy link
Collaborator

danielaparker commented Mar 23, 2023

I think Jayway's approach, if they want to implement the spec, would need to be to have a "spec-compliant" mode. Maybe even make that the default after a major version bump.

I think Jayway is in maintenance mode, I don't think anybody there would undertake the effort. And besides, the draft's approach to expression evaluation is fundamentally different than Jayway's, or for that matter, any existing implementation up to 2022. I think most conforming implementations are going to be new implementations, I would have said all, except I understand json-everything is doing that! But for the most part, the old implementations that have users and artifacts dependent on them are going to stay the same.

@gregsdennis
Copy link
Collaborator Author

I agree with that sentiment, and @hiltontj's recent efforts are proof.


Specifically, I think these wouldn't be too hard to support:

  • JSON object and array literals in expressions
  • leading and trailing whitespace tolerance
  • in operator
  • math operators
  • relative paths (I fear we alienated the contributor who proposed this)

@danielaparker
Copy link
Collaborator

danielaparker commented Mar 23, 2023

  • relative paths (I fear we alienated the contributor who proposed this)

Indeed, but at least we learned something about what "rough consensus" really meant.

@gregsdennis
Copy link
Collaborator Author

☝️ I've now added these things behind options in my lib. Really easy to implement.

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

2 participants