Remove args from Response and normalize method.

It is redundant and confusing to pass both the Request args and the
Request when creating a RestClient::Response. Instead, when the response
object needs to read args from the request, access them on the request
object itself. For determining the HTTP method of the request, switch to
calling RestClient::Request#method.

Also normalize the Request#method to be a lowercase string. This makes
handling of redirection and other method-specific functionality actually
work regardless of how the method was provided to Request.new (:get,
'GET', 'get').

Fixes: #461
Fixes: #462
Fixes: #463

lib/restclient/abstract_response.rb

@@ -5,7 +5,7 @@ module RestClient
 
   module AbstractResponse
 
-    attr_reader :net_http_res, :args, :request
+    attr_reader :net_http_res, :request
 
     def inspect
       raise NotImplementedError.new('must override in subclass')
@@ -31,9 +31,8 @@ module RestClient
       @raw_headers ||= @net_http_res.to_hash
     end
 
-    def response_set_vars(net_http_res, args, request)
+    def response_set_vars(net_http_res, request)
       @net_http_res = net_http_res
-      @args = args
       @request = request
 
       # prime redirection history
@@ -67,17 +66,27 @@ module RestClient
     end
 
     # Return the default behavior corresponding to the response code:
-    # the response itself for code in 200..206, redirection for 301, 302 and 307 in get and head cases, redirection for 303 and an exception in other cases
+    #
+    # For 20x status codes: return the response itself
+    #
+    # For 30x status codes:
+    #   301, 302, 307: redirect GET / HEAD if there is a Location header
+    #   303: redirect, changing method to GET, if there is a Location header
+    #
+    # For all other responses, raise a response exception
+    #
     def return!(&block)
-      if (200..207).include? code
+      case code
+      when 200..207
         self
-      elsif [301, 302, 307].include? code
+      when 301, 302, 307
-        unless [:get, :head].include? args[:method]
+        case request.method
-          raise exception_with_response
+        when 'get', 'head'
-        else
           follow_redirection(&block)
+        else
+          raise exception_with_response
         end
-      elsif code == 303
+      when 303
         follow_get_redirection(&block)
       else
         raise exception_with_response
@@ -96,13 +105,13 @@ module RestClient
     # Follow a redirection response by making a new HTTP request to the
     # redirection target.
     def follow_redirection(&block)
-      _follow_redirection(@args.dup, &block)
+      _follow_redirection(request.args.dup, &block)
     end
 
     # Follow a redirection response, but change the HTTP method to GET and drop
     # the payload from the original request.
     def follow_get_redirection(&block)
-      new_args = @args.dup
+      new_args = request.args.dup
       new_args[:method] = :get
       new_args.delete(:payload)
 

lib/restclient/raw_response.rb

@@ -19,9 +19,8 @@ module RestClient
       "<RestClient::RawResponse @code=#{code.inspect}, @file=#{file.inspect}, @request=#{request.inspect}>"
     end
 
-    def initialize(tempfile, net_http_res, args, request)
+    def initialize(tempfile, net_http_res, request)
       @net_http_res = net_http_res
-      @args = args
       @file = tempfile
       @request = request
     end

lib/restclient/request.rb

@@ -115,7 +115,7 @@ module RestClient
     end
 
     def initialize args
-      @method = args[:method] or raise ArgumentError, "must pass :method"
+      @method = normalize_method(args[:method])
       @headers = (args[:headers] || {}).dup
       if args[:url]
         @url = process_url_params(args[:url], headers)
@@ -332,7 +332,7 @@ module RestClient
     end
 
     def net_http_request_class(method)
-      Net::HTTP.const_get(method.to_s.capitalize)
+      Net::HTTP.const_get(method.capitalize, false)
     end
 
     def net_http_do_request(http, req, body=nil, &block)
@@ -533,6 +533,22 @@ module RestClient
       warned
     end
 
