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

JsonNode.GetPath() doesn't consider character escaping #83547

Open
gregsdennis opened this issue Mar 16, 2023 · 4 comments
Open

JsonNode.GetPath() doesn't consider character escaping #83547

gregsdennis opened this issue Mar 16, 2023 · 4 comments

Comments

@gregsdennis
Copy link
Contributor

gregsdennis commented Mar 16, 2023

Description

The JsonNode.GetPath() generally does well for C#-valid property names, but any string is valid as a key in JSON, and when a key has characters that need escaping, nothing happens and you get an invalid (unparsable) JSON Path as a result.

Reproduction Steps

JsonNode.Parse("""{"$defs":{"foo['bar":"baz"}}""")["$defs"]["foo['bar"].GetPath();

Expected behavior

Returns $.$defs['foo[\'bar'].

Returns $['$defs']['foo[\'bar'].

  • The single quote needs to be escaped.
  • $defs is not valid as a shorthand property name, so it needs to be in bracketed syntax.

Actual behavior

Returns $.$defs['foo['bar'].

Regression?

Unknown

Known Workarounds

None.

Configuration

Seems to happen everywhere. dotnetfiddle, Linux.

Will update with more details in a comment.

Other information

Originally reported json-everything/json-everything#406.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Mar 16, 2023
@ghost
Copy link

ghost commented Mar 16, 2023

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

The JsonNode.GetPath() generally does well for C#-valid property names, but any string is valid as a key in JSON, and when a key has characters that need escaping, nothing happens and you get an invalid (unparsable) JSON Path as a result.

Reproduction Steps

JsonNode.Parse("""{"$defs":{"foo['bar":"baz"}}""")["$defs"]["foo['bar"].GetPath();

Expected behavior

Returns $.$defs['foo[\'bar'].

Actual behavior

Returns $.$defs['foo['bar'].

Regression?

Unknown

Known Workarounds

None.

Configuration

Seems to happen everywhere. dotnetfiddle, Linux.

Will update with more details in a comment.

Other information

Originally reported json-everything/json-everything#406.

Author: gregsdennis
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@amis92
Copy link

amis92 commented Mar 16, 2023

The specific source code in question is the following:

if (propertyName.AsSpan().ContainsSpecialCharacters())
{
path.Add($"['{propertyName}']");
}
else
{
path.Add($".{propertyName}");
}

While it actively looks for special characters to use indexer syntax (['xyz']) instead of shorthand (.foo), it does not attempt to escape the apostrophe '.

@gregsdennis
Copy link
Contributor Author

I should also mention that perhaps JSON Path is the wrong syntax to use for identifying a single location within a JSON tree. JSON Pointer is more well-suited for that.

@krwq krwq added the bug label Mar 23, 2023
@krwq krwq added this to the Future milestone Mar 23, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Mar 23, 2023
@gregsdennis
Copy link
Contributor Author

gregsdennis commented Mar 27, 2023

☝️ I'm adding .GetPathFromRoot() and .GetPointerFromRoot() to my Json.More.Net library to support this until it's in the runtime.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 13, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants