vcr issue_417 - double http headers on curb redirect

[galori]

Hello,

I'm working on an issue in https://github.com/vcr/vcr (specifically vcr/vcr#417) and arrived here as part of my investigation.

A user opened an issue on our issues list in vcr, reporting "double entries" for various HTTP headers when hooking vcr into webmock with curb as the underlying HTTP library.

While investigating that issue and after getting pretty deep into the weeds with debugger I concluded that the core of the problem is that curb is returning double HTTP headers to WebMock which is then passing those on to vcr, when an HTTP redirect (301 or 302) happens.

Based on @bblimke and @myronmarston comments on that issue it is definitely not by design in vcr or in webmock. I'm not sure about curb yet.

The issue is already open on vcr, I'm opening this issue here to track it on this end as well.

I created this fork and added failing tests to showcase the problem. The failing test output is below.

(obviously, this PR is not ready to be pulled in, it is just a set of failing tests at the moment).

My main questions are:

1. Should this be fixed (or worked around) in webmock?
2. ..or should I take this further to Curb and see if it can be fixed there?

Thanks,
Gal

→ rspec spec/acceptance/curb/curb_spec.rb
Run options:
  include {:focus=>true}
  exclude {:without_webmock=>true}

All examples were filtered out; ignoring {:focus=>true}
......................................................................................................................................................................................................................................................................................FFFF.................................................................................................................................................................................................................................................................................FFFF................................................................................................................................................................................................................................................................................FFFF.............................................................................................................................................................................................................................................................................FFFF.............................................................................................................................................................................................................................................................................FFFF.................

