-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
MDEV-39999 Fix JSON path quoting for keys with special characters #5240
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
Open
akshatnehra
wants to merge
1
commit into
MariaDB:main
Choose a base branch
from
akshatnehra:MDEV-39999
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+132
−3
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| # | ||
| # MDEV-39999: JSON_SEARCH must quote keys containing special characters | ||
| # | ||
| SELECT JSON_SEARCH('{"a.b":"x"}', 'one', 'x') AS path; | ||
| path | ||
| "$.\"a.b\"" | ||
| SELECT JSON_SEARCH('{"a[0]":"x"}', 'one', 'x') AS path; | ||
| path | ||
| "$.\"a[0]\"" | ||
| SELECT JSON_SEARCH('{"normal":"x"}', 'one', 'x') AS path; | ||
| path | ||
| "$.normal" | ||
| SELECT JSON_SEARCH('{"hello world":"x"}', 'one', 'x') AS path; | ||
| path | ||
| "$.\"hello world\"" | ||
| SELECT JSON_SEARCH('{"a*b":"x"}', 'one', 'x') AS path; | ||
| path | ||
| "$.\"a*b\"" | ||
| SELECT JSON_SEARCH('{"a\\"b":"x"}', 'one', 'x') AS path; | ||
| path | ||
| "$.\"a\\\"b\"" | ||
| SELECT JSON_SEARCH('{"a\\\\b":"x"}', 'one', 'x') AS path; | ||
| path | ||
| "$.\"a\\\\b\"" | ||
| SELECT JSON_SEARCH('{"1abc":"x"}', 'one', 'x') AS path; | ||
| path | ||
| "$.1abc" | ||
| SELECT JSON_SEARCH('{"":"x"}', 'one', 'x') AS path; | ||
| path | ||
| "$.\"\"" | ||
| SELECT JSON_VALUE('{"":"x"}', JSON_UNQUOTE(JSON_SEARCH('{"":"x"}', 'one', 'x'))) AS val; | ||
| val | ||
| x | ||
| SELECT JSON_SEARCH('{"a.b":"x", "c":"x", "d[0]":"x"}', 'all', 'x') AS paths; | ||
| paths | ||
| ["$.\"a.b\"", "$.c", "$.\"d[0]\""] | ||
| SELECT JSON_VALUE('{"a.b":"found"}', JSON_UNQUOTE(JSON_SEARCH('{"a.b":"found"}', 'one', 'found'))) AS val; | ||
| val | ||
| found | ||
| SELECT JSON_VALUE('{"a\\\\b":"found"}', JSON_UNQUOTE(JSON_SEARCH('{"a\\\\b":"found"}', 'one', 'found'))) AS val; | ||
| val | ||
| found | ||
| SELECT JSON_SEARCH('{"a.b":{"c[d]":"x"}}', 'one', 'x') AS path; | ||
| path | ||
| "$.\"a.b\".\"c[d]\"" |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| # | ||
| # MDEV-39999: JSON path quoting for keys with special characters | ||
| # | ||
|
|
||
| --echo # | ||
| --echo # MDEV-39999: JSON_SEARCH must quote keys containing special characters | ||
| --echo # | ||
|
|
||
| # Dot in key | ||
| SELECT JSON_SEARCH('{"a.b":"x"}', 'one', 'x') AS path; | ||
|
|
||
| # Bracket in key | ||
| SELECT JSON_SEARCH('{"a[0]":"x"}', 'one', 'x') AS path; | ||
|
|
||
| # Normal key (no quoting needed) | ||
| SELECT JSON_SEARCH('{"normal":"x"}', 'one', 'x') AS path; | ||
|
|
||
| # Space in key | ||
| SELECT JSON_SEARCH('{"hello world":"x"}', 'one', 'x') AS path; | ||
|
|
||
| # Asterisk in key | ||
| SELECT JSON_SEARCH('{"a*b":"x"}', 'one', 'x') AS path; | ||
|
|
||
| # Embedded double-quote in key | ||
| SELECT JSON_SEARCH('{"a\\"b":"x"}', 'one', 'x') AS path; | ||
|
|
||
| # Backslash in key | ||
| SELECT JSON_SEARCH('{"a\\\\b":"x"}', 'one', 'x') AS path; | ||
|
|
||
| # Key starting with digit | ||
| SELECT JSON_SEARCH('{"1abc":"x"}', 'one', 'x') AS path; | ||
|
|
||
| # Empty key | ||
| SELECT JSON_SEARCH('{"":"x"}', 'one', 'x') AS path; | ||
| SELECT JSON_VALUE('{"":"x"}', JSON_UNQUOTE(JSON_SEARCH('{"":"x"}', 'one', 'x'))) AS val; | ||
|
|
||
| # JSON_SEARCH with 'all' mode - multiple matches | ||
| SELECT JSON_SEARCH('{"a.b":"x", "c":"x", "d[0]":"x"}', 'all', 'x') AS paths; | ||
|
|
||
| # Roundtrip: path from JSON_SEARCH works with JSON_VALUE | ||
| SELECT JSON_VALUE('{"a.b":"found"}', JSON_UNQUOTE(JSON_SEARCH('{"a.b":"found"}', 'one', 'found'))) AS val; | ||
|
|
||
| # Roundtrip with backslash key | ||
| SELECT JSON_VALUE('{"a\\\\b":"found"}', JSON_UNQUOTE(JSON_SEARCH('{"a\\\\b":"found"}', 'one', 'found'))) AS val; | ||
|
|
||
| # Nested object with special keys | ||
| SELECT JSON_SEARCH('{"a.b":{"c[d]":"x"}}', 'one', 'x') AS path; | ||
|
|
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4378,9 +4378,45 @@ static int append_json_path(String *str, const json_path_t *p) | |
| { | ||
| if (c->type & JSON_PATH_KEY) | ||
| { | ||
| if (str->append(".", 1) || | ||
| append_simple(str, c->key, c->key_end-c->key)) | ||
| const char *k= (const char *) c->key; | ||
| size_t k_len= c->key_end - c->key; | ||
| bool needs_quote= (k_len == 0); | ||
| for (size_t i= 0; i < k_len; i++) | ||
| { | ||
| if (k[i] == '.' || k[i] == '[' || k[i] == ']' || | ||
| k[i] == '"' || k[i] == ' ' || k[i] == '*' || k[i] == '\\') | ||
| { | ||
| needs_quote= true; | ||
| break; | ||
| } | ||
| } | ||
| if (str->append(".", 1)) | ||
| return TRUE; | ||
| if (needs_quote) | ||
| { | ||
| if (str->append('\\') || str->append('"')) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please consider using st_append_escaped() instead. See append_json_value_from_field(). And produce a "bracketed-selection" (see above). |
||
| return TRUE; | ||
| for (size_t i= 0; i < k_len; i++) | ||
| { | ||
| if (k[i] == '"' || k[i] == '\\') | ||
| { | ||
| if (str->append('\\') || str->append(k[i])) | ||
| return TRUE; | ||
| } | ||
| else | ||
| { | ||
| if (str->append(k[i])) | ||
| return TRUE; | ||
| } | ||
| } | ||
| if (str->append('\\') || str->append('"')) | ||
| return TRUE; | ||
| } | ||
| else | ||
| { | ||
| if (append_simple(str, c->key, k_len)) | ||
| return TRUE; | ||
| } | ||
| } | ||
| else /*JSON_PATH_ARRAY*/ | ||
| { | ||
|
|
||
Oops, something went wrong.
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.
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's more complex than this I'm afraid. https://www.rfc-editor.org/info/rfc9535/#segments-details defines what is a valid "shorthand" notation as follows:
This is implemented by json_escape() in the MariaDB code I believe.
So here's what I would suggest: either always produce bracketed-selection (see above) or call st_append_escaped (or json_escape directly), see if the result is different from the source string and if it is, go with the bracketed expression that you have, otherwise do the unescaped source string.
This will have the disadvantage that you will always produce a bracketed escaped string. But I believe this is not such a big deal performance wise.
You could of course consider implementing a non-outputting version of json_escape() and call that json_needs_escaping() for example. But I believe it needs to be done along the same lines at least.