Do not return new cookies when session isn’t changed

Author: hieuk09

lib/rack/session/abstract/id.rb

@@ -419,6 +419,7 @@ def cookie_value(data)
 
         # Sets the cookie back to the client with session id. We skip the cookie
         # setting if the value didn't change (sid is the same) or expires was given.
+        # Allow subclasses to set the cookie value in a different way.
 
         def set_cookie(request, response, cookie)
           if request.cookies[@key] != cookie[:value] || cookie[:expires]

lib/rack/session/constants.rb

@@ -7,6 +7,7 @@
 module Rack
   module Session
     RACK_SESSION                        = 'rack.session'
+    RACK_SESSION_WAS                    = 'rack.session.was'
     RACK_SESSION_OPTIONS                = 'rack.session.options'
     RACK_SESSION_UNPACKED_COOKIE_DATA   = 'rack.session.unpacked_cookie_data'
   end

lib/rack/session/cookie.rb

@@ -243,7 +243,13 @@ def unpacked_cookie_data(request)
             end
           end
 
-          request.set_header(k, session_data || {})
+          if session_data
+            request.set_header(RACK_SESSION_WAS, session_data.dup)
+            request.set_header(k, session_data)
+          else
+            request.set_header(RACK_SESSION_WAS, nil)
+            request.set_header(k, {})
+          end
         end
       end
 
@@ -253,6 +259,14 @@ def persistent_session_id!(data, sid = nil)
         data
       end
 
+      def set_cookie(request, response, cookie)
+        return if request.session.to_h == request.get_header(RACK_SESSION_WAS) &&
+                  !request.get_header(RACK_SESSION_OPTIONS).fetch(:renew, false) &&
+                  !cookie[:expires]
+
+        response.set_cookie(@key, cookie)
+      end
+
       class SessionId < DelegateClass(Session::SessionId)
         attr_reader :cookie_value
 

test/spec_session_cookie.rb

@@ -227,6 +227,15 @@ def decode(str); @calls << :decode; JSON.parse(str); end
     response["Set-Cookie"].must_match /SameSite=None/i
   end
 
+  it "does not create new cookies if cookies does not change" do
+    response = response_for(app: incrementor)
+    cookie   = response["Set-Cookie"]
+    cookie.must_include "rack.session="
+    response = response_for(app: only_session_id, cookie: cookie)
+    cookie   = response["Set-Cookie"]
+    cookie.must_be_nil
+  end
+
   it "allows using a lambda to specify same_site option, because some browsers require different settings" do
     # Details of why this might need to be set dynamically:
     # https://www.chromium.org/updates/same-site/incompatible-clients
@@ -253,14 +262,16 @@ def decode(str); @calls << :decode; JSON.parse(str); end
     response = response_for(app: incrementor)
     cookie   = response['Set-Cookie']
     response = response_for(app: only_session_id, cookie: cookie)
-    cookie   = response['Set-Cookie'] if response['Set-Cookie']
+    response['Set-Cookie'].must_be_nil
 
     response.body.wont_equal ""
     old_session_id = response.body
 
     response = response_for(app: renewer, cookie: cookie)
-    cookie   = response['Set-Cookie'] if response['Set-Cookie']
-    response = response_for(app: only_session_id, cookie: cookie)
+    new_cookie = response['Set-Cookie']
+    new_cookie.wont_equal cookie
+
+    response = response_for(app: only_session_id, cookie: new_cookie)
 
     response.body.wont_equal ""
     response.body.wont_equal old_session_id
@@ -276,7 +287,6 @@ def decode(str); @calls << :decode; JSON.parse(str); end
     response = response_for(app: destroy_session, cookie: response)
     response = response_for(app: only_session_id, cookie: response)
 
-    response.body.wont_equal ""
     response.body.wont_equal old_session_id
   end