Failures:

  1) Webmock with Curb using #http for requests it should behave like Curb with WebMock when after_request callback is declared passing response to callback when request is not stubbed for a redirect should return one and only one Server http header string
     Failure/Error: expect(server_header).to be_a String
       expected ["Microsoft-IIS/8.0", "Microsoft-IIS/8.0"] to be a kind of String
     Shared Example Group: "callbacks" called from ./spec/acceptance/webmock_shared.rb:35
     Shared Example Group: "with WebMock" called from ./spec/acceptance/curb/curb_spec.rb:10
     Shared Example Group: "Curb" called from ./spec/acceptance/curb/curb_spec.rb:341
     # ./spec/acceptance/shared/callbacks.rb:143:in `block (6 levels) in <top (required)>'

  2) Webmock with Curb using #http for requests it should behave like Curb with WebMock when after_request callback is declared passing response to callback when request is not stubbed for a redirect should return one and only one Date http header string
     Failure/Error: expect(date_header).to be_a String
       expected ["Sun, 29 Nov 2015 10:43:18 GMT", "Sun, 29 Nov 2015 10:43:18 GMT"] to be a kind of String
     Shared Example Group: "callbacks" called from ./spec/acceptance/webmock_shared.rb:35
     Shared Example Group: "with WebMock" called from ./spec/acceptance/curb/curb_spec.rb:10
     Shared Example Group: "Curb" called from ./spec/acceptance/curb/curb_spec.rb:341
     # ./spec/acceptance/shared/callbacks.rb:147:in `block (6 levels) in <top (required)>'

  3) Webmock with Curb using #http for requests it should behave like Curb with WebMock when after_request callback is declared passing response to callback when request is not stubbed for a redirect should a date header string that is a valid date
     Failure/Error: expect(Time.parse(date_header)).to be_a Time

     TypeError:
       no implicit conversion of Array into String
     Shared Example Group: "callbacks" called from ./spec/acceptance/webmock_shared.rb:35
     Shared Example Group: "with WebMock" called from ./spec/acceptance/curb/curb_spec.rb:10
     Shared Example Group: "Curb" called from ./spec/acceptance/curb/curb_spec.rb:341
     # ./spec/acceptance/shared/callbacks.rb:151:in `block (6 levels) in <top (required)>'

  4) Webmock with Curb using #http_* methods for requests it should behave like Curb with WebMock when after_request callback is declared passing response to callback when request is not stubbed for a redirect should pass the final response to callback with status and message
     Failure/Error: let(:status_code) {headers['status'][0]}

     NoMethodError:
       undefined method `[]' for nil:NilClass
     Shared Example Group: "callbacks" called from ./spec/acceptance/webmock_shared.rb:35
     Shared Example Group: "with WebMock" called from ./spec/acceptance/curb/curb_spec.rb:10
     Shared Example Group: "Curb" called from ./spec/acceptance/curb/curb_spec.rb:364
     # ./spec/acceptance/shared/callbacks.rb:134:in `block (6 levels) in <top (required)>'
     # ./spec/acceptance/shared/callbacks.rb:138:in `block (6 levels) in <top (required)>'

  5) Webmock with Curb using #http_* methods for requests it should behave like Curb with WebMock when after_request callback is declared passing response to callback when request is not stubbed for a redirect should return one and only one Server http header string
     Failure/Error: expect(server_header).to be_a String
       expected ["Microsoft-IIS/8.0", "Microsoft-IIS/8.0"] to be a kind of String
     Shared Example Group: "callbacks" called from ./spec/acceptance/webmock_shared.rb:35
     Shared Example Group: "with WebMock" called from ./spec/acceptance/curb/curb_spec.rb:10
     Shared Example Group: "Curb" called from ./spec/acceptance/curb/curb_spec.rb:364
     # ./spec/acceptance/shared/callbacks.rb:143:in `block (6 levels) in <top (required)>'

  6) Webmock with Curb using #http_* methods for requests it should behave like Curb with WebMock when after_request callback is declared passing response to callback when request is not stubbed for a redirect should return one and only one Date http header string
     Failure/Error: expect(date_header).to be_a String
       expected ["Sun, 29 Nov 2015 10:43:20 GMT", "Sun, 29 Nov 2015 10:43:20 GMT"] to be a kind of String
     Shared Example Group: "callbacks" called from ./spec/acceptance/webmock_shared.rb:35
     Shared Example Group: "with WebMock" called from ./spec/acceptance/curb/curb_spec.rb:10
     Shared Example Group: "Curb" called from ./spec/acceptance/curb/curb_spec.rb:364
     # ./spec/acceptance/shared/callbacks.rb:147:in `block (6 levels) in <top (required)>'

  7) Webmock with Curb using #http_* methods for requests it should behave like Curb with WebMock when after_request callback is declared passing response to callback when request is not stubbed for a redirect should a date header string that is a valid date
     Failure/Error: expect(Time.parse(date_header)).to be_a Time

     TypeError:
       no implicit conversion of Array into String
     Shared Example Group: "callbacks" called from ./spec/acceptance/webmock_shared.rb:35
     Shared Example Group: "with WebMock" called from ./spec/acceptance/curb/curb_spec.rb:10
     Shared Example Group: "Curb" called from ./spec/acceptance/curb/curb_spec.rb:364
     # ./spec/acceptance/shared/callbacks.rb:151:in `block (6 levels) in <top (required)>'


  8) Webmock with Curb using #perform for requests it should behave like Curb with WebMock when after_request callback is declared passing response to callback when request is not stubbed for a redirect should return one and only one Server http header string
      Failure/Error: expect(server_header).to be_a String
        expected ["Microsoft-IIS/8.0", "Microsoft-IIS/8.0"] to be a kind of String
      Shared Example Group: "callbacks" called from ./spec/acceptance/webmock_shared.rb:35
      Shared Example Group: "with WebMock" called from ./spec/acceptance/curb/curb_spec.rb:10
      Shared Example Group: "Curb" called from ./spec/acceptance/curb/curb_spec.rb:429
      # ./spec/acceptance/shared/callbacks.rb:143:in `block (6 levels) in <top (required)>'

  9) Webmock with Curb using #perform for requests it should behave like Curb with WebMock when after_request callback is declared passing response to callback when request is not stubbed for a redirect should return one and only one Date http header string
      Failure/Error: expect(date_header).to be_a String
        expected ["Sun, 29 Nov 2015 10:43:22 GMT", "Sun, 29 Nov 2015 10:43:22 GMT"] to be a kind of String
      Shared Example Group: "callbacks" called from ./spec/acceptance/webmock_shared.rb:35
      Shared Example Group: "with WebMock" called from ./spec/acceptance/curb/curb_spec.rb:10
      Shared Example Group: "Curb" called from ./spec/acceptance/curb/curb_spec.rb:429
      # ./spec/acceptance/shared/callbacks.rb:147:in `block (6 levels) in <top (required)>'

  10) Webmock with Curb using #perform for requests it should behave like Curb with WebMock when after_request callback is declared passing response to callback when request is not stubbed for a redirect should a date header string that is a valid date
      Failure/Error: expect(Time.parse(date_header)).to be_a Time

      TypeError:
        no implicit conversion of Array ["Sun, 29 Nov 2015 10:43:22 GMT", "Sun, 29 Nov 2015 10:43:22 GMT"] into String
      Shared Example Group: "callbacks" called from ./spec/acceptance/webmock_shared.rb:35
      Shared Example Group: "with WebMock" called from ./spec/acceptance/curb/curb_spec.rb:10
      Shared Example Group: "Curb" called from ./spec/acceptance/curb/curb_spec.rb:429
      # ./spec/acceptance/shared/callbacks.rb:151:in `block (6 levels) in <top (required)>'

  11) Webmock with Curb using .http_* methods for requests it should behave like Curb with WebMock when after_request callback is declared passing response to callback when request is not stubbed for a redirect should return one and only one Server http header string
      Failure/Error: expect(server_header).to be_a String
        expected ["Microsoft-IIS/8.0", "Microsoft-IIS/8.0"] to be a kind of String
      Shared Example Group: "callbacks" called from ./spec/acceptance/webmock_shared.rb:35
      Shared Example Group: "with WebMock" called from ./spec/acceptance/curb/curb_spec.rb:10
      Shared Example Group: "Curb" called from ./spec/acceptance/curb/curb_spec.rb:434
      # ./spec/acceptance/shared/callbacks.rb:143:in `block (6 levels) in <top (required)>'

  12) Webmock with Curb using .http_* methods for requests it should behave like Curb with WebMock when after_request callback is declared passing response to callback when request is not stubbed for a redirect should return one and only one Date http header string
      Failure/Error: expect(date_header).to be_a String
        expected ["Sun, 29 Nov 2015 10:43:22 GMT", "Sun, 29 Nov 2015 10:43:22 GMT"] to be a kind of String
      Shared Example Group: "callbacks" called from ./spec/acceptance/webmock_shared.rb:35
      Shared Example Group: "with WebMock" called from ./spec/acceptance/curb/curb_spec.rb:10
      Shared Example Group: "Curb" called from ./spec/acceptance/curb/curb_spec.rb:434
      # ./spec/acceptance/shared/callbacks.rb:147:in `block (6 levels) in <top (required)>'
