Ensure session id is set in session options hash [#1880 state:resolved]

rails · Feb 7, 2009 · 43c0938 · 43c0938
1 parent 78c6f48
commit 43c0938
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 18 deletions.
diff --git a/actionpack/lib/action_controller/session/abstract_store.rb b/actionpack/lib/action_controller/session/abstract_store.rb
@@ -17,16 +17,11 @@ def initialize(by, env)
           @loaded = false
         end
 
-        def id
-          load! unless @loaded
-          @id
-        end
-
         def session_id
           ActiveSupport::Deprecation.warn(
-            "ActionController::Session::AbstractStore::SessionHash#session_id" +
-            "has been deprecated.Please use #id instead.", caller)
-          id
+            "ActionController::Session::AbstractStore::SessionHash#session_id " +
+            "has been deprecated. Please use request.session_options[:id] instead.", caller)
+          @env[ENV_SESSION_OPTIONS_KEY][:id]
         end
 
         def [](key)
@@ -47,8 +42,8 @@ def to_hash
 
         def data
          ActiveSupport::Deprecation.warn(
-           "ActionController::Session::AbstractStore::SessionHash#data" +
-           "has been deprecated.Please use #to_hash instead.", caller)
+           "ActionController::Session::AbstractStore::SessionHash#data " +
+           "has been deprecated. Please use #to_hash instead.", caller)
           to_hash
         end
 
@@ -59,7 +54,8 @@ def loaded?
 
           def load!
             stale_session_check! do
-              @id, session = @by.send(:load_session, @env)
+              id, session = @by.send(:load_session, @env)
+              (@env[ENV_SESSION_OPTIONS_KEY] ||= {})[:id] = id
               replace(session)
               @loaded = true
             end
@@ -126,11 +122,7 @@ def call(env)
         if !session_data.is_a?(AbstractStore::SessionHash) || session_data.send(:loaded?) || options[:expire_after]
           session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.send(:loaded?)
 
-          if session_data.is_a?(AbstractStore::SessionHash)
-            sid = session_data.id
-          else
-            sid = generate_sid
-          end
+          sid = options[:id] || generate_sid
 
           unless set_session(env, sid, session_data.to_hash)
             return response

diff --git a/actionpack/lib/action_controller/session/cookie_store.rb b/actionpack/lib/action_controller/session/cookie_store.rb
@@ -88,7 +88,7 @@ def initialize(app, options = {})
 
       def call(env)
         env[ENV_SESSION_KEY] = AbstractStore::SessionHash.new(self, env)
-        env[ENV_SESSION_OPTIONS_KEY] = @default_options
+        env[ENV_SESSION_OPTIONS_KEY] = @default_options.dup
 
         status, headers, body = @app.call(env)
 

diff --git a/actionpack/test/controller/session/cookie_store_test.rb b/actionpack/test/controller/session/cookie_store_test.rb
@@ -30,6 +30,10 @@ def get_session_value
       render :text => "foo: #{session[:foo].inspect}"
     end
 
+    def get_session_id
+      render :text => "foo: #{session[:foo].inspect}; id: #{request.session_options[:id]}"
+    end
+
     def call_reset_session
       reset_session
       head :ok
@@ -106,6 +110,20 @@ def test_getting_session_value
    end
   end
 
+  def test_getting_session_id
+    with_test_route_set do
+      cookies[SessionKey] = SignedBar
+      get '/persistent_session_id'
+      assert_response :success
+      assert_equal response.body.size, 32
+      session_id = response.body
+
+      get '/get_session_id'
+      assert_response :success
+      assert_equal "foo: \"bar\"; id: #{session_id}", response.body
+    end
+  end
+
   def test_disregards_tampered_sessions
     with_test_route_set do
       cookies[SessionKey] = "BAh7BjoIZm9vIghiYXI%3D--123456780"

diff --git a/actionpack/test/controller/session/mem_cache_store_test.rb b/actionpack/test/controller/session/mem_cache_store_test.rb
@@ -16,6 +16,10 @@ def get_session_value
       render :text => "foo: #{session[:foo].inspect}"
     end
 
+    def get_session_id
+      render :text => "foo: #{session[:foo].inspect}; id: #{request.session_options[:id]}"
+    end
+
     def call_reset_session
       reset_session
       head :ok
@@ -50,7 +54,20 @@ def test_getting_nil_session_value
       with_test_route_set do
         get '/get_session_value'
         assert_response :success
-      assert_equal 'foo: nil', response.body
+        assert_equal 'foo: nil', response.body
+      end
+    end
+
+    def test_getting_session_id
+      with_test_route_set do
+        get '/set_session_value'
+        assert_response :success
+        assert cookies['_session_id']
+        session_id = cookies['_session_id']
+
+        get '/get_session_id'
+        assert_response :success
+        assert_equal "foo: \"bar\"; id: #{session_id}", response.body
       end
     end