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

Add tests for invalid surrogate pairs #87

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

Marcono1234
Copy link
Contributor

@Marcono1234 Marcono1234 commented Aug 1, 2024

Adds tests for invalid surrogate pairs, and one test for directly using a character > U+FFFF (𝄞) in the selector, instead of escaping it.


Note: Another interesting test would be

  • escaped + non-escaped surrogate character, for example: "selector": "$[\"\\uD800\uDC00\"]"
    (for this a JSONPath implementation might erroneously first convert \\uD800 to U+D800 before performing validation, and then consider this a valid surrogate pair)
  • malformed non-escaped surrogate pairs, for example "selector": "$[\"\uD800\uD800\"]"

Note in both cases the single \ instead of \\.

Both are not permitted by JSONPath because it says the query must be valid UTF-8, which is not the case here.

However, it depends on the programming language whether such malformed surrogate pairs are possible. For example for Java whose strings are UTF-16 based such selectors would be valid strings, but the JSONPath implementation would have to reject them.

The problem is that for other programming languages such strings are not valid (e.g. Rust uses UTF-8 for strings), so including for example "selector": "$[\"\\uD800\uDC00\"]" might prevent a Rust library from loading cts.json at all. Therefore such tests can probably not be added.

Copy link
Collaborator

@gregsdennis gregsdennis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These all fail for me when they're supposed to (but again, they fail incorrectly 😆).

I'm happy with this. Keep up the good work.

@gregsdennis gregsdennis requested a review from f3ath August 1, 2024 22:50
Copy link
Collaborator

@f3ath f3ath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job indeed. My implementation was not checking for child selector correctness.

@f3ath f3ath merged commit b1fdeaf into jsonpath-standard:main Aug 2, 2024
2 checks passed
@Marcono1234 Marcono1234 deleted the surrogate-pairs branch August 2, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants