From 945055bc7940c7a39f80deef638a1e06455275ec Mon Sep 17 00:00:00 2001 From: Dmytro Bihniak Date: Tue, 20 May 2025 12:37:39 +0200 Subject: [PATCH 1/2] fix(cookies): fix compatibility with rack 3 Do not join cookies with new like if they weren't before --- lib/secure_headers/middleware.rb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/secure_headers/middleware.rb b/lib/secure_headers/middleware.rb index e1c07c5b..edda2806 100644 --- a/lib/secure_headers/middleware.rb +++ b/lib/secure_headers/middleware.rb @@ -20,14 +20,12 @@ def call(env) # inspired by https://github.com/tobmatth/rack-ssl-enforcer/blob/6c014/lib/rack/ssl-enforcer.rb#L183-L194 def flag_cookies!(headers, config) - if cookies = headers["Set-Cookie"] - # Support Rails 2.3 / Rack 1.1 arrays as headers - cookies = cookies.split("\n") unless cookies.is_a?(Array) + cookies = headers["Set-Cookie"] + return unless cookies - headers["Set-Cookie"] = cookies.map do |cookie| - SecureHeaders::Cookie.new(cookie, config).to_s - end.join("\n") - end + cookies_array = cookies.is_a?(Array) ? cookies : cookies.split("\n") + secured_cookies = cookies_array.map { |cookie| SecureHeaders::Cookie.new(cookie, config).to_s } + headers["Set-Cookie"] = cookies.is_a?(Array) ? secured_cookies : secured_cookies.join("\n") end # disable Secure cookies for non-https requests From 1031d5132585fd9abefc0fef15812df92043c7d4 Mon Sep 17 00:00:00 2001 From: Dmytro Bihniak Date: Tue, 20 May 2025 13:18:29 +0200 Subject: [PATCH 2/2] fix(middleware): ensure headers are wrapped with `Rack::Headers` Add `Rack::Headers` wrapping to middleware to prevent header manipulation issues. Added a test to verify cookies remain as an array when flagged if already in array format. --- lib/secure_headers/middleware.rb | 1 + spec/lib/secure_headers/middleware_spec.rb | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/secure_headers/middleware.rb b/lib/secure_headers/middleware.rb index edda2806..c579f69a 100644 --- a/lib/secure_headers/middleware.rb +++ b/lib/secure_headers/middleware.rb @@ -9,6 +9,7 @@ def initialize(app) def call(env) req = Rack::Request.new(env) status, headers, response = @app.call(env) + headers = Rack::Headers[headers] config = SecureHeaders.config_for(req) flag_cookies!(headers, override_secure(env, config.cookies)) unless config.cookies == OPT_OUT diff --git a/spec/lib/secure_headers/middleware_spec.rb b/spec/lib/secure_headers/middleware_spec.rb index b3925f85..1dedbc37 100644 --- a/spec/lib/secure_headers/middleware_spec.rb +++ b/spec/lib/secure_headers/middleware_spec.rb @@ -83,9 +83,9 @@ module SecureHeaders end it "flags cookies with a combination of SameSite configurations" do - cookie_middleware = Middleware.new(lambda { |env| [200, env.merge("Set-Cookie" => ["_session=foobar", "_guest=true"]), "app"] }) + cookie_middleware = Middleware.new(lambda { |env| [200, env.merge("Set-Cookie" => "_session=foobar\n_guest=true"), "app"] }) - Configuration.default { |config| config.cookies = { samesite: { lax: { except: ["_session"] }, strict: { only: ["_session"] } }, httponly: OPT_OUT, secure: OPT_OUT} } + Configuration.default { |config| config.cookies = { samesite: { lax: { except: ["_session"] }, strict: { only: ["_session"] } }, httponly: OPT_OUT, secure: OPT_OUT } } request = Rack::Request.new("HTTPS" => "on") _, env = cookie_middleware.call request.env @@ -93,6 +93,16 @@ module SecureHeaders expect(env["Set-Cookie"]).to match("_guest=true; SameSite=Lax") end + it "keeps cookies as array after flagging if they are already an array" do + cookie_middleware = Middleware.new(lambda { |env| [200, env.merge("Set-Cookie" => ["_session=foobar", "_guest=true"]), "app"] }) + + Configuration.default { |config| config.cookies = { samesite: { lax: { except: ["_session"] }, strict: { only: ["_session"] } }, httponly: OPT_OUT, secure: OPT_OUT } } + request = Rack::Request.new("HTTPS" => "on") + _, env = cookie_middleware.call request.env + + expect(env["Set-Cookie"]).to match_array(["_session=foobar; SameSite=Strict", "_guest=true; SameSite=Lax"]) + end + it "disables secure cookies for non-https requests" do Configuration.default { |config| config.cookies = { secure: true, httponly: OPT_OUT, samesite: OPT_OUT } }