Fix reset_session with lazy cookie stores [#1601 state:resolved]

Signed-off-by: Joshua Peek <josh@joshpeek.com>

actionpack/lib/action_controller/session/abstract_store.rb

@@ -53,6 +53,10 @@ module ActionController
         end
 
         private
+          def loaded?
+            @loaded
+          end
+
           def load!
             @id, session = @by.send(:load_session, @env)
             replace(session)
@@ -91,19 +95,23 @@ module ActionController
 
       def call(env)
         session = SessionHash.new(self, env)
-        original_session = session.dup
 
         env[ENV_SESSION_KEY] = session
         env[ENV_SESSION_OPTIONS_KEY] = @default_options.dup
 
         response = @app.call(env)
 
-        session = env[ENV_SESSION_KEY]
-        unless session == original_session
+        session_data = env[ENV_SESSION_KEY]
+        if !session_data.is_a?(AbstractStore::SessionHash) || session_data.send(:loaded?)
           options = env[ENV_SESSION_OPTIONS_KEY]
-          sid = session.id
 
-          unless set_session(env, sid, session.to_hash)
+          if session_data.is_a?(AbstractStore::SessionHash)
+            sid = session_data.id
+          else
+            sid = generate_sid
+          end
+
+          unless set_session(env, sid, session_data.to_hash)
             return response
           end
 

actionpack/lib/action_controller/session/cookie_store.rb

@@ -89,16 +89,14 @@ module ActionController
       end
 
       def call(env)
-        session_data = AbstractStore::SessionHash.new(self, env)
-        original_value = session_data.dup
-
-        env[ENV_SESSION_KEY] = session_data
+        env[ENV_SESSION_KEY] = AbstractStore::SessionHash.new(self, env)
         env[ENV_SESSION_OPTIONS_KEY] = @default_options
 
         status, headers, body = @app.call(env)
 
-        unless env[ENV_SESSION_KEY] == original_value
-          session_data = marshal(env[ENV_SESSION_KEY].to_hash)
+        session_data = env[ENV_SESSION_KEY]
+        if !session_data.is_a?(AbstractStore::SessionHash) || session_data.send(:loaded?)
+          session_data = marshal(session_data.to_hash)
 
           raise CookieOverflow if session_data.size > MAX
 
@@ -153,7 +151,7 @@ module ActionController
 
         # Marshal a session hash into safe cookie data. Include an integrity hash.
         def marshal(session)
-          @verifier.generate( persistent_session_id!(session))
+          @verifier.generate(persistent_session_id!(session))
         end
 
         # Unmarshal cookie data to a hash and verify its integrity.

actionpack/test/controller/session/cookie_store_test.rb

@@ -32,6 +32,11 @@ class CookieStoreTest < ActionController::IntegrationTest
       render :text => "foo: #{session[:foo].inspect}"
     end
 
+    def call_reset_session
+      reset_session
+      head :ok
+    end
+
     def raise_data_overflow
       session[:foo] = 'bye!' * 1024
       head :ok
@@ -89,7 +94,7 @@ class CookieStoreTest < ActionController::IntegrationTest
     with_test_route_set do
       get '/set_session_value'
       assert_response :success
-      session_payload = Verifier.generate( Marshal.load(response.body) )
+      session_payload = Verifier.generate(Marshal.load(response.body))
       assert_equal ["_myapp_session=#{session_payload}; path=/"],
         headers['Set-Cookie']
    end
@@ -139,6 +144,25 @@ class CookieStoreTest < ActionController::IntegrationTest
     end
   end
 
+  def test_setting_session_value_after_session_reset
+    with_test_route_set do
+      get '/set_session_value'
+      assert_response :success
+      session_payload = Verifier.generate(Marshal.load(response.body))
+      assert_equal ["_myapp_session=#{session_payload}; path=/"],
+        headers['Set-Cookie']
+
+      get '/call_reset_session'
+      assert_response :success
+      assert_not_equal [], headers['Set-Cookie']
+      assert_not_equal session_payload, cookies[SessionKey]
+
+      get '/get_session_value'
+      assert_response :success
+      assert_equal 'foo: nil', response.body
+    end
+  end
+
   def test_persistent_session_id
     with_test_route_set do
       cookies[SessionKey] = SignedBar

actionpack/test/controller/session/mem_cache_store_test.rb

@@ -16,6 +16,11 @@ class MemCacheStoreTest < ActionController::IntegrationTest
       render :text => "foo: #{session[:foo].inspect}"
     end
 
+    def call_reset_session
+      reset_session
+      head :ok
+    end
+
     def rescue_action(e) raise end
   end
 
@@ -63,6 +68,22 @@ class MemCacheStoreTest < ActionController::IntegrationTest
         assert_equal nil, cookies['_session_id']
       end
     end
+
+    def test_setting_session_value_after_session_reset
+      with_test_route_set do
+        get '/set_session_value'
+        assert_response :success
+        assert cookies['_session_id']
+
+        get '/call_reset_session'
+        assert_response :success
+        assert_not_equal [], headers['Set-Cookie']
+
+        get '/get_session_value'
+        assert_response :success
+        assert_equal 'foo: nil', response.body
+      end
+    end
   rescue LoadError, RuntimeError
     $stderr.puts "Skipping MemCacheStoreTest tests. Start memcached and try again."
   end