From a42d4634d10d4f8bf7fb1ad50f789aa0559b7ad9 Mon Sep 17 00:00:00 2001 From: Myron Marston Date: Wed, 23 May 2012 23:22:21 -0700 Subject: [PATCH 1/2] Add a failing spec demonstrating a bug in the em-http-request adapter. When a request is made to a URL that returns a 3xx response and the :redirects option is set, the globally_stub_request/after_request hooks are not paired properly. Both hooks should receive the original request and the redirect-following request. This spec should probably be re-written to use the local webmock server, but I couldn't figure out how to get it to conditionally send a redirect response since it writes directly to the socket and doesn't (as far as I can tell) have the request info available in that scope...so there's not easy way to have it send a different response for different requests :(. See myronmarston/vcr#171 for the original VCR issue that caused me to investigate this bug. --- .../em_http_request/em_http_request_spec.rb | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/spec/acceptance/em_http_request/em_http_request_spec.rb b/spec/acceptance/em_http_request/em_http_request_spec.rb index 96c3f1ff5..8c7f531d5 100644 --- a/spec/acceptance/em_http_request/em_http_request_spec.rb +++ b/spec/acceptance/em_http_request/em_http_request_spec.rb @@ -11,6 +11,39 @@ include_context "with WebMock", :no_status_message + context 'when a real request is made and redirects are followed' do + before { WebMock.allow_net_connect! } + + # This url redirects to the https URL. + let(:http_url) { "http://raw.github.com:80/gist/fb555cb593f3349d53af/6921dd638337d3f6a51b0e02e7f30e3c414f70d6/vcr_gist" } + let(:https_url) { http_url.gsub('http', 'https').gsub('80', '443') } + + def make_request + EM.run do + request = EM::HttpRequest.new(http_url).get(:redirects => 1) + request.callback { EM.stop } + end + end + + it "invokes the globally_stub_request hook with both requests" do + urls = [] + WebMock.globally_stub_request { |r| urls << r.uri.to_s; nil } + + make_request + + urls.should eq([http_url, https_url]) + end + + it 'invokes the after_request hook with both requests' do + urls = [] + WebMock.after_request { |r| urls << r.uri.to_s } + + make_request + + urls.should eq([http_url, https_url]) + end + end + #functionality only supported for em-http-request 1.x if defined?(EventMachine::HttpConnection) describe "with middleware" do From 806818f3e87ad9d09acd7aba24ce91de0e8fcc06 Mon Sep 17 00:00:00 2001 From: Bartosz Blimke Date: Sun, 26 Aug 2012 22:57:05 +0200 Subject: [PATCH 2/2] Fixed em-http-adapter bug. When a request is made to a URL that returns a 3xx response and the :redirects option is set, the globally_stub_request/after_request hooks are now fired for the original request and the redirect-following request. --- .../em_http_request/em_http_request_1_x.rb | 6 ++++-- spec/acceptance/em_http_request/em_http_request_spec.rb | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/webmock/http_lib_adapters/em_http_request/em_http_request_1_x.rb b/lib/webmock/http_lib_adapters/em_http_request/em_http_request_1_x.rb index c384bbb2e..b4730646c 100644 --- a/lib/webmock/http_lib_adapters/em_http_request/em_http_request_1_x.rb +++ b/lib/webmock/http_lib_adapters/em_http_request/em_http_request_1_x.rb @@ -108,14 +108,16 @@ def send_request(head, body) end end - def set_deferred_status(status, *args) - if status == :succeeded && !stubbed_webmock_response && WebMock::CallbackRegistry.any_callbacks? + def unbind(reason = nil) + if !stubbed_webmock_response && WebMock::CallbackRegistry.any_callbacks? webmock_response = build_webmock_response WebMock::CallbackRegistry.invoke_callbacks( {:lib => :em_http_request, :real_request => true}, request_signature, webmock_response) end + @request_signature = nil + remove_instance_variable(:@stubbed_webmock_response) super end diff --git a/spec/acceptance/em_http_request/em_http_request_spec.rb b/spec/acceptance/em_http_request/em_http_request_spec.rb index 98b21e5b4..b0835d227 100644 --- a/spec/acceptance/em_http_request/em_http_request_spec.rb +++ b/spec/acceptance/em_http_request/em_http_request_spec.rb @@ -36,7 +36,7 @@ def make_request it 'invokes the after_request hook with both requests' do urls = [] - WebMock.after_request { |r| urls << r.uri.to_s } + WebMock.after_request { |req, res| urls << req.uri.to_s } make_request