+    # Parse a method and return a normalized string version.
+    #
+    # Raise ArgumentError if the method is falsy, but otherwise do no
+    # validation.
+    #
+    # @param method [String, Symbol]
+    #
+    # @return [String]
+    #
+    # @see net_http_request_class
+    #
+    def normalize_method(method)
+      raise ArgumentError.new('must pass :method') unless method
+      method.to_s.downcase
+    end
+
     def transmit uri, req, payload, & block
 
       # We set this to true in the net/http block so that we can distinguish
@@ -696,10 +712,10 @@ module RestClient
     def process_result res, & block
       if @raw_response
         # We don't decode raw requests
-        response = RawResponse.new(@tf, res, args, self)
+        response = RawResponse.new(@tf, res, self)
       else
         decoded = Request.decode(res['content-encoding'], res.body)
-        response = Response.create(decoded, res, args, self)
+        response = Response.create(decoded, res, self)
       end
 
       if block_given?

lib/restclient/response.rb

@@ -38,10 +38,10 @@ module RestClient
       "<RestClient::Response #{code.inspect} #{body_truncated(10).inspect}>"
     end
 
-    def self.create(body, net_http_res, args, request)
+    def self.create(body, net_http_res, request)
       result = self.new(body || '')
 
-      result.response_set_vars(net_http_res, args, request)
+      result.response_set_vars(net_http_res, request)
       fix_encoding(result)
 
       result

spec/helpers.rb

@@ -11,4 +11,9 @@ module Helpers
   ensure
     $stderr = original_stderr
   end
+
+  def request_double(url: 'http://example.com', method: 'get')
+    double('request', url: url, method: method, user: nil, password: nil,
+           redirection_history: nil, args: {url: url, method: method})
+  end
 end

spec/unit/abstract_response_spec.rb

@@ -1,6 +1,6 @@
 require_relative '_lib'
 
-describe RestClient::AbstractResponse do
+describe RestClient::AbstractResponse, :include_helpers do
 
   class MyAbstractResponse
 
@@ -8,9 +8,8 @@ describe RestClient::AbstractResponse do
 
     attr_accessor :size
 
-    def initialize net_http_res, args, request
+    def initialize net_http_res, request
       @net_http_res = net_http_res
-      @args = args
       @request = request
     end
 
@@ -18,8 +17,8 @@ describe RestClient::AbstractResponse do
 
   before do
     @net_http_res = double('net http response')
-    @request = double('restclient request', :url => 'http://example.com')
+    @request = double('restclient request', url: 'http://example.com', method: 'get')
-    @response = MyAbstractResponse.new(@net_http_res, {}, @request)
+    @response = MyAbstractResponse.new(@net_http_res, @request)
   end
 
   it "fetches the numeric response code" do
@@ -95,8 +94,15 @@ describe RestClient::AbstractResponse do
 
     it "should raise an error on a redirection after non-GET/HEAD requests" do
       @net_http_res.should_receive(:code).and_return('301')
-      @response.args.merge(:method => :put)
+      @request.should_receive(:method).and_return('put')
+      @response.should_not_receive(:follow_redirection)
       lambda { @response.return! }.should raise_error RestClient::RequestFailed
     end
+
+    it "should follow 302 redirect" do
+      @net_http_res.should_receive(:code).and_return('302')
+      @response.should_receive(:follow_redirection).and_return('fake-redirection')
+      @response.return!.should eq 'fake-redirection'
+    end
   end
 end

spec/unit/raw_response_spec.rb

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

spec/unit/request_spec.rb

@@ -264,7 +264,7 @@ describe RestClient::Request, :include_helpers do
 
   it "executes by constructing the Net::HTTP object, headers, and payload and calling transmit" do
     klass = double("net:http class")
-    @request.should_receive(:net_http_request_class).with(:put).and_return(klass)
+    @request.should_receive(:net_http_request_class).with('put').and_return(klass)
     klass.should_receive(:new).and_return('result')
     @request.should_receive(:transmit).with(@request.uri, 'result', kind_of(RestClient::Payload::Base))
     @request.execute
