Fix problem with unloaded ARStore sessions being loaded when they are garbage...

Fix problem with unloaded ARStore sessions being loaded when they are garbage collected, causing problems if there were AR objects in the session.


git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@3817 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

actionpack/lib/action_controller/session/active_record_store.rb

@@ -59,10 +59,8 @@ class Session < ActiveRecord::Base
         cattr_accessor :data_column_name
         self.data_column_name = 'data'
 
-        # Don't try to save if we haven't loaded the session.
-        before_update :loaded?
-        before_save   :marshal_data!
-        before_save   :raise_on_session_data_overflow!
+        before_save :marshal_data!
+        before_save :raise_on_session_data_overflow!
 
         class << self
           # Don't try to reload ARStore::Session in dev mode.
@@ -122,22 +120,24 @@ def data
           @data ||= self.class.unmarshal(read_attribute(@@data_column_name)) || {}
         end
 
+        # Has the session been loaded yet?
+        def loaded?
+          !! @data
+        end
+
         private
           attr_writer :data
 
           def marshal_data!
+            return false if !loaded?
             write_attribute(@@data_column_name, self.class.marshal(self.data))
           end
 
-          # Has the session been loaded yet?
-          def loaded?
-            !! @data
-          end
-
           # Ensures that the data about to be stored in the database is not
           # larger than the data storage column. Raises
           # ActionController::SessionOverflowError.
           def raise_on_session_data_overflow!
+            return false if !loaded?
             limit = self.class.data_column_size_limit
             if loaded? and limit and read_attribute(@@data_column_name).size > limit
               raise ActionController::SessionOverflowError
@@ -232,7 +232,12 @@ def data
           @data
         end
 
+        def loaded?
+          !! @data
+        end
+
         def save
+          return false if !loaded?
           marshaled_data = self.class.marshal(data)
 
           if @new_record

actionpack/test/activerecord/active_record_store_test.rb

@@ -62,6 +62,10 @@ def session_class
     CGI::Session::ActiveRecordStore::Session
   end
 
+  def session_id_column
+    "session_id"
+  end
+
   def setup
     session_class.create_table!
 
@@ -84,6 +88,20 @@ def test_model_attribute
     assert_equal({ 'foo' => 'bar' }, @new_session.model.data)
   end
 
+  def test_save_unloaded_session
+    c = session_class.connection
+    bogus_class = c.quote(Base64.encode64("\004\010o:\vBlammo\000"))
+    c.insert("INSERT INTO #{session_class.table_name} ('#{session_id_column}', 'data') VALUES ('abcdefghijklmnop', #{bogus_class})")
+
+    sess = session_class.find_by_session_id('abcdefghijklmnop')
+    assert_not_nil sess
+    assert !sess.loaded?
+
+    # because the session is not loaded, the save should be a no-op. If it
+    # isn't, this'll try and unmarshall the bogus class, and should get an error.
+    assert_nothing_raised { sess.save }
+  end
+
   def teardown
     session_class.drop_table!
   end
@@ -110,6 +128,10 @@ def test_protection_from_data_larger_than_column
 end
 
 class DeprecatedActiveRecordStoreTest < ActiveRecordStoreTest
+  def session_id_column
+    "sessid"
+  end
+
   def setup
     session_class.connection.execute 'create table old_sessions (id integer primary key, sessid text unique, data text)'
     session_class.table_name = 'old_sessions'