Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[JsonPath] Make the component RFC compliant#61132
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:7.3
Are you sure you want to change the base?
Conversation
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
*/ | ||
private function isJsonArrayStructure(mixed $value): bool | ||
{ | ||
return \is_array($value) && array_is_list($value) && (1 !== \count($value) || !isset($value[0])); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
why wouldn't['foo']
be a JSON array structure ?
alexandre-dauboisJul 17, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This method handles an edge case: when a json like[{"0": 5}]
is decoded to PHP, accessing@[0]
on the inner object should return nothing, because numeric index selectors only work on arrays, not objects per the RFC. So just callingarray_is_list()
would incorrectly allow it. The||
part of the condition specifically excludes single-element arrays with index 0 set to handle this distinction.
Does this answer your question?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
but this ignores 2 cases:
- AFAICT, it breaks the case of accessing single-element arrays
- you could totally have JSON objects with multiple numeric keys (
{"0": 5, "1": 12}
), which would be considered array structure in your method.
alexandre-dauboisJul 17, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It seems pretty complex to fix... Just in case,@mtarld, is there something in JsonStreamer that could also help here? We would need to "easily" differentiate an array from an object with numeric indices. MaybeRead/Lexer.php
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
For the case of strings usingjson_decode
, we might decode objects intostdClass
to distinguish objects from arrays. But this suffers from other limitations as this won't support object keys that are not valid names of PHP properties.
alexandre-dauboisJul 17, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Tried this approach in54281cb and succeeded on making the CI green. It makes things a bit more complicated but we can easily differentiate objects and arrays now. Also updated the compliance test suite because the specific cases we were dealing with suffered from the same problem when being decoded fromcts.json
.
this suffers from other limitations as this won't support object keys that are not valid names of PHP properties
Do you have an example? It seems to work pretty well, and as far as I know, stdClass are actually "allowed" to use wrong property names likehttps://3v4l.org/ede7M#vnull.
d1364da
to99ff096
CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
99ff096
to9c7a899
Compare
Uh oh!
There was an error while loading.Please reload this page.
This PR makes the JsonPath component RFC compliant by removing all skipped tests of the compliance test suite.