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

Support UTF-8 data when using the JSON serializer #39

Merged
merged 4 commits into from
Aug 11, 2024

Conversation

jcmfernandes
Copy link
Contributor

Closes #38

@jcmfernandes
Copy link
Contributor Author

The specs don't cover the setup V1 encryptor+JSON serializer. I feel this is OK now, as the V1 encryptor is battle-tested. I'm happy to hear otherwise.

Somewhat related: assuming that most use the default V1 serializer - Marshal - it's important to note that the JSON serializer can introduce subtle changes. A key one is that it transforms symbols into strings. This leads me to the question: should we default to the V2 encryptor in the next release or make it opt-in instead?

@ioquatix
Copy link
Member

We could consider using symbolize_names or similar when using JSON. I think it's less surprising in most cases.

@jcmfernandes jcmfernandes force-pushed the fix-json-marshal-with-utf8-data branch from 218d5e0 to aae5ea4 Compare February 13, 2024 09:25
@jcmfernandes
Copy link
Contributor Author

We could consider using symbolize_names or similar when using JSON. I think it's less surprising in most cases.

We could. The bigger point is that small differences are unavoidable. We could also support Marshal in encryptor V2. I deliberately stayed away from it, but it's a decision we can revisit.

jcmfernandes and others added 2 commits August 9, 2024 17:20
@jcmfernandes jcmfernandes requested a review from ioquatix August 9, 2024 21:56
@ioquatix
Copy link
Member

Thanks for persisting with this, I'll review it today.

@ioquatix ioquatix merged commit 4c10f8b into rack:main Aug 11, 2024
20 checks passed
ioquatix added a commit that referenced this pull request Jan 4, 2025
* Replace all instance of 'BINARY' with `Encoding::BINARY`
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.

JSON serialization doesn't support UTF-8
2 participants