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 unicode rendering in json. Correct size of response is now set #97

Merged
merged 2 commits into from Jan 10, 2014

Conversation

3 participants
@yuzeh
Contributor

yuzeh commented Jan 10, 2014

Previously, the number of characters in the string was used for Content-Length.

Now, the number of bytes in the string is used for Content-Length.

@@ -62,7 +62,7 @@ class ResponseBuilder {
case Some(j) =>
resp.headers.set("Content-Type", "application/json")
val jsonString = jsonMapper.writeValueAsString(j)
resp.headers.set("Content-Length", jsonString.length)
resp.headers.set("Content-Length", jsonString.getBytes.length)

This comment has been minimized.

@stuhood

stuhood Jan 10, 2014

Member

getBytes will force UTF-8 encoding of the data, and then throw away the result. You should try to avoid double-encoding by (re)using the encoded content from somewhere else.

This comment has been minimized.

@yuzeh

yuzeh Jan 10, 2014

Contributor

Should I be doing this instead?

val jsonString = jsonMapper.writeValueAsString(j).getBytes("utf-8")
resp.headers.set("Content-Length", jsonString.length)
resp.setContent(copiedBuffer(jsonString))

This comment has been minimized.

@stuhood

stuhood Jan 10, 2014

Member

Yea, that works. Although the naming would be:
val jsonBytes = ...getBytes("utf-8")

Dan Huang

capotej added a commit that referenced this pull request Jan 10, 2014

Merge pull request #97 from yuzeh/render_json_unicode_response_length
Fix unicode rendering in json. Correct size of response is now set

@capotej capotej merged commit 75ebe3c into twitter:master Jan 10, 2014

1 check passed

default The Travis CI build passed
Details

@yuzeh yuzeh deleted the yuzeh:render_json_unicode_response_length branch May 22, 2014

cacoco pushed a commit that referenced this pull request May 13, 2015

Merge pull request #97 from yuzeh/render_json_unicode_response_length
Fix unicode rendering in json. Correct size of response is now set
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment