Merge pull request #2041 from SAP-Oxygen/master-session-id-patch-with-test

Fixed session ID fixation for ActiveRecord::SessionStore (for master)

2 files changed, 37 insertions, 2 deletions

diff --git a/actionpack/test/activerecord/active_record_store_test.rb b/actionpack/test/activerecord/active_record_store_test.rb
index f0fb113860..768ac713ca 100644
--- a/actionpack/test/activerecord/active_record_store_test.rb
+++ b/actionpack/test/activerecord/active_record_store_test.rb
@@ -225,6 +225,36 @@ class ActiveRecordStoreTest < ActionDispatch::IntegrationTest
       assert_equal session_id, cookies['_session_id']
     end
   end
+  
+  def test_incoming_invalid_session_id_via_cookie_should_be_ignored
+    with_test_route_set do
+      open_session do |sess|
+        sess.cookies['_session_id'] = 'INVALID'
+        
+        sess.get '/set_session_value'
+        new_session_id = sess.cookies['_session_id']
+        assert_not_equal 'INVALID', new_session_id
+        
+        sess.get '/get_session_value'
+        new_session_id_2 = sess.cookies['_session_id']
+        assert_equal new_session_id, new_session_id_2
+      end
+    end
+  end
+
+  def test_incoming_invalid_session_id_via_parameter_should_be_ignored
+    with_test_route_set(:cookie_only => false) do
+      open_session do |sess|
+        sess.get '/set_session_value', :_session_id => 'INVALID'
+        new_session_id = sess.cookies['_session_id']
+        assert_not_equal 'INVALID', new_session_id
+        
+        sess.get '/get_session_value'
+        new_session_id_2 = sess.cookies['_session_id']
+        assert_equal new_session_id, new_session_id_2
+      end
+    end
+  end
 
   private
 
@@ -247,6 +277,7 @@ class ActiveRecordStoreTest < ActionDispatch::IntegrationTest
       session_class, ActiveRecord::SessionStore.session_class =
         ActiveRecord::SessionStore.session_class, "ActiveRecord::SessionStore::#{class_name.camelize}".constantize
       yield
+    ensure
       ActiveRecord::SessionStore.session_class = session_class
     end
 end
diff --git a/activerecord/lib/active_record/session_store.rb b/activerecord/lib/active_record/session_store.rb
index 929559c3ba..30a7ecd2a0 100644
--- a/activerecord/lib/active_record/session_store.rb
+++ b/activerecord/lib/active_record/session_store.rb
@@ -297,8 +297,12 @@ module ActiveRecord
     private
       def get_session(env, sid)
         Base.silence do
-          sid ||= generate_sid
-          session = find_session(sid)
+          unless sid and session = @@session_class.find_by_session_id(sid)
+            # If the sid was nil or if there is no pre-existing session under the sid,
+            # force the generation of a new sid and associate a new session associated with the new sid
+            sid = generate_sid
+            session = @@session_class.new(:session_id => sid, :data => {})
+          end
           env[SESSION_RECORD_KEY] = session
           [sid, session.data]
         end