Fixed redirection, and move the code to Response

This commit is contained in:
Julien Kirch 2010-02-10 21:31:59 +01:00
parent 196c9720ff
commit a63148d2b8
10 changed files with 117 additions and 100 deletions

View File

@ -52,8 +52,8 @@ See RestClient::Resource docs for details.
== Exceptions
* for results code between 200 and 206 a RestClient::Response will be returned
* for results code between 301 and 303 the redirection will be automatically followed
* for other result codes a RestClient::Exception holding the Response will be raised, a specific exception class will be thrown for know error codes
* for results code between 301 and 303 the redirection will be followed if the request is a get or a head
* for other cases a RestClient::Exception holding the Response will be raised, a specific exception class will be thrown for know error codes
RestClient.get 'http://example.com/resource'
➔ RestClient::ResourceNotFound: RestClient::ResourceNotFound
@ -68,14 +68,14 @@ See RestClient::Resource docs for details.
== Result handling
A block can be passed to the RestClient method, this block will then be called with the Response.
Response.return! can be called to invoke the default response's behavior (return the Response for 200..206, raise an exception in other cases).
Response.return! can be called to invoke the default response's behavior.
# Don't raise exceptions but return the response
RestClient.get('http://example.com/resource'){|response| response}
RestClient.get('http://example.com/resource'){|response| response }
➔ 404 Resource Not Found | text/html 282 bytes
# Manage a specific error code
RestClient.get('http://my-rest-service.com/resource'){ |response|
RestClient.get('http://my-rest-service.com/resource'){ |response, &block|
case response.code
when 200
p "It worked !"
@ -83,7 +83,16 @@ Response.return! can be called to invoke the default response's behavior (return
when 423
raise SomeCustomExceptionIfYouWant
else
response.return!
response.return! &block
end
}
# Follow redirections for all request types and not only for get and head
RestClient.get('http://my-rest-service.com/resource'){ |response, &block|
if (301..303).include? code
follow_redirection
else
response.return! &block
end
}

View File

@ -3,6 +3,7 @@
- Response is no more a String, and the mixin is replaced by an abstract_response, existing calls are redirected to response body with a warning.
- enable repeated parameters RestClient.post 'http://example.com/resource', :param1 => ['one', 'two', 'three'], => :param2 => 'foo' (patch provided by Rodrigo Panachi)
- fixed the redirect code concerning relative path and query string combination (patch provided by Kevin Read)
- only get and head redirections are now followed by default, as stated in the specification
The response change may be breaking in rare cases.

View File

@ -2,10 +2,11 @@ module RestClient
class AbstractResponse
attr_reader :net_http_res
attr_reader :net_http_res, :args
def initialize net_http_res
def initialize net_http_res, args
@net_http_res = net_http_res
@args = args
end
# HTTP status code
@ -41,10 +42,16 @@ module RestClient
end
# Return the default behavior corresponding to the response code:
# the response itself for code in 200..206 and an exception in other cases
def return!
# the response itself for code in 200..206, redirection for 301..303 in get and head cases, and an exception in other cases
def return! &block
if (200..206).include? code
self
elsif (301..303).include? code
unless [:get, :head].include? args[:method]
raise Exceptions::EXCEPTIONS_MAP[code], self
else
follow_redirection &block
end
elsif Exceptions::EXCEPTIONS_MAP[code]
raise Exceptions::EXCEPTIONS_MAP[code], self
else
@ -53,9 +60,19 @@ module RestClient
end
def inspect
"#{code} #{STATUSES[code]} | #{(headers[:content_type] || '').gsub(/;.*$/, '')} #{size} bytes\n"
"#{code} #{STATUSES[code]} | #{(headers[:content_type] || '').gsub(/;.*$/, '')} #{size} bytes\n"
end
# Follow a redirection
def follow_redirection &block
url = headers[:location]
if url !~ /^http/
url = URI.parse(args[:url]).merge(url).to_s
end
redirected_args = args.dup
redirected_args[:url] = url
Request.execute redirected_args, &block
end
def AbstractResponse.beautify_headers(headers)
headers.inject({}) do |out, (key, value)|

View File

@ -13,8 +13,8 @@ module RestClient
attr_reader :file
def initialize tempfile, net_http_res
super net_http_res
def initialize tempfile, net_http_res, args
super net_http_res, args
@file = tempfile
end

View File

@ -21,11 +21,10 @@ module RestClient
# * :ssl_client_cert, :ssl_client_key, :ssl_ca_file
class Request
attr_reader :method, :url, :payload, :headers, :processed_headers,
:cookies, :user, :password, :timeout, :open_timeout,
:verify_ssl, :ssl_client_cert, :ssl_client_key, :ssl_ca_file,
:raw_response
attr_reader :method, :url, :headers, :cookies,
:payload, :user, :password, :timeout,
:open_timeout, :raw_response, :verify_ssl, :ssl_client_cert,
:ssl_client_key, :ssl_ca_file, :processed_headers, :args
def self.execute(args, &block)
new(args).execute &block
@ -48,20 +47,10 @@ module RestClient
@ssl_ca_file = args[:ssl_ca_file] || nil
@tf = nil # If you are a raw request, this is your tempfile
@processed_headers = make_headers headers
@args = args
end
def execute &block
execute_inner &block
rescue Redirect => e
@processed_headers.delete "Content-Length"
@processed_headers.delete "Content-Type"
@url = e.url
@method = :get
@payload = nil
execute &block
end
def execute_inner &block
uri = parse_url_with_auth(url)
transmit uri, net_http_request_class(method).new(uri.request_uri, processed_headers), payload, &block
end
@ -197,29 +186,20 @@ module RestClient
http_response
end
def process_result res
def process_result res, &block
if @raw_response
# We don't decode raw requests
response = RawResponse.new(@tf, res)
response = RawResponse.new(@tf, res, args)
else
response = Response.new(Request.decode(res['content-encoding'], res.body), res)
response = Response.new(Request.decode(res['content-encoding'], res.body), res, args)
end
code = res.code.to_i
if (301..303).include? code
url = res.header['Location']
if url !~ /^http/
url = URI.parse(@url).merge(url).to_s
end
raise Redirect, url
if block_given?
block.call response, &block
else
if block_given?
yield response
else
response.return!
end
response.return! &block
end
end
def self.decode content_encoding, body

View File

@ -6,8 +6,8 @@ module RestClient
attr_reader :body
def initialize body, net_http_res
super net_http_res
def initialize body, net_http_res, args
super net_http_res, args
@body = body || ""
end

View File

@ -3,7 +3,7 @@ require File.dirname(__FILE__) + '/base'
describe RestClient::AbstractResponse do
before do
@net_http_res = mock('net http response')
@response = RestClient::AbstractResponse.new(@net_http_res)
@response = RestClient::AbstractResponse.new(@net_http_res, {})
end
it "fetches the numeric response code" do

View File

@ -4,7 +4,7 @@ describe RestClient::RawResponse do
before do
@tf = mock("Tempfile", :read => "the answer is 42", :open => true)
@net_http_res = mock('net http response')
@response = RestClient::RawResponse.new(@tf, @net_http_res)
@response = RestClient::RawResponse.new(@tf, @net_http_res, {})
end
it "behaves like string" do

View File

@ -1,5 +1,8 @@
require File.dirname(__FILE__) + '/base'
require 'webmock/rspec'
include WebMock
describe RestClient::Request do
before do
@request = RestClient::Request.new(:method => :put, :url => 'http://some/resource', :payload => 'payload')
@ -154,7 +157,7 @@ describe RestClient::Request do
@request.should_receive(:net_http_request_class).with(:put).and_return(klass)
klass.should_receive(:new).and_return('result')
@request.should_receive(:transmit).with(@uri, 'result', kind_of(RestClient::Payload::Base))
@request.execute_inner
@request.execute
end
it "transmits the request with Net::HTTP" do
@ -226,11 +229,6 @@ describe RestClient::Request do
lambda { @request.transmit(@uri, 'req', nil) }.should raise_error(RestClient::ServerBrokeConnection)
end
it "execute calls execute_inner" do
@request.should_receive(:execute_inner)
@request.execute
end
it "class method execute wraps constructor" do
req = mock("rest request")
RestClient::Request.should_receive(:new).with(1 => 2).and_return(req)
@ -238,44 +236,6 @@ describe RestClient::Request do
RestClient::Request.execute(1 => 2)
end
describe "redirection" do
it "raises a Redirect with the new location when the response is in the 30x range" do
res = mock('response', :code => '301', :header => { 'Location' => 'http://new/resource'}, :[] => ['content-encoding' => ''], :body => '' )
lambda { @request.process_result(res) }.should raise_error(RestClient::Redirect) { |e| e.url.should == 'http://new/resource'}
end
it "handles redirects with relative paths" do
res = mock('response', :code => '301', :header => { 'Location' => 'index' }, :[] => ['content-encoding' => ''], :body => '' )
lambda { @request.process_result(res) }.should raise_error(RestClient::Redirect) { |e| e.url.should == 'http://some/index' }
end
it "handles redirects with relative path and query string" do
res = mock('response', :code => '301', :header => { 'Location' => 'index?q=1' }, :[] => ['content-encoding' => ''], :body => '' )
lambda { @request.process_result(res) }.should raise_error(RestClient::Redirect) { |e| e.url.should == 'http://some/index?q=1' }
end
it "handles redirects with absolute paths" do
@request.instance_variable_set('@url', 'http://some/place/else')
res = mock('response', :code => '301', :header => { 'Location' => '/index' }, :[] => ['content-encoding' => ''], :body => '' )
lambda { @request.process_result(res) }.should raise_error(RestClient::Redirect) { |e| e.url.should == 'http://some/index' }
end
it "uses GET and clears payload and removes possible harmful headers when following 30x redirects" do
url = "http://example.com/redirected"
@request.should_receive(:execute_inner).once.ordered.and_raise(RestClient::Redirect.new(url))
@request.should_receive(:execute_inner).once.ordered do
@request.processed_headers.should_not have_key("Content-Length")
@request.url.should == url
@request.method.should == :get
@request.payload.should be_nil
end
@request.execute
end
end
describe "exception" do
it "raises Unauthorized when the response is 401" do
res = mock('response', :code => '401', :[] => ['content-encoding' => ''], :body => '' )

View File

@ -1,9 +1,12 @@
require File.dirname(__FILE__) + '/base'
require 'webmock/rspec'
include WebMock
describe RestClient::Response do
before do
@net_http_res = mock('net http response', :to_hash => {"Status" => ["200 OK"]})
@response = RestClient::Response.new('abc', @net_http_res)
@response = RestClient::Response.new('abc', @net_http_res, {})
end
it "behaves like string" do
@ -11,7 +14,7 @@ describe RestClient::Response do
end
it "accepts nil strings and sets it to empty for the case of HEAD" do
RestClient::Response.new(nil, @net_http_res).should.to_s == ""
RestClient::Response.new(nil, @net_http_res, {}).should.to_s == ""
end
it "test headers and raw headers" do
@ -22,14 +25,14 @@ describe RestClient::Response do
describe "cookie processing" do
it "should correctly deal with one Set-Cookie header with one cookie inside" do
net_http_res = mock('net http response', :to_hash => {"etag" => ["\"e1ac1a2df945942ef4cac8116366baad\""], "set-cookie" => ["main_page=main_page_no_rewrite; path=/; expires=Tue, 20-Jan-2015 15:03:14 GMT"]})
response = RestClient::Response.new('abc', net_http_res)
response = RestClient::Response.new('abc', net_http_res, {})
response.headers[:set_cookie].should == ["main_page=main_page_no_rewrite; path=/; expires=Tue, 20-Jan-2015 15:03:14 GMT"]
response.cookies.should == { "main_page" => "main_page_no_rewrite" }
end
it "should correctly deal with multiple cookies [multiple Set-Cookie headers]" do
net_http_res = mock('net http response', :to_hash => {"etag" => ["\"e1ac1a2df945942ef4cac8116366baad\""], "set-cookie" => ["main_page=main_page_no_rewrite; path=/; expires=Tue, 20-Jan-2015 15:03:14 GMT", "remember_me=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT", "user=somebody; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT"]})
response = RestClient::Response.new('abc', net_http_res)
response = RestClient::Response.new('abc', net_http_res, {})
response.headers[:set_cookie].should == ["main_page=main_page_no_rewrite; path=/; expires=Tue, 20-Jan-2015 15:03:14 GMT", "remember_me=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT", "user=somebody; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT"]
response.cookies.should == {
"main_page" => "main_page_no_rewrite",
@ -40,7 +43,7 @@ describe RestClient::Response do
it "should correctly deal with multiple cookies [one Set-Cookie header with multiple cookies]" do
net_http_res = mock('net http response', :to_hash => {"etag" => ["\"e1ac1a2df945942ef4cac8116366baad\""], "set-cookie" => ["main_page=main_page_no_rewrite; path=/; expires=Tue, 20-Jan-2015 15:03:14 GMT, remember_me=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT, user=somebody; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT"]})
response = RestClient::Response.new('abc', net_http_res)
response = RestClient::Response.new('abc', net_http_res, {})
response.cookies.should == {
"main_page" => "main_page_no_rewrite",
"remember_me" => "",
@ -53,7 +56,7 @@ describe RestClient::Response do
it "should return itself for normal codes" do
(200..206).each do |code|
net_http_res = mock('net http response', :code => '200')
response = RestClient::Response.new('abc', net_http_res)
response = RestClient::Response.new('abc', net_http_res, {})
response.return!
end
end
@ -62,7 +65,7 @@ describe RestClient::Response do
RestClient::Exceptions::EXCEPTIONS_MAP.each_key do |code|
unless (200..206).include? code
net_http_res = mock('net http response', :code => code.to_i)
response = RestClient::Response.new('abc', net_http_res)
response = RestClient::Response.new('abc', net_http_res, {})
lambda { response.return!}.should raise_error
end
end
@ -70,5 +73,52 @@ describe RestClient::Response do
end
describe "redirection" do
it "follows a redirection when the request is a get" do
stub_request(:get, 'http://some/resource').to_return(:body => '', :status => 301, :headers => {'Location' => 'http://new/resource'})
stub_request(:get, 'http://new/resource').to_return(:body => 'Foo')
RestClient::Request.execute(:url => 'http://some/resource', :method => :get).body.should == 'Foo'
end
it "doesn't follow a redirection when the request is a post" do
net_http_res = mock('net http response', :code => 301)
response = RestClient::Response.new('abc', net_http_res, {:method => :post})
lambda { response.return!}.should raise_error(RestClient::MovedPermanently)
end
it "doesn't follow a redirection when the request is a put" do
net_http_res = mock('net http response', :code => 301)
response = RestClient::Response.new('abc', net_http_res, {:method => :put})
lambda { response.return!}.should raise_error(RestClient::MovedPermanently)
end
it "follows a redirection when the request is a head" do
stub_request(:head, 'http://some/resource').to_return(:body => '', :status => 301, :headers => {'Location' => 'http://new/resource'})
stub_request(:head, 'http://new/resource').to_return(:body => 'Foo')
RestClient::Request.execute(:url => 'http://some/resource', :method => :head).body.should == 'Foo'
end
it "handles redirects with relative paths" do
stub_request(:get, 'http://some/resource').to_return(:body => '', :status => 301, :headers => {'Location' => 'index'})
stub_request(:get, 'http://some/index').to_return(:body => 'Foo')
RestClient::Request.execute(:url => 'http://some/resource', :method => :get).body.should == 'Foo'
end
it "handles redirects with relative path and query string" do
stub_request(:get, 'http://some/resource').to_return(:body => '', :status => 301, :headers => {'Location' => 'index?q=1'})
stub_request(:get, 'http://some/index?q=1').to_return(:body => 'Foo')
RestClient::Request.execute(:url => 'http://some/resource', :method => :get).body.should == 'Foo'
end
it "follow a redirection when the request is a get and the response is in the 30x range" do
stub_request(:get, 'http://some/resource').to_return(:body => '', :status => 301, :headers => {'Location' => 'http://new/resource'})
stub_request(:get, 'http://new/resource').to_return(:body => 'Foo')
RestClient::Request.execute(:url => 'http://some/resource', :method => :get).body.should == 'Foo'
end
end
end