From b086a5b33192f95b6a30d60fae45f048a00cac14 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 830a4e3..ad551f1 100644 --- a/lib/rack/session/cookie.rb +++ b/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 diff --git a/test/spec_session_cookie.rb b/test/spec_session_cookie.rb index 0e4094b..91e224b 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 445c2d1e58e5da4c92778f750affa20683f124e0 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 ad551f1..4f94334 100644 --- a/lib/rack/session/cookie.rb +++ b/lib/rack/session/cookie.rb @@ -244,7 +244,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) @@ -260,9 +260,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 @@ -322,6 +321,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