.
.
.

Finished in 8.14 seconds (files took 0.77305 seconds to load)
1398 examples, 20 failures

[galori]

This is an excerpt from my comments in the vcr issue:

This is what comes back from curb:

HTTP/1.1 302 Found
Server: GitHub.com
Date: Sun, 03 May 2015 17:36:22 GMT
Content-Type: text/html; charset=utf-8
Status: 302 Found
Content-Security-Policy: default-src *; script-src assets-cdn.github.com collector-cdn.github.com; object-src assets-cdn.github.com; style-src 'self' 'unsafe-inline' 'unsafe-eval' assets-cdn.github.com; img-src 'self' data: assets-cdn.github.com identicons.github.com www.google-analytics.com collector.githubapp.com *.githubusercontent.com *.gravatar.com *.wp.com; media-src 'none'; frame-src 'self' render.githubusercontent.com gist.github.com www.youtube.com player.vimeo.com checkout.paypal.com; font-src assets-cdn.github.com; connect-src 'self' live.github.com wss://live.github.com uploads.github.com status.github.com api.github.com www.google-analytics.com github-cloud.s3.amazonaws.com
Cache-Control: no-cache
Vary: X-PJAX
Location: https://github.com/login?return_to=https%3A%2F%2Fgithub.com%2Faccount
X-UA-Compatible: IE=Edge,chrome=1
X-Request-Id: 1e4ff06639f884dac0e17406a57da339
X-Runtime: 0.003159
X-GitHub-Request-Id: 4C153B28:741C:ACC81DF:55465C95
Strict-Transport-Security: max-age=31536000; includeSubdomains; preload
X-Content-Type-Options: nosniff
X-XSS-Protection: 1; mode=block
X-Frame-Options: deny
Vary: Accept-Encoding
X-Served-By: 7cc969f65c7ec8d9db2fa57dcc51d323

HTTP/1.1 200 OK
Server: GitHub.com
Date: Sun, 03 May 2015 17:36:22 GMT
Content-Type: text/html; charset=utf-8
Status: 200 OK
Content-Security-Policy: default-src *; script-src assets-cdn.github.com collector-cdn.github.com; object-src assets-cdn.github.com; style-src 'self' 'unsafe-inline' 'unsafe-eval' assets-cdn.github.com; img-src 'self' data: assets-cdn.github.com identicons.github.com www.google-analytics.com collector.githubapp.com *.githubusercontent.com *.gravatar.com *.wp.com; media-src 'none'; frame-src 'self' render.githubusercontent.com gist.github.com www.youtube.com player.vimeo.com checkout.paypal.com; font-src assets-cdn.github.com; connect-src 'self' live.github.com wss://live.github.com uploads.github.com status.github.com api.github.com www.google-analytics.com github-cloud.s3.amazonaws.com
Cache-Control: no-cache, no-store
Vary: X-PJAX
X-UA-Compatible: IE=Edge,chrome=1
Set-Cookie: logged_in=no; domain=.github.com; path=/; expires=Thu, 03-May-2035 17:36:22 GMT; secure; HttpOnly
Set-Cookie: _gh_sess=eyJzZXNzaW9uX2lkIjoiZmJkN2ZmYTAzYWRkNTg3NTJiOTY2ODRhODk1YjhkZTMiLCJfY3NyZl90b2tlbiI6Im1Vc0F4amxUN3JJNzg3TWVKTzZqeklWVnBoN2tiZFFSNnJLdEdKWXprcm89In0%3D--0e0265f07e2c8df27af926028770fef7dba5ca13; path=/; secure; HttpOnly
X-Request-Id: b0c175d6ba2b2cbb072d48f2018d0c69
X-Runtime: 0.010593
X-GitHub-Request-Id: 4C153B28:741C:ACC826A:55465C96
Strict-Transport-Security: max-age=31536000; includeSubdomains; preload
X-Content-Type-Options: nosniff
X-XSS-Protection: 1; mode=block
X-Frame-Options: deny
Vary: Accept-Encoding
X-Served-By: 926b734ea1992f8ee1f88ab967a93dac

