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 Content-Length with non ASCII-8BIT strings. #12

Closed
wants to merge 3 commits into from

Conversation

meh
Copy link
Contributor

@meh meh commented Sep 29, 2011

I was implementing a proxy and noticed I was getting a truncated result, after some time thinking it was my fault I thought about the Content-Length being wrong, and apparently evma_httpserver uses #length instead of #bytesize.

Putting #bytesize everything works properly.

Sorry for the change in the gemspec but it wasn't building with this error:

ERROR:  While executing gem ... (Gem::InvalidSpecificationException)
    cert_chain must not be nil

I also faced another problem with header names, it would be really best if you used the common names and not weirdly different names, for instance: Content-Length instead of Content-length. If it's wanted I could write a patch for this and send another pull request. (EDIT: #10 this does what I meant)

Thanks.

@@ -142,7 +146,7 @@ module EventMachine
#
def fixup_headers
if @content
@headers["Content-length"] = @content.to_s.length
@headers["Content-length"] = @content.bytesize
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be content instead of @content, to prevent calling nil.bytesize

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, @content presence is already checked above, there's no chance for it to be nil.

@rcarvalho
Copy link

Thanks for that fix, guys. Is anyone still taking ownership of this code? I see that they still haven't merged your fix in. You wouldn't have happened to publish a new version to rubygems.org would you? I guess I could just fork and merge in your changes myself.

rcarvalho pushed a commit to rcarvalho/evma_httpserver that referenced this pull request Jul 19, 2012
@jay2u
Copy link

jay2u commented Nov 23, 2012

This is actually still an issue. We are using a monkey patch to fix this at the moment (after spending quite some time finding the origin of this problem) only to find out there was already a pull request for this...

Any reason this cannot be merged into the master branch?

Don't get me wrong. Thanks for a great piece of software otherwise. Brilliant stuff...

@halida
Copy link

halida commented Nov 25, 2013

The newest gem version 0.2.1 still has this problem, consider release new version?

@philayres
Copy link

@halida - I just found this too. I worked around the issue directly in my code with:

response.content = content.force_encoding(Encoding::ASCII_8BIT)

Its a shame that the current version of the gem doesn't handle this. Time for a new version.

@jay2u
Copy link

jay2u commented Feb 19, 2014

@philayres Yes that would work but only with ASCII_8BIT. The issue here, amongst other things, is trying to avoid unicode body payloads becoming truncated.

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.

6 participants