From 312ee8637366572425c9b3147a6477583333cf34 Mon Sep 17 00:00:00 2001 From: brainopia Date: Mon, 30 Apr 2012 15:42:25 +0400 Subject: [PATCH 1/4] Use more appropriate one-liner for class declaration --- actionpack/lib/action_dispatch/middleware/cookies.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index e1016977ab..4b91515df4 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -82,7 +82,7 @@ class Cookies TOKEN_KEY = "action_dispatch.secret_token".freeze # Raised when storing more than 4K of session data. - class CookieOverflow < StandardError; end + CookieOverflow = Class.new StandardError class CookieJar #:nodoc: include Enumerable From dbe5162496623c919457da1db7b5843f3dbe733f Mon Sep 17 00:00:00 2001 From: brainopia Date: Mon, 30 Apr 2012 15:50:54 +0400 Subject: [PATCH 2/4] Simplify matching with array of possible domains --- actionpack/lib/action_dispatch/middleware/cookies.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 4b91515df4..9b9227b659 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -154,7 +154,7 @@ def handle_options(options) #:nodoc: end elsif options[:domain].is_a? Array # if host matches one of the supplied domains without a dot in front of it - options[:domain] = options[:domain].find {|domain| @host.include? domain[/^\.?(.*)$/, 1] } + options[:domain] = options[:domain].find {|domain| @host.include? domain.sub(/^\./, '') } end end From ff2667d21a2c183d031acce44d95d06a8c99c035 Mon Sep 17 00:00:00 2001 From: brainopia Date: Mon, 30 Apr 2012 16:32:53 +0400 Subject: [PATCH 3/4] Dont set cookie header for deletion of unexisting data --- .../lib/action_dispatch/middleware/cookies.rb | 3 +- actionpack/test/dispatch/cookies_test.rb | 32 ++++++++++++++----- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 9b9227b659..ecb336bb4e 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -182,8 +182,9 @@ def []=(key, options) # and setting its expiration date into the past. Like []=, you can pass in # an options hash to delete cookies with extra data such as a :path. def delete(key, options = {}) - options.symbolize_keys! + return unless @cookies.has_key? key.to_s + options.symbolize_keys! handle_options(options) value = @cookies.delete(key.to_s) diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 3e48d97e67..3c1cc5fb21 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -38,6 +38,8 @@ def logout head :ok end + alias delete_cookie logout + def delete_cookie_with_path cookies.delete("user_name", :path => '/beaten') head :ok @@ -235,23 +237,33 @@ def test_setting_test_cookie end def test_expiring_cookie + request.cookies[:user_name] = 'Joe' get :logout assert_cookie_header "user_name=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT" assert_equal({"user_name" => nil}, @response.cookies) end def test_delete_cookie_with_path + request.cookies[:user_name] = 'Joe' get :delete_cookie_with_path assert_cookie_header "user_name=; path=/beaten; expires=Thu, 01-Jan-1970 00:00:00 GMT" end + def test_delete_unexisting_cookie + request.cookies.clear + get :delete_cookie + assert @response.cookies.empty? + end + def test_deleted_cookie_predicate + cookies[:user_name] = 'Joe' cookies.delete("user_name") assert cookies.deleted?("user_name") assert_equal false, cookies.deleted?("another") end def test_deleted_cookie_predicate_with_mismatching_options + cookies[:user_name] = 'Joe' cookies.delete("user_name", :path => "/path") assert_equal false, cookies.deleted?("user_name", :path => "/different") end @@ -284,6 +296,7 @@ def test_permanent_signed_cookie end def test_delete_and_set_cookie + request.cookies[:user_name] = 'Joe' get :delete_and_set_cookie assert_cookie_header "user_name=david; path=/; expires=Mon, 10-Oct-2005 05:00:00 GMT" assert_equal({"user_name" => "david"}, @response.cookies) @@ -387,6 +400,7 @@ def test_cookie_with_all_domain_option_using_ipv6_address end def test_deleting_cookie_with_all_domain_option + request.cookies[:user_name] = 'Joe' get :delete_cookie_with_domain assert_response :success assert_cookie_header "user_name=; domain=.nextangle.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT" @@ -413,6 +427,7 @@ def test_cookie_with_all_domain_option_using_host_with_port_and_tld_length end def test_deleting_cookie_with_all_domain_option_and_tld_length + request.cookies[:user_name] = 'Joe' get :delete_cookie_with_domain_and_tld assert_response :success assert_cookie_header "user_name=; domain=.nextangle.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT" @@ -441,6 +456,7 @@ def test_cookie_with_several_preset_domains_using_shared_domain def test_deletings_cookie_with_several_preset_domains_using_one_of_these_domains @request.host = "example2.com" + request.cookies[:user_name] = 'Joe' get :delete_cookie_with_domains assert_response :success assert_cookie_header "user_name=; domain=example2.com; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT" @@ -448,19 +464,19 @@ def test_deletings_cookie_with_several_preset_domains_using_one_of_these_domains def test_deletings_cookie_with_several_preset_domains_using_other_domain @request.host = "other-domain.com" + request.cookies[:user_name] = 'Joe' get :delete_cookie_with_domains assert_response :success assert_cookie_header "user_name=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT" end - def test_cookies_hash_is_indifferent_access - get :symbol_key - assert_equal "david", cookies[:user_name] - assert_equal "david", cookies['user_name'] - get :string_key - assert_equal "dhh", cookies[:user_name] - assert_equal "dhh", cookies['user_name'] + get :symbol_key + assert_equal "david", cookies[:user_name] + assert_equal "david", cookies['user_name'] + get :string_key + assert_equal "dhh", cookies[:user_name] + assert_equal "dhh", cookies['user_name'] end @@ -575,4 +591,4 @@ def assert_not_cookie_header(expected) assert_not_equal expected.split("\n"), header end end -end \ No newline at end of file +end From 2d18dd34719b1f9a3d2e3a516ccb83e7067dcd91 Mon Sep 17 00:00:00 2001 From: brainopia Date: Mon, 30 Apr 2012 16:55:06 +0400 Subject: [PATCH 4/4] Dont stream back cookie value if it was set to the same value --- actionpack/lib/action_dispatch/middleware/cookies.rb | 10 ++++++---- actionpack/test/dispatch/cookies_test.rb | 12 ++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index ecb336bb4e..c66bcf2cc9 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -169,12 +169,14 @@ def []=(key, options) options = { :value => value } end - @cookies[key.to_s] = value - handle_options(options) - @set_cookies[key.to_s] = options - @delete_cookies.delete(key.to_s) + if @cookies[key.to_s] != value or options[:expires] + @cookies[key.to_s] = value + @set_cookies[key.to_s] = options + @delete_cookies.delete(key.to_s) + end + value end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 3c1cc5fb21..2467654a70 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -181,6 +181,18 @@ def test_setting_cookie assert_equal({"user_name" => "david"}, @response.cookies) end + def test_setting_the_same_value_to_cookie + request.cookies[:user_name] = 'david' + get :authenticate + assert response.cookies.empty? + end + + def test_setting_the_same_value_to_permanent_cookie + request.cookies[:user_name] = 'Jamie' + get :set_permanent_cookie + assert response.cookies, 'user_name' => 'Jamie' + end + def test_setting_with_escapable_characters get :set_with_with_escapable_characters assert_cookie_header "that+%26+guy=foo+%26+bar+%3D%3E+baz; path=/"