Make sure that Rails doesn't resent session_id cookie over and over again if...

Make sure that Rails doesn't resent session_id cookie over and over again if it's already there [#2485 state:resolved]

This apply to only Active Record store and Memcached store, as they both store only the session_id, which will be unchanged, in the cookie.
Signed-off-by: NJosé Valim <jose.valim@gmail.com>

actionpack/lib/action_controller/session/abstract_store.rb

@@ -139,21 +139,23 @@ def call(env)
             return response
           end
 
-          cookie = Rack::Utils.escape(@key) + '=' + Rack::Utils.escape(sid)
-          cookie << "; domain=#{options[:domain]}" if options[:domain]
-          cookie << "; path=#{options[:path]}" if options[:path]
-          if options[:expire_after]
-            expiry = Time.now + options[:expire_after]
-            cookie << "; expires=#{expiry.httpdate}"
-          end
-          cookie << "; Secure" if options[:secure]
-          cookie << "; HttpOnly" if options[:httponly]
-
-          headers = response[1]
-          unless headers[SET_COOKIE].blank?
-            headers[SET_COOKIE] << "\n#{cookie}"
-          else
-            headers[SET_COOKIE] = cookie
+          if (env["rack.request.cookie_hash"] && env["rack.request.cookie_hash"][@key] != sid) || options[:expire_after]
+            cookie = Rack::Utils.escape(@key) + '=' + Rack::Utils.escape(sid)
+            cookie << "; domain=#{options[:domain]}" if options[:domain]
+            cookie << "; path=#{options[:path]}" if options[:path]
+            if options[:expire_after]
+              expiry = Time.now + options[:expire_after]
+              cookie << "; expires=#{expiry.httpdate}"
+            end
+            cookie << "; Secure" if options[:secure]
+            cookie << "; HttpOnly" if options[:httponly]
+
+            headers = response[1]
+            unless headers[SET_COOKIE].blank?
+              headers[SET_COOKIE] << "\n#{cookie}"
+            else
+              headers[SET_COOKIE] = cookie
+            end
           end
         end
 

actionpack/test/activerecord/active_record_store_test.rb

@@ -107,6 +107,18 @@ def test_getting_session_id
     end
   end
 
+  def test_doesnt_write_session_cookie_if_session_id_is_already_exists
+    with_test_route_set do
+      get '/set_session_value'
+      assert_response :success
+      assert cookies['_session_id']
+
+      get '/get_session_value'
+      assert_response :success
+      assert_equal nil, headers['Set-Cookie'], "should not resend the cookie again if session_id cookie is already exists"
+    end
+  end
+
   def test_prevents_session_fixation
     with_test_route_set do
       get '/set_session_value'

actionpack/test/controller/session/mem_cache_store_test.rb

@@ -95,6 +95,18 @@ def test_getting_session_id
       end
     end
 
+    def test_doesnt_write_session_cookie_if_session_id_is_already_exists
+      with_test_route_set do
+        get '/set_session_value'
+        assert_response :success
+        assert cookies['_session_id']
+
+        get '/get_session_value'
+        assert_response :success
+        assert_equal nil, headers['Set-Cookie'], "should not resend the cookie again if session_id cookie is already exists"
+      end
+    end
+
     def test_prevents_session_fixation
       with_test_route_set do
         get '/get_session_value'