Skip to content

Revert the removal of UTF-8 force encoding in JSON loading #738

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

Merged
merged 1 commit into from
Apr 29, 2025

Conversation

paracycle
Copy link
Contributor

I think the removal of the force encoding in #719 was a mistake. For example, my shell prompt includes multi-byte UTF-8 characters, and when Spring::Client::Run#run_command collects the environment variables using ENV.to_hash, the resulting JSON string becomes ASCII-8BIT. This causes issues when the Spring::Application#serve tries to load the JSON string and passes it to JSON.load. Since OkJson expects UTF-8 encoded strings, it raises an error when it encounters the ASCII-8BIT string.

I've added a test case that replicates this issue to ensure that the encoding is handled correctly. The test case creates a JSON string with multi-byte UTF-8 characters as an ASCII-8BIT string and verifies that it can be loaded without raising an error.

I think the removal of the force encoding in rails#719 was a mistake. For example, my shell prompt includes multi-byte UTF-8 characters, and when `Spring::Client::Run#run_command` collects the environment variables using `ENV.to_hash`, the resulting JSON string becomes ASCII-8BIT. This causes issues when the `Spring::Application#serve` tries to load the JSON string and passes it to `JSON.load`. Since `OkJson` expects UTF-8 encoded strings, it raises an error when it encounters the ASCII-8BIT string.

I've added a test case that replicates this issue to ensure that the encoding is handled correctly. The test case creates a JSON string with multi-byte UTF-8 characters as an ASCII-8BIT string and verifies that it can be loaded without raising an error.
@paracycle paracycle force-pushed the uk-fix-string-encoding branch from 08a7d9e to 303bff7 Compare April 29, 2025 18:58
@tenderlove tenderlove merged commit 046d50a into rails:main Apr 29, 2025
6 checks passed
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.

2 participants