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

HTTP gem does not work with this gem #89

Closed
kapso opened this issue Jan 31, 2020 · 6 comments · Fixed by #91
Closed

HTTP gem does not work with this gem #89

kapso opened this issue Jan 31, 2020 · 6 comments · Fixed by #91
Assignees

Comments

@kapso
Copy link

kapso commented Jan 31, 2020

When I enable this gem, the HTTP gem does not return any data. So basically the following request returns an empty response.

content = HTTP.get(URL).body.to_s

Versions I am using -

ruby '2.7.0'
gem 'httplog', '1.4.0'
gem 'rails', '6.0.2.1'
gem 'http', '4.3.0'
@trusche trusche self-assigned this Jan 31, 2020
@trusche
Copy link
Owner

trusche commented Jan 31, 2020

Well that's not good... :) Checking now

@trusche
Copy link
Owner

trusche commented Jan 31, 2020

Well, it works fine with ruby 2.6.5 for me. With this setup:

# Gemfile
source 'https://rubygems.org'
ruby '2.6.5'

gem 'http'
gem 'httplog', '1.4.0'
# test.rb
require 'http'
require 'httplog'

puts HTTP.get('http://example.com').body.to_s

running bundle exec test.rb logs the request and spits out the content of example.com. Checking ruby 2.7 now...

@trusche
Copy link
Owner

trusche commented Jan 31, 2020

Works the same for me with ruby 2.7.0. http gem version is 4.3.0 - which one do you have? And can you provide a minimum example that reproduces the issue?

@trusche trusche assigned kapso and unassigned trusche Jan 31, 2020
@trusche trusche changed the title HTTP gem does not working this gem HTTP gem does not work with this gem Jan 31, 2020
@kapso
Copy link
Author

kapso commented Jan 31, 2020

I am using gem 'http', '4.3.0'

@maximeg
Copy link
Contributor

maximeg commented Feb 1, 2020

I did stumble on that on one personal project (and hopefully, we weren't affected at work due to binary only responses).

I was going to make a PR for that this morning.

The culprit is the new added .dup in HttpLog#log_body:

data = parse_body(body.dup, mask_body, encoding, content_type)

With that, you have two instances of HTTP::Response::Body. The first one to be "read" (by using to_s) will consume the response from the connection. The other to be read will get nothing.

If the content type is "loggable", the instance in httplog is read and the instance you get is empty. If binary (hence not "loggable"), that's the response you get.

A proper way is:

  • not dup here, but in parsse_body if needed
  • call to_s explicitly before that instead of relying on the implicit to_s of the string interpolation.

@kapso
Copy link
Author

kapso commented Feb 7, 2020

Thanks, @maximeg

Curious when this fix will be released?

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 a pull request may close this issue.

3 participants