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 Net::HTTP nil body bug #3825

Merged
merged 1 commit into from
May 18, 2016

Conversation

chrisberkhout
Copy link

No description provided.

@headius
Copy link
Member

headius commented Apr 28, 2016

Huh, I can't even find this code in current net/http.rb.

@headius
Copy link
Member

headius commented Apr 28, 2016

It appears based on ruby/ruby@b1a0509 this functionality was moved to response.rb and additional guards were added to ensure empty bodies don't get decompressed. If you can reproduce this, it would be good to know if a Ruby 2.2-compatible runtime passes your case.

@chrisberkhout
Copy link
Author

@headius The problem is not in the JRuby 9k series, only in JRuby 1.x (including the latest 1.7.25), using the 1.9 stdlib (later stdlib versions do fix it).

I noticed there are multiple versions of the stdlib in the 1.7.25 code. My install used the 1.9 versions, and I'm not sure how the others are used (looked for documentation and command line options, but didn't find any).

My intention with the PR is that the upcoming 1.7.26 release doesn't have this no method on nil issue. I asked on IRC and understood that the PR should merge into the branch jruby-1_7 (as this one requests) to be in the next 1.x release.

@headius
Copy link
Member

headius commented Apr 28, 2016

@chrisberkhout Thank you for clarifying about 9k!

The 1.8 and 2.0 stdlibs in JRuby 1.7.x are used when running with --1.8 or --2.0 modes. The 2.0 stdlib probably doesn't have its own copy of net/http. We could fix 1.8, but it's a pretty low priority.

Thanks for the PR!

@chrisberkhout
Copy link
Author

@headius No worries.

I checked again, and the weird response we had is correctly processed by 1.8 or 2.0. So that's it! :)

@headius
Copy link
Member

headius commented May 2, 2016

@chrisberkhout Thanks for confirming!

@BanzaiMan BanzaiMan closed this May 17, 2016
@chrisberkhout
Copy link
Author

@BanzaiMan I'm wondering, why wasn't this merged?

@enebo enebo reopened this May 18, 2016
@enebo enebo merged commit 5b83abb into jruby:jruby-1_7 May 18, 2016
@enebo
Copy link
Member

enebo commented May 18, 2016

@chrisberkhout I think @BanzaiMan thought it had already been merged.

@chrisberkhout
Copy link
Author

Cool. Thanks @enebo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants