Regenerate sid when sbdy tries to fixate the session

Fixed broken test.

Thanks Stephen Richards for reporting.

2 files changed, 11 insertions, 12 deletions

diff --git a/actionpack/lib/action_dispatch/middleware/session/cache_store.rb b/actionpack/lib/action_dispatch/middleware/session/cache_store.rb
index 1db6194271..625050dc4b 100644
--- a/actionpack/lib/action_dispatch/middleware/session/cache_store.rb
+++ b/actionpack/lib/action_dispatch/middleware/session/cache_store.rb
@@ -16,9 +16,9 @@ module ActionDispatch
 
       # Get a session from the cache.
       def get_session(env, sid)
-        sid ||= generate_sid
-        session = @cache.read(cache_key(sid))
-        session ||= {}
+        unless sid and session = @cache.read(cache_key(sid))
+          sid, session = generate_sid, {}
+        end
         [sid, session]
       end
 
diff --git a/actionpack/test/dispatch/session/cache_store_test.rb b/actionpack/test/dispatch/session/cache_store_test.rb
index 73e056de23..0d88d1d29e 100644
--- a/actionpack/test/dispatch/session/cache_store_test.rb
+++ b/actionpack/test/dispatch/session/cache_store_test.rb
@@ -149,16 +149,15 @@ class CacheStoreTest < ActionDispatch::IntegrationTest
 
   def test_prevents_session_fixation
     with_test_route_set do
-      get '/get_session_value'
-      assert_response :success
-      assert_equal 'foo: nil', response.body
-      session_id = cookies['_session_id']
+      assert_equal nil, @cache.read('_session_id:0xhax')
 
-      reset!
+      cookies['_session_id'] = '0xhax'
+      get '/set_session_value'
 
-      get '/set_session_value', :_session_id => session_id
       assert_response :success
-      assert_not_equal session_id, cookies['_session_id']
+      assert_not_equal '0xhax', cookies['_session_id']
+      assert_equal nil, @cache.read('_session_id:0xhax')
+      assert_equal({'foo' => 'bar'}, @cache.read("_session_id:#{cookies['_session_id']}"))
     end
   end
 
@@ -170,8 +169,8 @@ class CacheStoreTest < ActionDispatch::IntegrationTest
         end
 
         @app = self.class.build_app(set) do |middleware|
-          cache = ActiveSupport::Cache::MemoryStore.new
-          middleware.use ActionDispatch::Session::CacheStore, :key => '_session_id', :cache => cache
+          @cache = ActiveSupport::Cache::MemoryStore.new
+          middleware.use ActionDispatch::Session::CacheStore, :key => '_session_id', :cache => @cache
           middleware.delete "ActionDispatch::ShowExceptions"
         end