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 CONTENT_LENGTH in RackResponse #379

Merged
merged 1 commit into from Mar 27, 2014
Merged

Conversation

Claster
Copy link
Contributor

@Claster Claster commented Mar 26, 2014

Hello!

I think RackResponse calculates CONTENT_LENGTH wrongly. It takes body.size. But if body has non-ASCII symbols, they will be stored in two bytes each but the length of each of them will be one.
I'm confident it is right to take body.bytesize instead.

@bblimke
Copy link
Owner

bblimke commented Mar 26, 2014

Cheers, you're right. I'm happy to merge it once travis passes :)

@Claster
Copy link
Contributor Author

Claster commented Mar 27, 2014

Fixed magic encoding. Still one test fails with jruby-head. It also fails without my commit, so I suggest it is a bug in Net::HTTP implementation in jruby :-/
https://gist.github.com/Claster/e18074cb530016e196b7
Also tests don't launch at all with ree and ruby 1.8 since rake wants ruby 1.9.

@bblimke
Copy link
Owner

bblimke commented Mar 27, 2014

Ok, I'll have to deal with rake issue in master. Thanks!

bblimke added a commit that referenced this pull request Mar 27, 2014
Fix CONTENT_LENGTH in RackResponse
@bblimke bblimke merged commit 628c4b0 into bblimke:master Mar 27, 2014
@ixti
Copy link
Contributor

ixti commented Apr 5, 2014

Although travis failed on rake. Actually there's an issue with PR itself on 1.8.7 and ree. You use force_encoding which was introduced in 1.9.2 only.

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