And then Webmock parses that output and creates the following Webmock::Response structure out of it:

#<WebMock::Response:0x000001034e28f8 @headers={
    "Server"=>["GitHub.com", "GitHub.com"], 
    "Date"=>["Sun, 03 May 2015 21:08:02 GMT", "Sun, 03 May 2015 21:08:02 GMT"], 
    "Content-Type"=>["text/html; charset=utf-8", "text/html; charset=utf-8"], 
    "Status"=>["200 OK", "302 Found"], 
    "Content-Security-Policy"=>["default-src *; script-src assets-cdn.github.com collector-cdn.github.com; object-src assets-cdn.github.com; style-src 'self' 'unsafe-inline' 'unsafe-eval' assets-cdn.github.com; img-src 'self' data: assets-cdn.github.com identicons.github.com www.google-analytics.com collector.githubapp.com *.githubusercontent.com *.gravatar.com *.wp.com; media-src 'none'; frame-src 'self' render.githubusercontent.com gist.github.com www.youtube.com player.vimeo.com checkout.paypal.com; font-src assets-cdn.github.com; connect-src 'self' live.github.com wss://live.github.com uploads.github.com status.github.com api.github.com www.google-analytics.com github-cloud.s3.amazonaws.com", "default-src *; script-src assets-cdn.github.com collector-cdn.github.com; object-src assets-cdn.github.com; style-src 'self' 'unsafe-inline' 'unsafe-eval' assets-cdn.github.com; img-src 'self' data: assets-cdn.github.com identicons.github.com www.google-analytics.com collector.githubapp.com *.githubusercontent.com *.gravatar.com *.wp.com; media-src 'none'; frame-src 'self' render.githubusercontent.com gist.github.com www.youtube.com player.vimeo.com checkout.paypal.com; font-src assets-cdn.github.com; connect-src 'self' live.github.com wss://live.github.com uploads.github.com status.github.com api.github.com www.google-analytics.com github-cloud.s3.amazonaws.com"], 
    "Cache-Control"=>["no-cache", "no-cache, no-store"], "Vary"=>["Accept-Encoding", "Accept-Encoding", "X-PJAX", "X-PJAX"], 
    "Location"=>"https://github.com/login?return_to=https%3A%2F%2Fgithub.com%2Faccount", 
    "X-Ua-Compatible"=>["IE=Edge,chrome=1", "IE=Edge,chrome=1"], 
    "X-Request-Id"=>["0fdeda7e8366569cbce2927a0357e842", "2d4bc82ee170d9ed4b1dace382004860"], 
    "X-Runtime"=>["0.005396", "0.008280"], 
    "X-Github-Request-Id"=>["4C153B28:0C0A:BA79F0A:55468E31", "4C153B28:0C0A:BA79FB6:55468E32"], 
    "Strict-Transport-Security"=>["max-age=31536000; includeSubdomains; preload", "max-age=31536000; includeSubdomains; preload"], 
    "X-Content-Type-Options"=>["nosniff", "nosniff"], 
    "X-Xss-Protection"=>["1; mode=block", "1; mode=block"], 
    "X-Frame-Options"=>["deny", "deny"], 
    "X-Served-By"=>["44f77bef9757b092723b0a6870733b02", "4580e595d77515caa5593194518fa90f"], 
    "Set-Cookie"=>["_gh_sess=eyJzZXNzaW9uX2lkIjoiNzlhN2U3MDVjYmRkMjIzNmEwMGI0OGQ4MjgzNmI2YTYiLCJfY3NyZl90b2tlbiI6IldzTnZ3Qm5seWp6a1hCR25EaFNZTkk3MW5sNzBEZXQzRzNKMWJhSUEyUE09In0%3D--5be807732479387d13905f97f1b43db12dc4a6a4; path=/; secure; HttpOnly", "logged_in=no; domain=.github.com; path=/; expires=Thu, 03-May-2035 21:08:02 GMT; secure; HttpOnly"]
}, 
@status=[200, "OK"], @body="", @exception=nil, @should_timeout=nil>