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

Replace JSON11-produced hex escape codes with unicode escape codes #879

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wjordan
Copy link

@wjordan wjordan commented Sep 26, 2024

Description

Since #784, JSON11.stringify produces JSON5 documents with hex escape codes (\x1b), which aren't standard JSON and cause JSON.parse to error. When using JSON11 code path, this PR applies a fix to replace all \xXX escape codes with the JSON-compatible equivalent Unicode escape codes (\u00XX).

This PR adds a unit test verifying the fix by ensuring a round-trip s.deserialize(s.serialize(obj)) with an object containing long numerals and ANSI escape sequences completes successfully without error.

Issues Resolved

Fixes opensearch-project/OpenSearch-Dashboards#7367.

Check List

  • New functionality includes testing.
    • All tests pass
  • Linter check was successfull - yarn run lint doesn't show any errors
  • Commits are signed per the DCO using --signoff
  • Changelog was updated.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

JSON11.stringify produces JSON5 documents with hex escape codes (`\x1b`),
which aren't standard JSON and cause `JSON.parse` to error.
When using JSON11, replace all `\xXX` escape codes with the JSON-compatible
equivalent Unicode escape codes (`\u00XX`).

Fixes opensearch-project/OpenSearch-Dashboards#7367.

Signed-off-by: Will Jordan <[email protected]>
@nhtruong
Copy link
Collaborator

Thanks @wjordan
Do you think this is a bug in JSON11 library?
Don't forget to update the CHANGELOG.md file.

Signed-off-by: Will Jordan <[email protected]>
@wjordan
Copy link
Author

wjordan commented Sep 27, 2024

Do you think this is a bug in JSON11 library?

No, the json11 library writes JSON5 documents as expected, the bug is in the serialization code in this repo. See the added unit test which fails without this PR.

A feature request for json11 to take an option to stringify that would return JSON-compatible strings (instead of JSON5) would be another way to solve this issue instead of doing a regex replace here, but I wouldn't call the lack of such an option a bug.

@AMoo-Miki
Copy link
Contributor

AMoo-Miki commented Sep 27, 2024

Ideally, this should be handled on JSON11. I will find how that is happening.

Update: JSON11 v2.0.0 was published with a change in behavior.

@wjordan can u try bumping JSON11 to 2, without the changes in this PR to see if it solves it for you?

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.

[BUG] - OpenSearch Dashboard V2.15.0 - JSON.parse: bad escaped character
3 participants