@@ -273,7 +273,7 @@ describe RestClient::Request, :include_helpers do
   it "IPv6: executes by constructing the Net::HTTP object, headers, and payload and calling transmit" do
     @request = RestClient::Request.new(:method => :put, :url => 'http://[::1]/some/resource', :payload => 'payload')
     klass = double("net:http class")
-    @request.should_receive(:net_http_request_class).with(:put).and_return(klass)
+    @request.should_receive(:net_http_request_class).with('put').and_return(klass)
 
     if RUBY_VERSION >= "2.0.0"
       klass.should_receive(:new).with(kind_of(URI), kind_of(Hash)).and_return('result')

spec/unit/response_spec.rb

@@ -4,8 +4,8 @@ describe RestClient::Response, :include_helpers do
   before do
     @net_http_res = double('net http response', :to_hash => {"Status" => ["200 OK"]}, :code => 200)
     @example_url = 'http://example.com'
-    @request = double('http request', :user => nil, :password => nil, :url => @example_url, :redirection_history => nil)
+    @request = request_double(url: @example_url, method: 'get')
-    @response = RestClient::Response.create('abc', @net_http_res, {}, @request)
+    @response = RestClient::Response.create('abc', @net_http_res, @request)
   end
 
   it "behaves like string" do
@@ -17,7 +17,7 @@ describe RestClient::Response, :include_helpers do
   end
 
   it "accepts nil strings and sets it to empty for the case of HEAD" do
-    RestClient::Response.create(nil, @net_http_res, {}, @request).to_s.should eq ""
+    RestClient::Response.create(nil, @net_http_res, @request).to_s.should eq ""
   end
 
   describe 'header processing' do
@@ -29,8 +29,8 @@ describe RestClient::Response, :include_helpers do
     it 'handles multiple headers by joining with comma' do
       @net_http_res = double('net http response', :to_hash => {'My-Header' => ['foo', 'bar']}, :code => 200)
       @example_url = 'http://example.com'
-      @request = double('http request', :user => nil, :password => nil, :url => @example_url, :redirection_history => nil)
+      @request = request_double(url: @example_url, method: 'get')
-      @response = RestClient::Response.create('abc', @net_http_res, {}, @request)
+      @response = RestClient::Response.create('abc', @net_http_res, @request)
 
       @response.raw_headers['My-Header'].should eq ['foo', 'bar']
       @response.headers[:my_header].should eq 'foo, bar'
@@ -42,14 +42,14 @@ describe RestClient::Response, :include_helpers do
       header_val = "main_page=main_page_no_rewrite; path=/; expires=Sat, 10-Jan-2037 15:03:14 GMT".freeze
 
       net_http_res = double('net http response', :to_hash => {"etag" => ["\"e1ac1a2df945942ef4cac8116366baad\""], "set-cookie" => [header_val]})
-      response = RestClient::Response.create('abc', net_http_res, {}, @request)
+      response = RestClient::Response.create('abc', net_http_res, @request)
       response.headers[:set_cookie].should eq [header_val]
       response.cookies.should eq({ "main_page" => "main_page_no_rewrite" })
     end
 
     it "should correctly deal with multiple cookies [multiple Set-Cookie headers]" do
       net_http_res = double('net http response', :to_hash => {"etag" => ["\"e1ac1a2df945942ef4cac8116366baad\""], "set-cookie" => ["main_page=main_page_no_rewrite; path=/; expires=Sat, 10-Jan-2037 15:03:14 GMT", "remember_me=; path=/; expires=Sat, 10-Jan-2037 00:00:00 GMT", "user=somebody; path=/; expires=Sat, 10-Jan-2037 00:00:00 GMT"]})
