From 2ae8d3079b96d343f8cea8513929d656013f880e Mon Sep 17 00:00:00 2001
From: Pratik Naik <pratiknaik@gmail.com>
Date: Wed, 28 Jan 2009 05:05:07 +0000
Subject: Session cookie header should always be set if :expire_after option is
 specified

---
 .../action_controller/session/abstract_store.rb    |  6 ++--
 .../lib/action_controller/session/cookie_store.rb  |  6 ++--
 .../test/controller/session/cookie_store_test.rb   | 36 +++++++++++++++++++---
 3 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/actionpack/lib/action_controller/session/abstract_store.rb b/actionpack/lib/action_controller/session/abstract_store.rb
index bf09fd33c5..9434c2e05e 100644
--- a/actionpack/lib/action_controller/session/abstract_store.rb
+++ b/actionpack/lib/action_controller/session/abstract_store.rb
@@ -102,8 +102,10 @@ module ActionController
         response = @app.call(env)
 
         session_data = env[ENV_SESSION_KEY]
-        if !session_data.is_a?(AbstractStore::SessionHash) || session_data.send(:loaded?)
-          options = env[ENV_SESSION_OPTIONS_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)
             sid = session_data.id
diff --git a/actionpack/lib/action_controller/session/cookie_store.rb b/actionpack/lib/action_controller/session/cookie_store.rb
index 6ad6369950..5a728d1877 100644
--- a/actionpack/lib/action_controller/session/cookie_store.rb
+++ b/actionpack/lib/action_controller/session/cookie_store.rb
@@ -93,12 +93,14 @@ module ActionController
         status, headers, body = @app.call(env)
 
         session_data = env[ENV_SESSION_KEY]
-        if !session_data.is_a?(AbstractStore::SessionHash) || session_data.send(:loaded?)
+        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?)
           session_data = marshal(session_data.to_hash)
 
           raise CookieOverflow if session_data.size > MAX
 
-          options = env[ENV_SESSION_OPTIONS_KEY]
           cookie = Hash.new
           cookie[:value] = session_data
           unless options[:expire_after].nil?
diff --git a/actionpack/test/controller/session/cookie_store_test.rb b/actionpack/test/controller/session/cookie_store_test.rb
index b6a38f47aa..95d2eb11c4 100644
--- a/actionpack/test/controller/session/cookie_store_test.rb
+++ b/actionpack/test/controller/session/cookie_store_test.rb
@@ -6,13 +6,11 @@ class CookieStoreTest < ActionController::IntegrationTest
   SessionSecret = 'b3c631c314c0bbca50c1b2843150fe33'
 
   DispatcherApp = ActionController::Dispatcher.new
-  CookieStoreApp = ActionController::Session::CookieStore.new(DispatcherApp,
-                     :key => SessionKey, :secret => SessionSecret)
+  CookieStoreApp = ActionController::Session::CookieStore.new(DispatcherApp, :key => SessionKey, :secret => SessionSecret)
 
   Verifier = ActiveSupport::MessageVerifier.new(SessionSecret, 'SHA1')
 
-  SignedBar = "BAh7BjoIZm9vIghiYXI%3D--" +
-    "fef868465920f415f2c0652d6910d3af288a0367"
+  SignedBar = "BAh7BjoIZm9vIghiYXI%3D--fef868465920f415f2c0652d6910d3af288a0367"
 
   class TestController < ActionController::Base
     def no_session_access
@@ -177,6 +175,36 @@ class CookieStoreTest < ActionController::IntegrationTest
     end
   end
 
+  def test_session_store_with_expire_after
+    app = ActionController::Session::CookieStore.new(DispatcherApp, :key => SessionKey, :secret => SessionSecret, :expire_after => 5.hours)
+    @integration_session = open_session(app)
+
+    with_test_route_set do
+      # First request accesses the session
+      time = Time.local(2008, 4, 24)
+      Time.stubs(:now).returns(time)
+      expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d-%b-%Y %H:%M:%S GMT")
+
+      cookies[SessionKey] = SignedBar
+
+      get '/set_session_value'
+      assert_response :success
+
+      cookie_body = response.body
+      assert_equal ["_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; httponly"], headers['Set-Cookie']
+
+      # Second request does not access the session
+      time = Time.local(2008, 4, 25)
+      Time.stubs(:now).returns(time)
+      expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d-%b-%Y %H:%M:%S GMT")
+
+      get '/no_session_access'
+      assert_response :success
+
+      assert_equal ["_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; httponly"], headers['Set-Cookie']
+    end
+  end
+
   private
     def with_test_route_set
       with_routing do |set|
-- 
cgit v1.2.3