From 4069874a9b3a52a89d2e6949b003b2ef9f9296b1 Mon Sep 17 00:00:00 2001 From: Hieu Nguyen Date: Wed, 6 Aug 2025 16:02:21 +0800 Subject: [PATCH 1/2] =?UTF-8?q?Do=20not=20return=20new=20cookies=20when=20?= =?UTF-8?q?session=20isn=E2=80=99t=20changed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/rack/session/abstract/id.rb | 1 + lib/rack/session/constants.rb | 1 + lib/rack/session/cookie.rb | 16 +++++++++++++++- test/spec_session_cookie.rb | 18 ++++++++++++++---- 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/lib/rack/session/abstract/id.rb b/lib/rack/session/abstract/id.rb index 52446a5..a789912 100644 --- a/lib/rack/session/abstract/id.rb +++ b/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] diff --git a/lib/rack/session/constants.rb b/lib/rack/session/constants.rb index d40d9a2..d0fce4c 100644 --- a/lib/rack/session/constants.rb +++ b/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 diff --git a/lib/rack/session/cookie.rb b/lib/rack/session/cookie.rb index c7dabd1..2c77dd2 100644 --- a/lib/rack/session/cookie.rb +++ b/lib/rack/session/cookie.rb @@ -245,7 +245,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 @@ -255,6 +261,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 diff --git a/test/spec_session_cookie.rb b/test/spec_session_cookie.rb index 1ae7bbd..bb0ed32 100644 --- a/test/spec_session_cookie.rb +++ b/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 From 4fd4aaac6ff714f4f578b97c9df9a8ac1e3e4f2c Mon Sep 17 00:00:00 2001 From: Hieu Nguyen Date: Tue, 7 Oct 2025 08:51:47 +0800 Subject: [PATCH 2/2] Use deep duplication and improve performance of set_cookie --- lib/rack/session/abstract/id.rb | 4 ++++ lib/rack/session/cookie.rb | 11 +++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/rack/session/abstract/id.rb b/lib/rack/session/abstract/id.rb index a789912..d3bae8c 100644 --- a/lib/rack/session/abstract/id.rb +++ b/lib/rack/session/abstract/id.rb @@ -37,6 +37,10 @@ def private_id def empty?; false; end def inspect; public_id.inspect; end + def ==(other) + other.public_id == public_id + end + private def hash_sid(sid) diff --git a/lib/rack/session/cookie.rb b/lib/rack/session/cookie.rb index 2c77dd2..8b7bf76 100644 --- a/lib/rack/session/cookie.rb +++ b/lib/rack/session/cookie.rb @@ -246,7 +246,7 @@ def unpacked_cookie_data(request) end if session_data - request.set_header(RACK_SESSION_WAS, session_data.dup) + request.set_header(RACK_SESSION_WAS, deep_dup(session_data)) request.set_header(k, session_data) else request.set_header(RACK_SESSION_WAS, nil) @@ -262,9 +262,8 @@ def persistent_session_id!(data, sid = nil) 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] + return if !request.get_header(RACK_SESSION_OPTIONS).fetch(:renew, false) && + !cookie[:expires] && request.session.to_h == request.get_header(RACK_SESSION_WAS) response.set_cookie(@key, cookie) end @@ -324,6 +323,10 @@ def secure?(options) @legacy_hmac || (options[:coder] && options[:let_coder_handle_secure_encoding]) end + + def deep_dup(data) + ::Marshal.load(::Marshal.dump(data)) + end end end end