-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Fix JsonNode.GetPath() to properly escape characters in property names #122841
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
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.
Pull request overview
This PR fixes JsonNode.GetPath() to produce valid JSON Path syntax when property names contain special characters requiring escaping. The fix addresses two specific issues: (1) property names starting with $ now use bracket notation instead of invalid dot notation, and (2) single quotes and backslashes in property names are now properly escaped within bracket notation.
Key Changes:
- Added
$to the set of special characters that trigger bracket notation in JSON Path - Implemented
AppendEscapedPropertyNamehelper methods to escape'as\'and\as\\within bracket notation - Updated all JSON Path generation sites (
JsonObject.GetPath,WriteStack.AppendPropertyName,ReadStack.AppendPropertyName) to use the new escaping
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| JsonReaderHelper.cs | Added $ to special characters set; implemented two overloads of AppendEscapedPropertyName for escaping quotes and backslashes |
| JsonObject.cs | Updated GetPath to call AppendEscapedPropertyName when using bracket notation |
| WriteStack.cs | Updated AppendPropertyName to call AppendEscapedPropertyName when using bracket notation |
| ReadStack.cs | Updated AppendPropertyName to call AppendEscapedPropertyName when using bracket notation |
| ParentPathRootTests.cs | Added comprehensive test cases covering $-prefixed names, quotes, backslashes, and nested scenarios |
| PolymorphicTests.CustomTypeHierarchies.cs | Updated test expectations from dot notation (e.g., $.$type) to bracket notation (e.g., $['$type']) |
| ReferenceHandlerTests.Deserialize.cs | Updated test expectations from dot notation to bracket notation for $-prefixed metadata properties |
| ConstructorTests.Exceptions.cs | Updated test expectations from dot notation to bracket notation for $-prefixed properties |
| return; | ||
| } | ||
|
|
||
| // Slow path: escape single quotes and backslashes. |
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.
@copilot, instead of using ContainsAny, this can use IndexOfAny. If the result is < 0, then it can just append the propertyName and return. If it's > 0, it can append propertyName.Slice(0, i). And then it can walk character by character starting from i as is done below. That way, if the character that needs escaping is later rather than earlier, we don't have to spend time individually appending each individual character up until that point.
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.
Done in cd3590f. Used IndexOfAny to find the first character needing escaping, append the prefix slice directly (if any), then walk character by character only from that position.
| /// Appends a property name escaped for use in JSON Path single-quoted bracket notation. | ||
| /// Escapes single quotes as \' and backslashes as \\. | ||
| /// </summary> | ||
| public static void AppendEscapedPropertyName(this StringBuilder builder, string propertyName) |
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.
@copilot, same comment as above. Under #if NET, StringBuilder supports appending a span, so it can append a slice. Otherwise, StringBuilder supports appending a pointer/length, so it can pin the slice and append it that way.
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.
Done in cd3590f. Applied the same optimization. Under #if NET, StringBuilder appends the span slice directly. Otherwise, it pins the string and uses the pointer/length overload.
…acter iteration for prefix Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Description
JsonNode.GetPath()produces invalid JSON Path syntax when property names contain characters requiring escaping. Two issues:$(e.g.,$defs,$ref) used dot notation ($.$defs) instead of bracket notation ($['$defs'])Before:
After:
// Returns: $['$defs']['foo[\'bar'] (valid JSON Path)Changes:
$to special characters triggering bracket notation inJsonReaderHelper.csAppendEscapedPropertyNamehelper methods that escape'→\'and\→\\, optimized usingIndexOfAnyto append prefix slices directly instead of per-character iterationJsonObject.GetPath,WriteStack.AppendPropertyName, andReadStack.AppendPropertyNameto use escapingCustomer Impact
Users receiving invalid JSON Path strings from
GetPath()that cannot be parsed or used with JSON Path tools.Regression
No. This behavior has existed since the feature was introduced.
Testing
$-prefixed names, single quotes, backslashes, and combinationsRisk
Low. The change makes output spec-compliant. Existing consumers expecting the old (invalid) format may need adjustment.
Package authoring no longer needed in .NET 9
IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.
Original prompt
JsonNode.GetPath()doesn't consider character escaping #83547💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.