Merge pull request #365 from rest-client/ab-cookies

Add standards-compliant cookie handling.

Merge branch 'ab-cookies' into 2.0.x

Conflicts:
	history.md
	lib/restclient/abstract_response.rb
	lib/restclient/response.rb
	lib/restclient/version.rb
	spec/unit/request_spec.rb
	spec/unit/response_spec.rb

lib/restclient/abstract_response.rb

@@ -1,10 +1,11 @@
 require 'cgi'
+require 'http-cookie'
 
 module RestClient
 
   module AbstractResponse
 
-    attr_reader :net_http_res, :args
+    attr_reader :net_http_res, :args, :request
 
     # HTTP status code
     def code
@@ -22,11 +23,36 @@ def raw_headers
       @raw_headers ||= @net_http_res.to_hash
     end
 
+    def response_set_vars(net_http_res, args, request)
+      @net_http_res = net_http_res
+      @args = args
+      @request = request
+    end
+
     # Hash of cookies extracted from response headers
     def cookies
-      @cookies ||= (self.headers[:set_cookie] || {}).inject({}) do |out, cookie_content|
-        out.merge parse_cookie(cookie_content)
+      hash = {}
+
+      cookie_jar.cookies.each do |cookie|
+        hash[cookie.name] = cookie.value
       end
+
+      hash
+    end
+
+    # Cookie jar extracted from response headers.
+    #
+    # @return [HTTP::CookieJar]
+    #
+    def cookie_jar
+      return @cookie_jar if @cookie_jar
+
+      jar = HTTP::CookieJar.new
+      headers.fetch(:set_cookie, []).each do |cookie|
+        jar.parse(cookie, @request.url)
+      end
+
+      @cookie_jar = jar
     end
 
     # Return the default behavior corresponding to the response code:
@@ -66,26 +92,28 @@ def description
     # @param result [Net::HTTPResponse, nil]
     #
     def follow_redirection request = nil, result = nil, & block
+      new_args = @args.dup
+
       url = headers[:location]
       if url !~ /^http/
-        url = URI.parse(args[:url]).merge(url).to_s
+        url = URI.parse(request.url).merge(url).to_s
       end
-      args[:url] = url
+      new_args[:url] = url
       if request
         if request.max_redirects == 0
           raise MaxRedirectsReached
         end
-        args[:password] = request.password
-        args[:user] = request.user
-        args[:headers] = request.headers
-        args[:max_redirects] = request.max_redirects - 1
-        # pass any cookie set in the result
-        if result && result['set-cookie']
-          args[:headers][:cookies] = (args[:headers][:cookies] || {}).merge(parse_cookie(result['set-cookie']))
-        end
+        new_args[:password] = request.password
+        new_args[:user] = request.user
+        new_args[:headers] = request.headers
+        new_args[:max_redirects] = request.max_redirects - 1
+
+        # TODO: figure out what to do with original :cookie, :cookies values
+        new_args[:headers]['Cookie'] = HTTP::Cookie.cookie_value(
+          cookie_jar.cookies(new_args.fetch(:url)))
       end
 
-      Request.execute args, &block
+      Request.execute(new_args, &block)
     end
 
     def self.beautify_headers(headers)

lib/restclient/raw_response.rb

@@ -13,12 +13,13 @@ class RawResponse
 
     include AbstractResponse
 
-    attr_reader :file
+    attr_reader :file, :request
 
-    def initialize tempfile, net_http_res, args
+    def initialize(tempfile, net_http_res, args, request)
       @net_http_res = net_http_res
       @args = args
       @file = tempfile
+      @request = request
     end
 
     def to_s

lib/restclient/request.rb

@@ -531,9 +531,9 @@ def fetch_body(http_response)
     def process_result res, & block
       if @raw_response
         # We don't decode raw requests
-        response = RawResponse.new(@tf, res, args)
+        response = RawResponse.new(@tf, res, args, self)
       else
-        response = Response.create(Request.decode(res['content-encoding'], res.body), res, args)
+        response = Response.create(Request.decode(res['content-encoding'], res.body), res, args, self)
       end
 
       if block_given?

lib/restclient/response.rb

@@ -6,18 +6,17 @@ module Response
 
     include AbstractResponse
 
-    attr_accessor :args, :net_http_res
-
     def body
       self
     end
 
-    def self.create body, net_http_res, args
+    def self.create body, net_http_res, args, request
       result = body || ''
       result.extend Response
-      result.net_http_res = net_http_res
-      result.args = args
+
+      result.response_set_vars(net_http_res, args, request)
       fix_encoding(result)
+
       result
     end
 

rest-client.gemspec

@@ -23,6 +23,7 @@ Gem::Specification.new do |s|
   s.add_development_dependency('rdoc', '>= 2.4.2', '< 5.0')
   s.add_development_dependency('rubocop')
 
+  s.add_dependency('http-cookie', '>= 1.0.2', '< 2.0')
   s.add_dependency('mime-types', '>= 1.16', '< 3.0')
   s.add_dependency('netrc', '~> 0.8')
 

spec/unit/abstract_response_spec.rb

@@ -8,16 +8,18 @@ class MyAbstractResponse
 
     attr_accessor :size
 
-    def initialize net_http_res, args
+    def initialize net_http_res, args, request
       @net_http_res = net_http_res
       @args = args
+      @request = request
     end
 
   end
 
   before do
     @net_http_res = double('net http response')
