From 00e12b35c2ccf1714c3733e51b64b031142a73cd Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Sun, 5 Jun 2016 18:39:31 -0400 Subject: [PATCH] Move max_redirects check into Response#return! Previous refactoring would have broken the ability to call `.follow_redirection` on a redirection response exception like RestClient::Found. Instead, raise the exception at the time that the response is first prepared. --- lib/restclient/abstract_response.rb | 11 ++++++++--- spec/unit/abstract_response_spec.rb | 2 ++ spec/unit/response_spec.rb | 16 ++++++++++++++++ 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/lib/restclient/abstract_response.rb b/lib/restclient/abstract_response.rb index c7456f8..f0dc1cb 100644 --- a/lib/restclient/abstract_response.rb +++ b/lib/restclient/abstract_response.rb @@ -82,11 +82,13 @@ module RestClient when 301, 302, 307 case request.method when 'get', 'head' + check_max_redirects follow_redirection(&block) else raise exception_with_response end when 303 + check_max_redirects follow_get_redirection(&block) else raise exception_with_response @@ -178,9 +180,6 @@ module RestClient end new_args[:url] = url - if request.max_redirects <= 0 - raise exception_with_response - end new_args[:password] = request.password new_args[:user] = request.user new_args[:headers] = request.headers @@ -201,6 +200,12 @@ module RestClient new_req.execute(&block) end + def check_max_redirects + if request.max_redirects <= 0 + raise exception_with_response + end + end + def exception_with_response begin klass = Exceptions::EXCEPTIONS_MAP.fetch(code) diff --git a/spec/unit/abstract_response_spec.rb b/spec/unit/abstract_response_spec.rb index 3230254..6824aa2 100644 --- a/spec/unit/abstract_response_spec.rb +++ b/spec/unit/abstract_response_spec.rb @@ -128,6 +128,7 @@ describe RestClient::AbstractResponse, :include_helpers do it "should follow 302 redirect" do @net_http_res.should_receive(:code).and_return('302') + @response.should_receive(:check_max_redirects).and_return('fake-check') @response.should_receive(:follow_redirection).and_return('fake-redirection') @response.return!.should eq 'fake-redirection' end @@ -136,6 +137,7 @@ describe RestClient::AbstractResponse, :include_helpers do @net_http_res = response_double(code: 302, location: nil) @request = request_double() @response = MyAbstractResponse.new(@net_http_res, @request) + @response.should_receive(:check_max_redirects).and_return('fake-check') lambda { @response.return! }.should raise_error RestClient::Found end end diff --git a/spec/unit/response_spec.rb b/spec/unit/response_spec.rb index c229769..60f6c7e 100644 --- a/spec/unit/response_spec.rb +++ b/spec/unit/response_spec.rb @@ -216,6 +216,22 @@ describe RestClient::Response, :include_helpers do } WebMock.should have_requested(:get, 'http://some/redirect-2').times(5) end + + it "allows for manual following of redirects" do + stub_request(:get, 'http://some/redirect-1').to_return(:body => '', :status => 301, :headers => {'Location' => 'http://some/resource'}) + stub_request(:get, 'http://some/resource').to_return(:body => 'Qux', :status => 200) + + begin + RestClient::Request.execute(url: 'http://some/redirect-1', method: :get, max_redirects: 0) + rescue RestClient::MovedPermanently => err + resp = err.response.follow_redirection + else + raise 'notreached' + end + + resp.code.should eq 200 + resp.body.should eq 'Qux' + end end end