Properly format normalized path names #113
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The RFC defines how to [format normalized path names]. Notably, it
defines the escape character as `ESC normal-escapable`, but doesn't
define `ESC` in that section. It is, however, defined for [name selector
syntax] as `%x5C; \ backslash`. I'm therefore reasonably sure that the
current behavior of serde_json_path when formatting normalized paths is
incorrect, as it results in no escaping of `'` at all.
For example, using [the sandbox] to select `$.*` from
`{"O'Reilly": true}` returns
```json
[
{
"loc": "$['O'Reilly']",
"node": true
}
]
```
I worked this out while building the location feature into [Go jsonpath]
(many thanks for the inspiration in your crate!). This [playground link]
shows what I believe to be the correct escaping (doubled for inclusion
in JSON):
``` json
[
{
"node": true,
"path": "$['O\\'Reilly']"
}
]
```
So expand `Display for PathElement` to properly format characters
strictly according to the spec. Iterate over each character in a name
and escape exactly the characters it defines to be escaped.
[format normalized path names]: https://www.rfc-editor.org/rfc/rfc9535#section-2.7
[name selector syntax]: https://www.rfc-editor.org/rfc/rfc9535#name-syntax-3
[the sandbox]: https://serdejsonpath.live
[Go jsonpath]: https://github.com/theory/jsonpath
[playground link]: https://theory.github.io/jsonpath/?p=%2524.*&j=%257B%2522O%27Reilly%2522%253A%2520true%257D&o=1
theory
force-pushed
the
path-element-format
branch
from
d603cee to
c56a4e5
Compare
hiltontj
added
help wanted
|
Great find @theory and thanks for opening the issue! Another important thing that this made me realize is that I don't think the compliance test suite is providing a set of tests for validating how a library forms normalized paths. I will see if I can find time to sort this out, but for now added some labels to 🎣 if anyone wants to try and fix it. |
|
This PR should fix it. |
|
Oh 🤦 I thought this was an issue, and not a PR. I will take a look... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The RFC defines how to format normalized path names. Notably, it defines the escape character as
ESC normal-escapable, but doesn't defineESCin that section. It is, however, defined for name selector syntax as%x5C; \ backslash. I'm therefore reasonably sure that the current behavior of serde_json_path when formatting normalized paths is incorrect, as it results in no escaping of'at all.For example, using the sandbox to select
$.*from{"O'Reilly": true}returns[ { "loc": "$['O'Reilly']", "node": true } ]I worked this out while building the location feature into Go jsonpath (many thanks for the inspiration in your crate!). This playground link shows what I believe to be the correct escaping (doubled for inclusion in JSON):
[ { "node": true, "path": "$['O\\'Reilly']" } ]So expand
Display for PathElementto properly format characters strictly according to the spec. Iterate over each character in a name and escape exactly the characters it defines to be escaped.