From b927d67decb9d4e5103b5991b7e26a4dab4eca92 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 4 Feb 2014 09:31:48 -0800 Subject: Renamed session_serializer option to cookies_serializer --- actionpack/lib/action_dispatch.rb | 2 - .../lib/action_dispatch/middleware/cookies.rb | 45 ++++++++++++++-------- .../middleware/session/json_serializer.rb | 13 ------- .../middleware/session/marshal_serializer.rb | 14 ------- actionpack/test/dispatch/cookies_test.rb | 12 +++--- 5 files changed, 36 insertions(+), 50 deletions(-) delete mode 100644 actionpack/lib/action_dispatch/middleware/session/json_serializer.rb delete mode 100644 actionpack/lib/action_dispatch/middleware/session/marshal_serializer.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index a56d827b1a..3dd2e2a45c 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -84,8 +84,6 @@ module ActionDispatch autoload :CookieStore, 'action_dispatch/middleware/session/cookie_store' autoload :MemCacheStore, 'action_dispatch/middleware/session/mem_cache_store' autoload :CacheStore, 'action_dispatch/middleware/session/cache_store' - autoload :JsonSerializer, 'action_dispatch/middleware/session/json_serializer' - autoload :MarshalSerializer, 'action_dispatch/middleware/session/marshal_serializer' end mattr_accessor :test_app diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 531654895b..7e8a395d93 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -89,7 +89,7 @@ module ActionDispatch ENCRYPTED_SIGNED_COOKIE_SALT = "action_dispatch.encrypted_signed_cookie_salt".freeze SECRET_TOKEN = "action_dispatch.secret_token".freeze SECRET_KEY_BASE = "action_dispatch.secret_key_base".freeze - SESSION_SERIALIZER = "action_dispatch.session_serializer".freeze + COOKIES_SERIALIZER = "action_dispatch.cookies_serializer".freeze # Cookies can typically store 4096 bytes. MAX_COOKIE_SIZE = 4096 @@ -212,7 +212,7 @@ module ActionDispatch secret_token: env[SECRET_TOKEN], secret_key_base: env[SECRET_KEY_BASE], upgrade_legacy_signed_cookies: env[SECRET_TOKEN].present? && env[SECRET_KEY_BASE].present?, - session_serializer: env[SESSION_SERIALIZER] + serializer: env[COOKIES_SERIALIZER] } end @@ -374,14 +374,40 @@ module ActionDispatch end end + class JsonSerializer + def self.load(value) + JSON.parse(value, quirks_mode: true) + end + + def self.dump(value) + JSON.generate(value, quirks_mode: true) + end + end + + module SerializedCookieJars + protected + def serializer + serializer = @options[:serializer] || :marshal + case serializer + when :marshal + Marshal + when :json + JsonSerializer + else + serializer + end + end + end + class SignedCookieJar #:nodoc: include ChainedCookieJars + include SerializedCookieJars def initialize(parent_jar, key_generator, options = {}) @parent_jar = parent_jar @options = options secret = key_generator.generate_key(@options[:signed_cookie_salt]) - @verifier = ActiveSupport::MessageVerifier.new(secret) + @verifier = ActiveSupport::MessageVerifier.new(secret, serializer: serializer) end def [](name) @@ -426,6 +452,7 @@ module ActionDispatch class EncryptedCookieJar #:nodoc: include ChainedCookieJars + include SerializedCookieJars def initialize(parent_jar, key_generator, options = {}) if ActiveSupport::LegacyKeyGenerator === key_generator @@ -464,18 +491,6 @@ module ActionDispatch rescue ActiveSupport::MessageVerifier::InvalidSignature, ActiveSupport::MessageEncryptor::InvalidMessage nil end - - def serializer - serializer = @options[:session_serializer] || :marshal - case serializer - when :marshal - ActionDispatch::Session::MarshalSerializer - when :json - ActionDispatch::Session::JsonSerializer - else - serializer - end - end end # UpgradeLegacyEncryptedCookieJar is used by ActionDispatch::Session::CookieStore diff --git a/actionpack/lib/action_dispatch/middleware/session/json_serializer.rb b/actionpack/lib/action_dispatch/middleware/session/json_serializer.rb deleted file mode 100644 index d341853f7a..0000000000 --- a/actionpack/lib/action_dispatch/middleware/session/json_serializer.rb +++ /dev/null @@ -1,13 +0,0 @@ -module ActionDispatch - module Session - class JsonSerializer - def self.load(value) - JSON.parse(value, quirks_mode: true) - end - - def self.dump(value) - JSON.generate(value, quirks_mode: true) - end - end - end -end diff --git a/actionpack/lib/action_dispatch/middleware/session/marshal_serializer.rb b/actionpack/lib/action_dispatch/middleware/session/marshal_serializer.rb deleted file mode 100644 index 26622f682d..0000000000 --- a/actionpack/lib/action_dispatch/middleware/session/marshal_serializer.rb +++ /dev/null @@ -1,14 +0,0 @@ -module ActionDispatch - module Session - class MarshalSerializer - def self.load(value) - Marshal.load(value) - end - - def self.dump(value) - Marshal.dump(value) - end - end - end -end - diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 6101acdc25..25a2fe199c 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -379,28 +379,28 @@ class CookiesTest < ActionController::TestCase assert_equal 'bar', cookies.encrypted[:foo] end - class CustomJsonSerializer + class CustomSerializer def self.load(value) - JSON.load(value) + " and loaded" + value.to_s + " and loaded" end def self.dump(value) - JSON.dump(value + " was dumped") + value.to_s + " was dumped" end end def test_encrypted_cookie_using_serializer_object - @request.env["action_dispatch.session_serializer"] = CustomJsonSerializer + @request.env["action_dispatch.cookies_serializer"] = CustomSerializer get :set_encrypted_cookie assert_equal 'bar was dumped and loaded', cookies.encrypted[:foo] end def test_encrypted_cookie_using_json_serializer - @request.env["action_dispatch.session_serializer"] = :json + @request.env["action_dispatch.cookies_serializer"] = :json get :set_encrypted_cookie cookies = @controller.send :cookies assert_not_equal 'bar', cookies[:foo] - assert_raises TypeError do + assert_raises ::JSON::ParserError do cookies.signed[:foo] end assert_equal 'bar', cookies.encrypted[:foo] -- cgit v1.2.3 From 54641fa2e3d248bf89e1a9954e06d6bbefa7bafa Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Wed, 5 Feb 2014 03:15:11 -0800 Subject: Just very so slightly better test coverage --- actionpack/test/dispatch/cookies_test.rb | 72 +++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 15 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 25a2fe199c..8c52ed348d 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -11,6 +11,16 @@ require 'active_support/key_generator' require 'active_support/message_verifier' class CookiesTest < ActionController::TestCase + class CustomSerializer + def self.load(value) + value.to_s + " and loaded" + end + + def self.dump(value) + value.to_s + " was dumped" + end + end + class TestController < ActionController::Base def authenticate cookies["user_name"] = "david" @@ -364,35 +374,60 @@ class CookiesTest < ActionController::TestCase assert_equal 45, @controller.send(:cookies).signed[:user_id] end + def test_signed_cookie_using_default_serializer + get :set_signed_cookie + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.signed[:user_id] + end + + def test_signed_cookie_using_marshal_serializer + @request.env["action_dispatch.cookies_serializer"] = :marshal + get :set_signed_cookie + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.signed[:user_id] + end + + def test_signed_cookie_using_json_serializer + @request.env["action_dispatch.cookies_serializer"] = :json + get :set_signed_cookie + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.signed[:user_id] + end + + def test_signed_cookie_using_custom_serializer + @request.env["action_dispatch.cookies_serializer"] = CustomSerializer + get :set_signed_cookie + assert_not_equal 45, cookies[:user_id] + assert_equal '45 was dumped and loaded', cookies.signed[:user_id] + end + def test_accessing_nonexistant_signed_cookie_should_not_raise_an_invalid_signature get :set_signed_cookie assert_nil @controller.send(:cookies).signed[:non_existant_attribute] end - def test_encrypted_cookie + def test_encrypted_cookie_using_default_serializer get :set_encrypted_cookie cookies = @controller.send :cookies assert_not_equal 'bar', cookies[:foo] - assert_raises TypeError do + assert_raise TypeError do cookies.signed[:foo] end assert_equal 'bar', cookies.encrypted[:foo] end - class CustomSerializer - def self.load(value) - value.to_s + " and loaded" - end - - def self.dump(value) - value.to_s + " was dumped" - end - end - - def test_encrypted_cookie_using_serializer_object - @request.env["action_dispatch.cookies_serializer"] = CustomSerializer + def test_encrypted_cookie_using_marshal_serializer + @request.env["action_dispatch.cookies_serializer"] = :marshal get :set_encrypted_cookie - assert_equal 'bar was dumped and loaded', cookies.encrypted[:foo] + cookies = @controller.send :cookies + assert_not_equal 'bar', cookies[:foo] + assert_raises TypeError do + cookies.signed[:foo] + end + assert_equal 'bar', cookies.encrypted[:foo] end def test_encrypted_cookie_using_json_serializer @@ -406,6 +441,13 @@ class CookiesTest < ActionController::TestCase assert_equal 'bar', cookies.encrypted[:foo] end + def test_encrypted_cookie_using_custom_serializer + @request.env["action_dispatch.cookies_serializer"] = CustomSerializer + get :set_encrypted_cookie + assert_not_equal 45, cookies.encrypted[:foo] + assert_equal 'bar was dumped and loaded', cookies.encrypted[:foo] + end + def test_accessing_nonexistant_encrypted_cookie_should_not_raise_invalid_message get :set_encrypted_cookie assert_nil @controller.send(:cookies).encrypted[:non_existant_attribute] -- cgit v1.2.3 From fafe8ece9d406cfbb197cc424baaa15a5772fae5 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Wed, 5 Feb 2014 03:17:28 -0800 Subject: Added HybridSerializer to upgrade existing marshal cookies (wip: need tests) --- actionpack/lib/action_dispatch/middleware/cookies.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 7e8a395d93..fa94f9c9e4 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -384,6 +384,18 @@ module ActionDispatch end end + class HybridSerializer < JsonSerializer + MARSHAL_SIGNATURE = "\x04\x08".freeze + + def self.load(value) + if value.start_with?(MARSHAL_SIGNATURE) + Marshal.load(value) + else + super + end + end + end + module SerializedCookieJars protected def serializer @@ -393,6 +405,8 @@ module ActionDispatch Marshal when :json JsonSerializer + when :hybrid + HybridSerializer else serializer end -- cgit v1.2.3 From 25f68ac6a25b5b2ee2b30832ec052e5649d4f10c Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Wed, 5 Feb 2014 03:24:49 -0800 Subject: Removed an old test --- actionpack/test/dispatch/cookies_test.rb | 5 ----- 1 file changed, 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 8c52ed348d..6162cea6b7 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -369,11 +369,6 @@ class CookiesTest < ActionController::TestCase assert_equal 'Jamie', @controller.send(:cookies).permanent[:user_name] end - def test_signed_cookie - get :set_signed_cookie - assert_equal 45, @controller.send(:cookies).signed[:user_id] - end - def test_signed_cookie_using_default_serializer get :set_signed_cookie cookies = @controller.send :cookies -- cgit v1.2.3 From d4b7aa735a0044da6b751cb72ce5d4fd979476d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Sat, 8 Feb 2014 01:23:06 -0200 Subject: Tests for the HybridSerializer --- actionpack/test/dispatch/cookies_test.rb | 68 ++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) (limited to 'actionpack') diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 6162cea6b7..985abeb215 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -399,6 +399,38 @@ class CookiesTest < ActionController::TestCase assert_equal '45 was dumped and loaded', cookies.signed[:user_id] end + def test_signed_cookie_using_hybrid_serializer_can_read_from_marshal_dumped_value + @request.env["action_dispatch.cookies_serializer"] = :hybrid + + key_generator = @request.env["action_dispatch.key_generator"] + signed_cookie_salt = @request.env["action_dispatch.signed_cookie_salt"] + secret = key_generator.generate_key(signed_cookie_salt) + legacy_value = ActiveSupport::MessageVerifier.new(secret, serializer: Marshal).generate(45) + @request.headers["Cookie"] = "user_id=#{legacy_value}" + + get :get_signed_cookie + + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.signed[:user_id] + end + + def test_signed_cookie_using_hybrid_serializer_can_read_from_json_dumped_value + @request.env["action_dispatch.cookies_serializer"] = :hybrid + + key_generator = @request.env["action_dispatch.key_generator"] + signed_cookie_salt = @request.env["action_dispatch.signed_cookie_salt"] + secret = key_generator.generate_key(signed_cookie_salt) + legacy_value = ActiveSupport::MessageVerifier.new(secret, serializer: JSON).generate(45) + @request.headers["Cookie"] = "user_id=#{legacy_value}" + + get :get_signed_cookie + + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.signed[:user_id] + end + def test_accessing_nonexistant_signed_cookie_should_not_raise_an_invalid_signature get :set_signed_cookie assert_nil @controller.send(:cookies).signed[:non_existant_attribute] @@ -443,6 +475,42 @@ class CookiesTest < ActionController::TestCase assert_equal 'bar was dumped and loaded', cookies.encrypted[:foo] end + def test_encrypted_cookie_using_hybrid_serializer_can_read_from_marshal_dumped_value + @request.env["action_dispatch.cookies_serializer"] = :hybrid + + key_generator = @request.env["action_dispatch.key_generator"] + encrypted_cookie_salt = @request.env["action_dispatch.encrypted_cookie_salt"] + encrypted_signed_cookie_salt = @request.env["action_dispatch.encrypted_signed_cookie_salt"] + secret = key_generator.generate_key(encrypted_cookie_salt) + sign_secret = key_generator.generate_key(encrypted_signed_cookie_salt) + legacy_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: Marshal).encrypt_and_sign(45) + @request.headers["Cookie"] = "user_id=#{legacy_value}" + + get :get_encrypted_cookie + + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.encrypted[:user_id] + end + + def test_encrypted_cookie_using_hybrid_serializer_can_read_from_marshal_json_value + @request.env["action_dispatch.cookies_serializer"] = :hybrid + + key_generator = @request.env["action_dispatch.key_generator"] + encrypted_cookie_salt = @request.env["action_dispatch.encrypted_cookie_salt"] + encrypted_signed_cookie_salt = @request.env["action_dispatch.encrypted_signed_cookie_salt"] + secret = key_generator.generate_key(encrypted_cookie_salt) + sign_secret = key_generator.generate_key(encrypted_signed_cookie_salt) + legacy_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: JSON).encrypt_and_sign(45) + @request.headers["Cookie"] = "user_id=#{legacy_value}" + + get :get_encrypted_cookie + + cookies = @controller.send :cookies + assert_not_equal 45, cookies[:user_id] + assert_equal 45, cookies.encrypted[:user_id] + end + def test_accessing_nonexistant_encrypted_cookie_should_not_raise_invalid_message get :set_encrypted_cookie assert_nil @controller.send(:cookies).encrypted[:non_existant_attribute] -- cgit v1.2.3 From 6de4888e0485d50c7d5e3b97a485e126b561ef6c Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Sat, 8 Feb 2014 10:16:43 -0800 Subject: Fixed minor typo in test code --- actionpack/test/dispatch/cookies_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 985abeb215..4da19933f0 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -493,7 +493,7 @@ class CookiesTest < ActionController::TestCase assert_equal 45, cookies.encrypted[:user_id] end - def test_encrypted_cookie_using_hybrid_serializer_can_read_from_marshal_json_value + def test_encrypted_cookie_using_hybrid_serializer_can_read_from_json_dumped_value @request.env["action_dispatch.cookies_serializer"] = :hybrid key_generator = @request.env["action_dispatch.key_generator"] -- cgit v1.2.3 From ba6861d032013416034cdb20012962b97460795f Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Sat, 8 Feb 2014 11:28:53 -0800 Subject: Changed the tests to ensure HybridSerializer actually migrates the cookies (currently failing) --- actionpack/test/dispatch/cookies_test.rb | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 4da19933f0..0c3d3d687c 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -399,20 +399,24 @@ class CookiesTest < ActionController::TestCase assert_equal '45 was dumped and loaded', cookies.signed[:user_id] end - def test_signed_cookie_using_hybrid_serializer_can_read_from_marshal_dumped_value + def test_signed_cookie_using_hybrid_serializer_can_migrate_marshal_dumped_value_to_json @request.env["action_dispatch.cookies_serializer"] = :hybrid key_generator = @request.env["action_dispatch.key_generator"] signed_cookie_salt = @request.env["action_dispatch.signed_cookie_salt"] secret = key_generator.generate_key(signed_cookie_salt) - legacy_value = ActiveSupport::MessageVerifier.new(secret, serializer: Marshal).generate(45) - @request.headers["Cookie"] = "user_id=#{legacy_value}" + + marshal_value = ActiveSupport::MessageVerifier.new(secret, serializer: Marshal).generate(45) + @request.headers["Cookie"] = "user_id=#{marshal_value}" get :get_signed_cookie cookies = @controller.send :cookies assert_not_equal 45, cookies[:user_id] assert_equal 45, cookies.signed[:user_id] + + json_value = ActiveSupport::MessageVerifier.new(secret, serializer: JSON).generate(45) + assert_equal @response.cookies['user_id'], json_value end def test_signed_cookie_using_hybrid_serializer_can_read_from_json_dumped_value @@ -421,8 +425,8 @@ class CookiesTest < ActionController::TestCase key_generator = @request.env["action_dispatch.key_generator"] signed_cookie_salt = @request.env["action_dispatch.signed_cookie_salt"] secret = key_generator.generate_key(signed_cookie_salt) - legacy_value = ActiveSupport::MessageVerifier.new(secret, serializer: JSON).generate(45) - @request.headers["Cookie"] = "user_id=#{legacy_value}" + json_value = ActiveSupport::MessageVerifier.new(secret, serializer: JSON).generate(45) + @request.headers["Cookie"] = "user_id=#{json_value}" get :get_signed_cookie @@ -475,7 +479,7 @@ class CookiesTest < ActionController::TestCase assert_equal 'bar was dumped and loaded', cookies.encrypted[:foo] end - def test_encrypted_cookie_using_hybrid_serializer_can_read_from_marshal_dumped_value + def test_encrypted_cookie_using_hybrid_serializer_can_migrate_marshal_dumped_value_to_json @request.env["action_dispatch.cookies_serializer"] = :hybrid key_generator = @request.env["action_dispatch.key_generator"] @@ -483,14 +487,18 @@ class CookiesTest < ActionController::TestCase encrypted_signed_cookie_salt = @request.env["action_dispatch.encrypted_signed_cookie_salt"] secret = key_generator.generate_key(encrypted_cookie_salt) sign_secret = key_generator.generate_key(encrypted_signed_cookie_salt) - legacy_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: Marshal).encrypt_and_sign(45) - @request.headers["Cookie"] = "user_id=#{legacy_value}" + + marshal_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: Marshal).encrypt_and_sign(45) + @request.headers["Cookie"] = "user_id=#{marshal_value}" get :get_encrypted_cookie cookies = @controller.send :cookies assert_not_equal 45, cookies[:user_id] assert_equal 45, cookies.encrypted[:user_id] + + json_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: JSON).encrypt_and_sign(45) + assert_equal @response.cookies["user_id"], json_value end def test_encrypted_cookie_using_hybrid_serializer_can_read_from_json_dumped_value @@ -501,8 +509,8 @@ class CookiesTest < ActionController::TestCase encrypted_signed_cookie_salt = @request.env["action_dispatch.encrypted_signed_cookie_salt"] secret = key_generator.generate_key(encrypted_cookie_salt) sign_secret = key_generator.generate_key(encrypted_signed_cookie_salt) - legacy_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: JSON).encrypt_and_sign(45) - @request.headers["Cookie"] = "user_id=#{legacy_value}" + json_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: JSON).encrypt_and_sign(45) + @request.headers["Cookie"] = "user_id=#{json_value}" get :get_encrypted_cookie -- cgit v1.2.3 From a6ce984b49519de7701aa13d04300c9d03cf8f72 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Sat, 8 Feb 2014 23:56:40 -0500 Subject: Convert FlashHash in a Hash with indifferent access --- actionpack/lib/action_dispatch/middleware/flash.rb | 19 +++++++++++++++---- actionpack/test/controller/flash_hash_test.rb | 10 ++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index 89003e7a5e..419bcb8a73 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -50,13 +50,14 @@ module ActionDispatch end def []=(k, v) + k = k.to_s @flash[k] = v @flash.discard(k) v end def [](k) - @flash[k] + @flash[k.to_s] end # Convenience accessor for flash.now[:alert]=. @@ -92,7 +93,7 @@ module ActionDispatch end def initialize(flashes = {}, discard = []) #:nodoc: - @discard = Set.new(discard) + @discard = Set.new(stringify_array(discard)) @flashes = flashes @now = nil end @@ -106,16 +107,17 @@ module ActionDispatch end def []=(k, v) + k = k.to_s @discard.delete k @flashes[k] = v end def [](k) - @flashes[k] + @flashes[k.to_s] end def update(h) #:nodoc: - @discard.subtract h.keys + @discard.subtract stringify_array(h.keys) @flashes.update h self end @@ -129,6 +131,7 @@ module ActionDispatch end def delete(key) + key = key.to_s @discard.delete key @flashes.delete key self @@ -186,6 +189,7 @@ module ActionDispatch # flash.keep # keeps the entire flash # flash.keep(:notice) # keeps only the "notice" entry, the rest of the flash is discarded def keep(k = nil) + k = k.to_s if k @discard.subtract Array(k || keys) k ? self[k] : self end @@ -195,6 +199,7 @@ module ActionDispatch # flash.discard # discard the entire flash at the end of the current action # flash.discard(:warning) # discard only the "warning" entry at the end of the current action def discard(k = nil) + k = k.to_s if k @discard.merge Array(k || keys) k ? self[k] : self end @@ -231,6 +236,12 @@ module ActionDispatch def now_is_loaded? @now end + + def stringify_array(array) + array.map do |item| + item.kind_of?(Symbol) ? item.to_s : item + end + end end def initialize(app) diff --git a/actionpack/test/controller/flash_hash_test.rb b/actionpack/test/controller/flash_hash_test.rb index 5490d9394b..50b36a0567 100644 --- a/actionpack/test/controller/flash_hash_test.rb +++ b/actionpack/test/controller/flash_hash_test.rb @@ -67,6 +67,16 @@ module ActionDispatch assert_equal({'flashes' => {'message' => 'Hello'}, 'discard' => %w[message]}, hash.to_session_value) end + def test_from_session_value_on_json_serializer + decrypted_data = "{ \"session_id\":\"d98bdf6d129618fc2548c354c161cfb5\", \"flash\":{\"discard\":[], \"flashes\":{\"message\":\"hey you\"}} }" + session = ActionDispatch::Cookies::JsonSerializer.load(decrypted_data) + hash = Flash::FlashHash.from_session_value(session['flash']) + + assert_equal({'discard' => %w[message], 'flashes' => { 'message' => 'hey you'}}, hash.to_session_value) + assert_equal "hey you", hash[:message] + assert_equal "hey you", hash["message"] + end + def test_empty? assert @hash.empty? @hash['zomg'] = 'bears' -- cgit v1.2.3 From a668beffd64106a1e1fedb71cc25eaaa11baf0c1 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Sun, 9 Feb 2014 00:35:10 -0500 Subject: Stringify the incoming hash in FlashHash Stringify the incoming as well to handle incoming symbol keys from marshalled sessions --- actionpack/lib/action_dispatch/middleware/flash.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index 419bcb8a73..1e45a38e5f 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/hash/keys' + module ActionDispatch class Request < Rack::Request # Access the contents of the flash. Use flash["notice"] to @@ -94,7 +96,7 @@ module ActionDispatch def initialize(flashes = {}, discard = []) #:nodoc: @discard = Set.new(stringify_array(discard)) - @flashes = flashes + @flashes = flashes.stringify_keys @now = nil end -- cgit v1.2.3 From ead947a3b2bc672b6064a6d0d33905d45299d22e Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Sun, 9 Feb 2014 01:12:11 -0800 Subject: Re-write legacy (marshal) cookies on read --- .../lib/action_dispatch/middleware/cookies.rb | 60 ++++++++++++++-------- actionpack/test/dispatch/cookies_test.rb | 32 ++++++------ 2 files changed, 57 insertions(+), 35 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index fa94f9c9e4..2af45d43bb 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -384,29 +384,48 @@ module ActionDispatch end end - class HybridSerializer < JsonSerializer - MARSHAL_SIGNATURE = "\x04\x08".freeze - + # Passing the NullSerializer downstream to the Message{Encryptor,Verifier} + # allows us to handle the (de)serialization step within the cookie jar, + # which gives us the opportunity to detect and migrate legacy cookies. + class NullSerializer def self.load(value) - if value.start_with?(MARSHAL_SIGNATURE) - Marshal.load(value) - else - super - end + value + end + + def self.dump(value) + value end end module SerializedCookieJars + MARSHAL_SIGNATURE = "\x04\x08".freeze + protected + def needs_migration?(value) + @options[:serializer] == :hybrid && value.start_with?(MARSHAL_SIGNATURE) + end + + def serialize(name, value) + serializer.dump(value) + end + + def deserialize(name, value) + if value + if needs_migration?(value) + self[name] = Marshal.load(value) + else + serializer.load(value) + end + end + end + def serializer serializer = @options[:serializer] || :marshal case serializer when :marshal Marshal - when :json + when :json, :hybrid JsonSerializer - when :hybrid - HybridSerializer else serializer end @@ -421,21 +440,21 @@ module ActionDispatch @parent_jar = parent_jar @options = options secret = key_generator.generate_key(@options[:signed_cookie_salt]) - @verifier = ActiveSupport::MessageVerifier.new(secret, serializer: serializer) + @verifier = ActiveSupport::MessageVerifier.new(secret, serializer: NullSerializer) end def [](name) if signed_message = @parent_jar[name] - verify(signed_message) + deserialize name, verify(signed_message) end end def []=(name, options) if options.is_a?(Hash) options.symbolize_keys! - options[:value] = @verifier.generate(options[:value]) + options[:value] = @verifier.generate(serialize(name, options[:value])) else - options = { :value => @verifier.generate(options) } + options = { :value => @verifier.generate(serialize(name, options)) } end raise CookieOverflow if options[:value].size > MAX_COOKIE_SIZE @@ -459,7 +478,7 @@ module ActionDispatch def [](name) if signed_message = @parent_jar[name] - verify(signed_message) || verify_and_upgrade_legacy_signed_message(name, signed_message) + deserialize(name, verify(signed_message)) || verify_and_upgrade_legacy_signed_message(name, signed_message) end end end @@ -478,12 +497,12 @@ module ActionDispatch @options = options secret = key_generator.generate_key(@options[:encrypted_cookie_salt]) sign_secret = key_generator.generate_key(@options[:encrypted_signed_cookie_salt]) - @encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: serializer) + @encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: NullSerializer) end def [](name) if encrypted_message = @parent_jar[name] - decrypt_and_verify(encrypted_message) + deserialize name, decrypt_and_verify(encrypted_message) end end @@ -493,7 +512,8 @@ module ActionDispatch else options = { :value => options } end - options[:value] = @encryptor.encrypt_and_sign(options[:value]) + + options[:value] = @encryptor.encrypt_and_sign(serialize(name, options[:value])) raise CookieOverflow if options[:value].size > MAX_COOKIE_SIZE @parent_jar[name] = options @@ -516,7 +536,7 @@ module ActionDispatch def [](name) if encrypted_or_signed_message = @parent_jar[name] - decrypt_and_verify(encrypted_or_signed_message) || verify_and_upgrade_legacy_signed_message(name, encrypted_or_signed_message) + deserialize(name, decrypt_and_verify(encrypted_or_signed_message)) || verify_and_upgrade_legacy_signed_message(name, encrypted_or_signed_message) end end end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 0c3d3d687c..ba7aaa338d 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -415,8 +415,8 @@ class CookiesTest < ActionController::TestCase assert_not_equal 45, cookies[:user_id] assert_equal 45, cookies.signed[:user_id] - json_value = ActiveSupport::MessageVerifier.new(secret, serializer: JSON).generate(45) - assert_equal @response.cookies['user_id'], json_value + verifier = ActiveSupport::MessageVerifier.new(secret, serializer: JSON) + assert_equal 45, verifier.verify(@response.cookies['user_id']) end def test_signed_cookie_using_hybrid_serializer_can_read_from_json_dumped_value @@ -433,6 +433,8 @@ class CookiesTest < ActionController::TestCase cookies = @controller.send :cookies assert_not_equal 45, cookies[:user_id] assert_equal 45, cookies.signed[:user_id] + + assert_nil @response.cookies["user_id"] end def test_accessing_nonexistant_signed_cookie_should_not_raise_an_invalid_signature @@ -475,7 +477,7 @@ class CookiesTest < ActionController::TestCase def test_encrypted_cookie_using_custom_serializer @request.env["action_dispatch.cookies_serializer"] = CustomSerializer get :set_encrypted_cookie - assert_not_equal 45, cookies.encrypted[:foo] + assert_not_equal 'bar', cookies.encrypted[:foo] assert_equal 'bar was dumped and loaded', cookies.encrypted[:foo] end @@ -488,17 +490,17 @@ class CookiesTest < ActionController::TestCase secret = key_generator.generate_key(encrypted_cookie_salt) sign_secret = key_generator.generate_key(encrypted_signed_cookie_salt) - marshal_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: Marshal).encrypt_and_sign(45) - @request.headers["Cookie"] = "user_id=#{marshal_value}" + marshal_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: Marshal).encrypt_and_sign("bar") + @request.headers["Cookie"] = "foo=#{marshal_value}" get :get_encrypted_cookie cookies = @controller.send :cookies - assert_not_equal 45, cookies[:user_id] - assert_equal 45, cookies.encrypted[:user_id] + assert_not_equal "bar", cookies[:foo] + assert_equal "bar", cookies.encrypted[:foo] - json_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: JSON).encrypt_and_sign(45) - assert_equal @response.cookies["user_id"], json_value + encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: JSON) + assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"]) end def test_encrypted_cookie_using_hybrid_serializer_can_read_from_json_dumped_value @@ -509,14 +511,16 @@ class CookiesTest < ActionController::TestCase encrypted_signed_cookie_salt = @request.env["action_dispatch.encrypted_signed_cookie_salt"] secret = key_generator.generate_key(encrypted_cookie_salt) sign_secret = key_generator.generate_key(encrypted_signed_cookie_salt) - json_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: JSON).encrypt_and_sign(45) - @request.headers["Cookie"] = "user_id=#{json_value}" + json_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: JSON).encrypt_and_sign("bar") + @request.headers["Cookie"] = "foo=#{json_value}" get :get_encrypted_cookie cookies = @controller.send :cookies - assert_not_equal 45, cookies[:user_id] - assert_equal 45, cookies.encrypted[:user_id] + assert_not_equal "bar", cookies[:foo] + assert_equal "bar", cookies.encrypted[:foo] + + assert_nil @response.cookies["foo"] end def test_accessing_nonexistant_encrypted_cookie_should_not_raise_invalid_message @@ -834,8 +838,6 @@ class CookiesTest < ActionController::TestCase assert_equal "dhh", cookies['user_name'] end - - def test_setting_request_cookies_is_indifferent_access cookies.clear cookies[:user_name] = "andrew" -- cgit v1.2.3 From b97e087321f33283d836c5b5964976c88230349a Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 11 Feb 2014 00:38:36 -0800 Subject: Fixed broken flash tests --- actionpack/lib/action_dispatch/middleware/flash.rb | 2 +- actionpack/test/controller/flash_test.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index 1e45a38e5f..b82f0f0825 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -120,7 +120,7 @@ module ActionDispatch def update(h) #:nodoc: @discard.subtract stringify_array(h.keys) - @flashes.update h + @flashes.update h.stringify_keys self end diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb index 9ceab91e42..25a4857eba 100644 --- a/actionpack/test/controller/flash_test.rb +++ b/actionpack/test/controller/flash_test.rb @@ -175,13 +175,13 @@ class FlashTest < ActionController::TestCase assert_equal(:foo_indeed, flash.discard(:foo)) # valid key passed assert_nil flash.discard(:unknown) # non existent key passed - assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.discard().to_hash) # nothing passed - assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.discard(nil).to_hash) # nothing passed + assert_equal({"foo" => :foo_indeed, "bar" => :bar_indeed}, flash.discard().to_hash) # nothing passed + assert_equal({"foo" => :foo_indeed, "bar" => :bar_indeed}, flash.discard(nil).to_hash) # nothing passed assert_equal(:foo_indeed, flash.keep(:foo)) # valid key passed assert_nil flash.keep(:unknown) # non existent key passed - assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.keep().to_hash) # nothing passed - assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.keep(nil).to_hash) # nothing passed + assert_equal({"foo" => :foo_indeed, "bar" => :bar_indeed}, flash.keep().to_hash) # nothing passed + assert_equal({"foo" => :foo_indeed, "bar" => :bar_indeed}, flash.keep(nil).to_hash) # nothing passed end def test_redirect_to_with_alert -- cgit v1.2.3 From 9fc7a6fcedd3adc820d9d481d9362313c356747b Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 11 Feb 2014 00:43:37 -0800 Subject: Missed FlashHash#replace --- actionpack/lib/action_dispatch/middleware/flash.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index b82f0f0825..4821d2a899 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -160,7 +160,7 @@ module ActionDispatch def replace(h) #:nodoc: @discard.clear - @flashes.replace h + @flashes.replace h.stringify_keys self end -- cgit v1.2.3 From ecf04f19b0754de8a2937acd9b03e42e94a570aa Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 11 Feb 2014 00:45:44 -0800 Subject: Added changelog entry for Flash changes [ci skip] --- actionpack/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 15541d58b5..d3177df1c3 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,7 @@ +* `FlashHash` now behaves like a `HashWithIndifferentAccess`. + + *Guillermo Iguaran* + * Set the `:shallow_path` scope option as each scope is generated rather than waiting until the `shallow` option is set. Also make the behavior of the `:shallow` resource option consistent with the behavior of the `shallow` method. -- cgit v1.2.3 From 0b86a6e950ed78822470793deddbec41c6d105f5 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 11 Feb 2014 02:13:09 -0800 Subject: Updated CHANGELOG, docs, guides and release notes. Also added a `cookies_serializer.rb` initializer to the app template. --- actionpack/CHANGELOG.md | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index d3177df1c3..342f670e78 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,17 @@ +* Add new config option `config.action_dispatch.cookies_serializer` for + specifying a serializer for the signed and encrypted cookie jars. + + The possible values are: + + * `:json` - serialize cookie values with `JSON` + * `:marshal` - serialize cookie values with `Marshal` + * `:hybrid` - transparently migrate existing `Marshal` cookie values to `JSON` + + For new apps `:json` option is added by default and `:marshal` is used + when no option is specified to maintain backwards compatibility. + + *Łukasz Sarnacki*, *Matt Aimonetti*, *Guillermo Iguaran*, *Godfrey Chan*, *Rafael Mendonça França* + * `FlashHash` now behaves like a `HashWithIndifferentAccess`. *Guillermo Iguaran* @@ -20,21 +34,6 @@ *Josh Jordan* -* Add `:serializer` option for `config.session_store :cookie_store`. This - changes default serializer when using `:cookie_store`. - - It is possible to pass: - - * `:json` which is a secure wrapper on JSON using `JSON.parse` and - `JSON.generate` methods with quirks mode; - * `:marshal` which is a wrapper on Marshal; - * serializer class with `load` and `dump` methods defined. - - For new apps `:json` option is added by default and :marshal is used - when no option is specified. - - *Łukasz Sarnacki*, *Matt Aimonetti* - * Ensure that `request.filtered_parameters` is reset between calls to `process` in `ActionController::TestCase`. -- cgit v1.2.3 From 7a3ef9842b3cbfe6dbe14700086824d163ce4d51 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 11 Feb 2014 02:55:48 -0800 Subject: Migrate hash-based cookie values correctly --- actionpack/lib/action_dispatch/middleware/cookies.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 2af45d43bb..31341dba63 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -181,7 +181,7 @@ module ActionDispatch def verify_and_upgrade_legacy_signed_message(name, signed_message) @legacy_verifier.verify(signed_message).tap do |value| - self[name] = value + self[name] = { value: value } end rescue ActiveSupport::MessageVerifier::InvalidSignature nil @@ -412,7 +412,9 @@ module ActionDispatch def deserialize(name, value) if value if needs_migration?(value) - self[name] = Marshal.load(value) + Marshal.load(value).tap do |value| + self[name] = { value: value } + end else serializer.load(value) end -- cgit v1.2.3 From dafc0eef4dd3393864e7b28bf74c8e7834083d60 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 11 Feb 2014 03:56:35 -0800 Subject: rm warning about variable shadowing --- actionpack/lib/action_dispatch/middleware/cookies.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 31341dba63..18e64704f6 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -412,8 +412,8 @@ module ActionDispatch def deserialize(name, value) if value if needs_migration?(value) - Marshal.load(value).tap do |value| - self[name] = { value: value } + Marshal.load(value).tap do |v| + self[name] = { value: v } end else serializer.load(value) -- cgit v1.2.3