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

Fix wrong behavior in marshalling invalid UTF-8 byte with EscapeHTML=false #664

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

Conversation

kz-sher
Copy link

@kz-sher kz-sher commented Mar 9, 2023

See #663

@kz-sher kz-sher changed the title Fix invalid behavior in marshalling invalid UTF-8 byte with EscapeHTML=false Fix wrong behavior in marshalling invalid UTF-8 byte with EscapeHTML=false Mar 9, 2023
@kz-sher
Copy link
Author

kz-sher commented Mar 13, 2023

@taowen

@taowen
Copy link
Contributor

taowen commented Mar 13, 2023

split it into two functions, duplicate the code is ok

@kz-sher kz-sher force-pushed the fix-json-marshal-invalid-utf8-char branch from 88045f2 to ca479af Compare March 13, 2023 09:56
@kz-sher
Copy link
Author

kz-sher commented Mar 13, 2023

done

@taowen
Copy link
Contributor

taowen commented Mar 13, 2023

this will affect all users of writeStringSlowPath. do not want to make such a impactful change.

@kz-sher
Copy link
Author

kz-sher commented Mar 13, 2023

@taowen This is the same with https://go.dev/src/encoding/json/encode.go#L1029. The code changes essentially restrict contents to be UTF-8. According to golang/go#36686 (comment), I think the users should have accepted this behavior. If they opt for another encodings, they should follow JSON convention by doing transformation in advance.

@taowen
Copy link
Contributor

taowen commented Mar 15, 2023

they can opt to use the standard library

@kz-sher
Copy link
Author

kz-sher commented Mar 16, 2023

@taowen I think jsoniter is created to be one of the better alternatives of official JSON pkg. Should you be considering this feature to be added in another ways such as having Config.MarshalByteWithUTF8Coerced option? It seems like its Java version has this support.

@taowen
Copy link
Contributor

taowen commented Mar 16, 2023

you may add my wechat "nctaowen"

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