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 encoding to UTF-8 for res.json and res.jsonp #5606

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

UlisesGascon
Copy link
Member

Based on the IETF RFC4627:

JSON text SHALL be encoded in Unicode. The default encoding is UTF-8

I updated some references for the res.json and res.jsonp. The thing is that the tests are passing w/o additional changes so I am wondering if I am missing something here.

@wesleytodd
Copy link
Member

Pretty sure this is something we would need to land in v5 right? I think explicit is sometimes better, but I would need to do some research on how different clients behave to know for sure. Have you looked beyond the spec to see if there is some reason this might have been left off despite being pretty clearly specified?

@dougwilson
Copy link
Contributor

It was left off bc code elsewhere adds it already. If you look at the http response qithout this change the charset is there in published versions.

The reason it is added elsewhere is bc nothing in that function is writing out utf-8 bytes to the response, so it leaves the part of the code that does that responsible to setting rhe charset of the bytes it actually wrote on the wire.

@wesleytodd
Copy link
Member

Ah, that makes sense. Thanks for the clarification! Do you have a link to where it does happen, just making sure I understand this going forward.

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.

None yet

3 participants