-      response = RestClient::Response.create('abc', net_http_res, {}, @request)
+      response = RestClient::Response.create('abc', net_http_res, @request)
       response.headers[:set_cookie].should eq ["main_page=main_page_no_rewrite; path=/; expires=Sat, 10-Jan-2037 15:03:14 GMT", "remember_me=; path=/; expires=Sat, 10-Jan-2037 00:00:00 GMT", "user=somebody; path=/; expires=Sat, 10-Jan-2037 00:00:00 GMT"]
       response.cookies.should eq({
         "main_page" => "main_page_no_rewrite",
@@ -60,7 +60,7 @@ describe RestClient::Response, :include_helpers do
 
     it "should correctly deal with multiple cookies [one Set-Cookie header with multiple cookies]" do
       net_http_res = double('net http response', :to_hash => {"etag" => ["\"e1ac1a2df945942ef4cac8116366baad\""], "set-cookie" => ["main_page=main_page_no_rewrite; path=/; expires=Sat, 10-Jan-2037 15:03:14 GMT, remember_me=; path=/; expires=Sat, 10-Jan-2037 00:00:00 GMT, user=somebody; path=/; expires=Sat, 10-Jan-2037 00:00:00 GMT"]})
-      response = RestClient::Response.create('abc', net_http_res, {}, @request)
+      response = RestClient::Response.create('abc', net_http_res, @request)
       response.cookies.should eq({
         "main_page" => "main_page_no_rewrite",
         "remember_me" => "",
@@ -73,7 +73,7 @@ describe RestClient::Response, :include_helpers do
     it "should return itself for normal codes" do
       (200..206).each do |code|
         net_http_res = response_double(:code => '200')
-        resp = RestClient::Response.create('abc', net_http_res, {}, @request)
+        resp = RestClient::Response.create('abc', net_http_res, @request)
         resp.return!
       end
     end
@@ -82,7 +82,7 @@ describe RestClient::Response, :include_helpers do
       RestClient::Exceptions::EXCEPTIONS_MAP.each_key do |code|
         unless (200..207).include? code
           net_http_res = response_double(:code => code.to_i)
-          resp = RestClient::Response.create('abc', net_http_res, {}, @request)
+          resp = RestClient::Response.create('abc', net_http_res, @request)
           lambda { resp.return! }.should raise_error
         end
       end
@@ -128,8 +128,9 @@ describe RestClient::Response, :include_helpers do
 
     it "doesn't follow a 301 when the request is a post" do
       net_http_res = response_double(:code => 301)
+
       response = RestClient::Response.create('abc', net_http_res,
-                                             {:method => :post}, @request)
+                                             request_double(method: 'post'))
       lambda {
         response.return!
       }.should raise_error(RestClient::MovedPermanently)
@@ -138,7 +139,7 @@ describe RestClient::Response, :include_helpers do
     it "doesn't follow a 302 when the request is a post" do
       net_http_res = response_double(:code => 302)
       response = RestClient::Response.create('abc', net_http_res,
-                                             {:method => :post}, @request)
+                                             request_double(method: 'post'))
       lambda {
         response.return!
       }.should raise_error(RestClient::Found)
@@ -147,7 +148,8 @@ describe RestClient::Response, :include_helpers do
     it "doesn't follow a 307 when the request is a post" do
       net_http_res = response_double(:code => 307)
       response = RestClient::Response.create('abc', net_http_res,
-                                             {:method => :post}, @request)
+                                             request_double(method: 'post'))
+      response.should_not_receive(:follow_redirection)
       lambda {
         response.return!
       }.should raise_error(RestClient::TemporaryRedirect)
@@ -156,7 +158,7 @@ describe RestClient::Response, :include_helpers do
     it "doesn't follow a redirection when the request is a put" do
       net_http_res = response_double(:code => 301)
       response = RestClient::Response.create('abc', net_http_res,
-                                             {:method => :put}, @request)
+                                             request_double(method: 'put'))
       lambda {
         response.return!
       }.should raise_error(RestClient::MovedPermanently)