Merge pull request #13945 from rails/json_cookie_serializer_improvements

Cookies serializer improvements

3 files changed, 150 insertions, 25 deletions

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'
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
diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb
index 6101acdc25..ba7aaa338d 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"
@@ -359,9 +369,72 @@ class CookiesTest < ActionController::TestCase
     assert_equal 'Jamie', @controller.send(:cookies).permanent[:user_name]
   end
 
-  def test_signed_cookie
+  def test_signed_cookie_using_default_serializer
     get :set_signed_cookie
-    assert_equal 45, @controller.send(:cookies).signed[:user_id]
+    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_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)
+
+    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]
+
+    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
+    @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)
+    json_value = ActiveSupport::MessageVerifier.new(secret, serializer: JSON).generate(45)
+    @request.headers["Cookie"] = "user_id=#{json_value}"
+
+    get :get_signed_cookie
+
+    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
@@ -369,43 +442,87 @@ class CookiesTest < ActionController::TestCase
     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 CustomJsonSerializer
-    def self.load(value)
-      JSON.load(value) + " and loaded"
-    end
-
-    def self.dump(value)
-      JSON.dump(value + " was dumped")
-    end
-  end
-
-  def test_encrypted_cookie_using_serializer_object
-    @request.env["action_dispatch.session_serializer"] = CustomJsonSerializer
+  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
-    @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]
   end
 
+  def test_encrypted_cookie_using_custom_serializer
+    @request.env["action_dispatch.cookies_serializer"] = CustomSerializer
+    get :set_encrypted_cookie
+    assert_not_equal 'bar', cookies.encrypted[:foo]
+    assert_equal 'bar was dumped and loaded', cookies.encrypted[:foo]
+  end
+
+  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"]
+    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)
+
+    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 "bar", cookies[:foo]
+    assert_equal "bar", cookies.encrypted[:foo]
+
+    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
+    @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)
+    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 "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
     get :set_encrypted_cookie
     assert_nil @controller.send(:cookies).encrypted[:non_existant_attribute]
@@ -721,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"