-    @response = MyAbstractResponse.new(@net_http_res, {})
+    @request = double('restclient request', :url => 'http://example.com')
+    @response = MyAbstractResponse.new(@net_http_res, {}, @request)
   end
 
   it "fetches the numeric response code" do
@@ -53,7 +55,8 @@ def initialize net_http_res, args
 
   it "extract strange cookies" do
     @net_http_res.should_receive(:to_hash).and_return('set-cookie' => ['session_id=ZJ/HQVH6YE+rVkTpn0zvTQ==; path=/'])
-    @response.cookies.should eq({ 'session_id' => 'ZJ%2FHQVH6YE+rVkTpn0zvTQ%3D%3D' })
+    @response.headers.should eq({:set_cookie => ['session_id=ZJ/HQVH6YE+rVkTpn0zvTQ==; path=/']})
+    @response.cookies.should eq({ 'session_id' => 'ZJ/HQVH6YE+rVkTpn0zvTQ==' })
   end
 
   it "doesn't escape cookies" do

spec/unit/raw_response_spec.rb

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

spec/unit/response_spec.rb

@@ -3,8 +3,9 @@
 describe RestClient::Response, :include_helpers do
   before do
     @net_http_res = double('net http response', :to_hash => {"Status" => ["200 OK"]}, :code => 200)
-    @request = double('http request', :user => nil, :password => nil)
-    @response = RestClient::Response.create('abc', @net_http_res, {})
+    @example_url = 'http://example.com'
+    @request = double('http request', :user => nil, :password => nil, :url => @example_url)
+    @response = RestClient::Response.create('abc', @net_http_res, {}, @request)
   end
 
   it "behaves like string" do
@@ -16,7 +17,7 @@
   end
 
   it "accepts nil strings and sets it to empty for the case of HEAD" do
-    RestClient::Response.create(nil, @net_http_res, {}).to_s.should eq ""
+    RestClient::Response.create(nil, @net_http_res, {}, @request).to_s.should eq ""
   end
 
   it "test headers and raw headers" do
@@ -26,16 +27,18 @@
 
   describe "cookie processing" do
     it "should correctly deal with one Set-Cookie header with one cookie inside" do
-      net_http_res = double('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.create('abc', net_http_res, {})
-      response.headers[:set_cookie].should eq ["main_page=main_page_no_rewrite; path=/; expires=Tue, 20-Jan-2015 15:03:14 GMT"]
+      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.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=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.create('abc', net_http_res, {})
-      response.headers[:set_cookie].should eq ["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"]
+      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.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",
         "remember_me" => "",
@@ -44,8 +47,8 @@
     end
 
     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=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.create('abc', net_http_res, {})
+      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.cookies.should eq({
         "main_page" => "main_page_no_rewrite",
         "remember_me" => "",
@@ -58,7 +61,7 @@
     it "should return itself for normal codes" do
       (200..206).each do |code|
         net_http_res = response_double(:code => '200')
-        response = RestClient::Response.create('abc', net_http_res, {})
+        response = RestClient::Response.create('abc', net_http_res, {}, @request)
         response.return! @request
       end
     end
@@ -67,7 +70,7 @@
       RestClient::Exceptions::EXCEPTIONS_MAP.each_key do |code|
         unless (200..207).include? code
           net_http_res = response_double(:code => code.to_i)
-          response = RestClient::Response.create('abc', net_http_res, {})
+          response = RestClient::Response.create('abc', net_http_res, {}, @request)
           lambda { response.return!}.should raise_error
         end
       end
@@ -90,32 +93,38 @@
     end
 
     it "follows a redirection and keep the cookies" do
+      stub_request(:get, 'http://some/resource').to_return(:body => '', :status => 301, :headers => {'Set-Cookie' => 'Foo=Bar', 'Location' => 'http://some/new_resource', })
+      stub_request(:get, 'http://some/new_resource').with(:headers => {'Cookie' => 'Foo=Bar'}).to_return(:body => 'Qux')
+      RestClient::Request.execute(:url => 'http://some/resource', :method => :get).body.should eq 'Qux'
+    end
+
+    it 'does not keep cookies across domains' do
       stub_request(:get, 'http://some/resource').to_return(:body => '', :status => 301, :headers => {'Set-Cookie' => 'Foo=Bar', 'Location' => 'http://new/resource', })
-      stub_request(:get, 'http://new/resource').with(:headers => {'Cookie' => 'Foo=Bar'}).to_return(:body => 'Qux')
+      stub_request(:get, 'http://new/resource').with(:headers => {'Cookie' => ''}).to_return(:body => 'Qux')
       RestClient::Request.execute(:url => 'http://some/resource', :method => :get).body.should eq 'Qux'
     end
 
     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})
+      response = RestClient::Response.create('abc', net_http_res, {:method => :post}, @request)
       lambda { response.return!(@request)}.should raise_error(RestClient::MovedPermanently)
     end
 
     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})
+      response = RestClient::Response.create('abc', net_http_res, {:method => :post}, @request)
       lambda { response.return!(@request)}.should raise_error(RestClient::Found)
     end
 
     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})
+      response = RestClient::Response.create('abc', net_http_res, {:method => :post}, @request)
       lambda { response.return!(@request)}.should raise_error(RestClient::TemporaryRedirect)
     end
 
     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})
+      response = RestClient::Response.create('abc', net_http_res, {:method => :put}, @request)
       lambda { response.return!(@request)}.should raise_error(RestClient::MovedPermanently)
     end