Cookies serializer improvements

actionpack/CHANGELOG.md

@@ -1,3 +1,21 @@
+*   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*
+
 *   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.
@@ -16,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`.
 

actionpack/lib/action_dispatch.rb

@@ -84,8 +84,6 @@ module Session
     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

actionpack/lib/action_dispatch/middleware/cookies.rb

@@ -89,7 +89,7 @@ class Cookies
     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
@@ -181,7 +181,7 @@ def initialize(*args)
 
       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
@@ -212,7 +212,7 @@ def self.options_for_env(env) #:nodoc:
           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,28 +374,89 @@ def []=(name, options)
       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
+
+    # 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)
+        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)
+              Marshal.load(value).tap do |v|
+                self[name] = { value: v }
+              end
+            else
+              serializer.load(value)
+            end
+          end
+        end
+
+        def serializer
+          serializer = @options[:serializer] || :marshal
+          case serializer
+          when :marshal
+            Marshal
+          when :json, :hybrid
+            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: 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
@@ -419,13 +480,14 @@ class UpgradeLegacySignedCookieJar < SignedCookieJar #:nodoc:
 
       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
 
     class EncryptedCookieJar #:nodoc:
       include ChainedCookieJars
+      include SerializedCookieJars
 
       def initialize(parent_jar, key_generator, options = {})
         if ActiveSupport::LegacyKeyGenerator === key_generator
@@ -437,12 +499,12 @@ def initialize(parent_jar, key_generator, options = {})
         @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
 
@@ -452,7 +514,8 @@ def []=(name, options)
         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
@@ -464,18 +527,6 @@ def decrypt_and_verify(encrypted_message)
         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
@@ -487,7 +538,7 @@ class UpgradeLegacyEncryptedCookieJar < EncryptedCookieJar #:nodoc:
 
       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

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 <tt>flash["notice"]</tt> to
@@ -50,13 +52,14 @@ def initialize(flash)
       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 <tt>flash.now[:alert]=</tt>.
@@ -92,8 +95,8 @@ def to_session_value
       end
 
       def initialize(flashes = {}, discard = []) #:nodoc:
-        @discard = Set.new(discard)
-        @flashes = flashes
+        @discard = Set.new(stringify_array(discard))
+        @flashes = flashes.stringify_keys
         @now     = nil
       end
 
@@ -106,17 +109,18 @@ def initialize_copy(other)
       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
-        @flashes.update h
+        @discard.subtract stringify_array(h.keys)
+        @flashes.update h.stringify_keys
         self
       end
 
@@ -129,6 +133,7 @@ def key?(name)
       end
 
       def delete(key)
+        key = key.to_s
         @discard.delete key
         @flashes.delete key
         self
@@ -155,7 +160,7 @@ def each(&block)
 
       def replace(h) #:nodoc:
         @discard.clear
-        @flashes.replace h
+        @flashes.replace h.stringify_keys
         self
       end
 
@@ -186,6 +191,7 @@ def now
       #    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 +201,7 @@ def keep(k = nil)
       #     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 +238,12 @@ def notice=(message)
       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)

actionpack/test/controller/flash_hash_test.rb

@@ -67,6 +67,16 @@ def test_from_session_value
       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'