From d69ebb849a78c07a4efc869789c4bc90e8741482 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@gmail.com>
Date: Thu, 24 Jun 2010 20:02:23 +0200
Subject: Avoid deserializing cookies too early, which causes session objects
 to not be available yet. Unfortunately, could not reproduce this in a test
 case.

---
 .../middleware/session/abstract_store.rb           | 33 +++++++++++-----------
 .../middleware/session/cookie_store.rb             |  7 ++++-
 .../test/dispatch/session/cookie_store_test.rb     |  6 ++--
 3 files changed, 25 insertions(+), 21 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb
index 7623a94234..2dff139b49 100644
--- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb
+++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb
@@ -18,29 +18,28 @@ module ActionDispatch
         def initialize(by, env, default_options)
           @by = by
           @env = env
-          merge!(default_options)
           @session_id_loaded = false
+          merge!(default_options)
         end
 
-        alias_method :get_without_session_load, :[]
-
         def [](key)
           if key == :id
-            load_session_id! unless has_session_id?
+            load_session_id! unless key?(:id) || has_session_id?
           end
           super(key)
         end
 
-        private
+      private
 
-          def has_session_id?
-            get_without_session_load(:id).present? || @session_id_loaded
-          end
+        def has_session_id?
+          @session_id_loaded
+        end
 
-          def load_session_id!
-            self[:id] = @by.send(:extract_session_id, @env)
-            @session_id_loaded = true
-          end
+        def load_session_id!
+          self[:id] = @by.send(:extract_session_id, @env)
+        ensure
+          @session_id_loaded = true
+        end
       end
 
       class SessionHash < Hash
@@ -99,7 +98,7 @@ module ActionDispatch
         def destroy
           clear
           @by.send(:destroy, @env) if @by
-          @env[ENV_SESSION_OPTIONS_KEY].delete(:id) if @env && @env[ENV_SESSION_OPTIONS_KEY]
+          @env[ENV_SESSION_OPTIONS_KEY][:id] = nil if @env && @env[ENV_SESSION_OPTIONS_KEY]
           @loaded = false
         end
 
@@ -164,8 +163,8 @@ module ActionDispatch
         session_data = env[ENV_SESSION_KEY]
         options = env[ENV_SESSION_OPTIONS_KEY]
 
-        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) || session_data.loaded? || options[:expire_after]
+          session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.loaded?
 
           sid = options[:id] || generate_sid
           session_data = session_data.to_hash
@@ -189,7 +188,7 @@ module ActionDispatch
 
         def prepare!(env)
           env[ENV_SESSION_KEY] = SessionHash.new(self, env)
-          env[ENV_SESSION_OPTIONS_KEY] = OptionsHash.new(self, env, @default_options.dup)
+          env[ENV_SESSION_OPTIONS_KEY] = OptionsHash.new(self, env, @default_options)
         end
 
         def generate_sid
@@ -207,7 +206,7 @@ module ActionDispatch
         end
 
         def extract_session_id(env)
-          request = Rack::Request.new(env)
+          request = ActionDispatch::Request.new(env)
           sid = request.cookies[@key]
           sid ||= request.params[@key] unless @cookie_only
           sid
diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb
index 7c5626735b..6c1567f470 100644
--- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb
+++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb
@@ -65,8 +65,9 @@ module ActionDispatch
           request = ActionDispatch::Request.new(env)
           if data = request.cookie_jar.signed[@key]
             data.stringify_keys!
+          else
+            {}
           end
-          data
         end
 
         def set_cookie(request, options)
@@ -77,6 +78,10 @@ module ActionDispatch
           persistent_session_id!(session_data, sid)
         end
 
+        def exists?(env)
+          ActionDispatch::Request.new(env).cookie_jar.key?(@key)
+        end
+
         def destroy(env)
           # session data is stored on client; nothing to do here
         end
diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb
index 787524ab7b..6aca22b456 100644
--- a/actionpack/test/dispatch/session/cookie_store_test.rb
+++ b/actionpack/test/dispatch/session/cookie_store_test.rb
@@ -205,21 +205,21 @@ class CookieStoreTest < ActionController::IntegrationTest
   def test_session_store_without_domain 
     with_test_route_set do
       get '/set_session_value'
-      assert_no_match /domain\=/, headers['Set-Cookie']
+      assert_no_match(/domain\=/, headers['Set-Cookie'])
     end
   end
 
   def test_session_store_with_nil_domain
     with_test_route_set(:domain => nil) do
       get '/set_session_value'
-      assert_no_match /domain\=/, headers['Set-Cookie']
+      assert_no_match(/domain\=/, headers['Set-Cookie'])
     end
   end
 
   def test_session_store_with_all_domains
     with_test_route_set(:domain => :all) do
       get '/set_session_value'
-      assert_match /domain=\.example\.com/, headers['Set-Cookie']
+      assert_match(/domain=\.example\.com/, headers['Set-Cookie'])
     end
   end
 
-- 
cgit v1.2.3