When using .to_rack with open-uri, StringIO#status dos not contain the HTTP status message

[postmodern]

I discovered that when using stub_request(...).to_rack with open-uri causes the resulting StringIO object to not contain the status message (ex: "OK"), and only the status code (ex: "200").

Example

require 'rspec'
require 'webmock/rspec'
require 'rack'
require 'robots'

describe Robots do
  let(:robots_txt) do
    [
      'User-Agent: *',
      'Disallow: /'
    ].join($/)
  end

  let(:root_uri) { 'http://example.com' }

  let(:rack_app) do
    proc { |env|
      if env['PATH_INFO'] == '/robots.txt'
        [200, {'Conten-Type' => 'text/plain'}, [robots_txt]]
      else
        [200, {'Conten-Type' => 'text/html'}, ['hello']]
      end
    }
  end

  before do
    stub_request(:get, "#{root_uri}/robots.txt").to_rack(rack_app)
  end

  let(:user_agent) { 'Ruby' }

  subject do
    described_class.get_robots_txt("#{root_uri}/", user_agent)
  end

  it "StringIO#status should contain 'OK'" do
    expect(subject.status).to be == ['200', 'OK']
  end
end

Output

Failures:

  1) Robots StringIO#status should contain 'OK'
     Failure/Error: expect(subject.status).to be == ['200', 'OK']
     
       expected: == ["200", "OK"]
            got:    ["200", ""]
     # ./spec/robots_spec.rb:37:in `block (2 levels) in '

[JoshCheek]

Synopsis of the problem

• Here it calls the app and passes the status as an option (side note: having that class inheriting from Response has got to be a mistake or a hack, it led me on a goose chase for a bit).
• Which gets passed to options= on line 20, here.
• Which calls status= on line 77, here
• Which sees that 200 is an Integer and sets the status to [status, ""] here

Probable Solution

Probably the right thing to do is have RackResponse#evaluate look up the reason phrase and set the status option to [status, reason_phrase] when constructing the response here.

Example of the solution

Adding these three lines to the test causes it to pass (or, as a gist):

require 'webrick/httpstatus' # picked this one b/c it's in the stdlib
rack_response_filename = $LOADED_FEATURES.find { |filename| filename =~ /rack_response/ }
eval File.read(rack_response_filename).sub(/status$/, "[status, WEBrick::HTTPStatus.reason_phrase(status)]")

PR?

Happy to submit a PR, if anyone on the receiving end cares to choose how webmock should get a list of statuses to reason phrases. I think the webmock option is reasonable since it ships with Ruby and the code is small, doesn't do anything fancy, and only requires 1 file, which is a small formatter for logging. Another option is to just copy/paste the list, but then you have to update it when these things change, which sounds annoying.

[bblimke]

@postmodern thanks for raising the issue.

@JoshCheek your suggested solution seems fine to me. I'd be happy to accept a PR

Since it's about Rack, perhaps it's worth sticking to it and using Rack::Utils::SYMBOL_TO_STATUS_CODE[status] instead of Webrick though?

[JoshCheek]

My reservation is that Rack is only a dev dependency, and it seems too big (~75 files) to make a runtime dependency just to get a list of statuses. Really, it'd be nice if Ruby broke the statuses out from webrick, seems like that list would be independently useful in a number of places (eg the cgi lib).

The one I used is here, and this is the other file it requires.

We could also just copy lines 59-111.

[bblimke]

If anybody wants to use to_rack they need to have rack loaded anyway.

The following line already depends on Rack

webmock/lib/webmock/rack_response.rb

Line 48 in 4ecb5c1

env['rack.version'] = Rack::VERSION

Rack is not and is not going to be a runtime dependency of WebMock.

If code use to_rack then rack needs to be required manually apart from webmock, but it's not a WebMock dependency.

[JoshCheek]

Okay, not super stoked about it, but since it already expects the user to have a dependency on Rack or fake it out, I'm okay with it. I'll submit the PR in a bit.

[bblimke]

Do you see a way to use it without Rack loaded?

[bblimke]

You don't need to have all gems that are referenced by the gem declared as runtime dependencies I.e webmock references most http client gems, but it doesn't mean that it depends on them.

[JoshCheek]

One alternative is to just pull them from an RFC and put them in the source. You could get the list with something like this:

$ curl 'https://tools.ietf.org/html/rfc2616' 2>/dev/null | nokogiri -e 'require "pp"; pp $_.css(".h4").map(&:text).grep(/^10\./).map { |h| h.split(/\s+/, 3).drop(1) }.map { |code, reason| [code.to_i, reason] }.to_h'

The downside is that I just went looking and don't see an RFC that includes all the options. Even Rack and Webrick deviate on which statuses they support:

$ ruby -r webrick -r rack -e 'r, w = Rack::Utils::SYMBOL_TO_STATUS_CODE.values, WEBrick::HTTPStatus::StatusMessage.keys; p (r-w) + (w-r)'
[102, 208, 226, 308, 506, 508, 510]

Another alternative is to make a gem containing just the status codes, ideally other libs would wind up adding deps on it, so that there is a single authoritative source for this kind of info, within Ruby, containing no deps of its own.

But your solution of using Rack seems to be the most correct / least effort, given all the considerations, so I'll send that for now, and if anyone likes any of the other ideas, it's easy enough to do them.

[postmodern]

I'd also use Rack or try to derive the status messages from Net::HTTPResponse::CODE_TO_OBJ. That or just dump the status messages to a YAML file.