From ffab89c3ae50d252151a4b1a62f2f5aaaa712755 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Wed, 27 May 2020 19:41:59 -0700 Subject: [PATCH] Fix using Rack::Session::Cookie with coder: Rack::Session::Cookie::Base64::{JSON,Zip} This is an issue because SessionId doesn't round trip through JSON. However, it probably has the same security issue as before SessionId was introduced. It may be better to eliminate the session id completely for cookie sessions, since there is no reason cookie sessions need an id (an id is only needed for memcache/memory/database sessions). Fixes #1666 --- CHANGELOG.md | 1 + lib/rack/session/abstract/id.rb | 7 ++++++- ...sion_abstract_persisted_secure_secure_session_hash.rb | 9 +++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a115d7e1f..e91cafde4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ All notable changes to this project will be documented in this file. For info on ### Fixed +- Fix using Rack::Session::Cookie with coder: Rack::Session::Cookie::Base64::{JSON,Zip}. ([#1666](https://github.com/rack/rack/issues/1666), [@jeremyevans](https://github.com/jeremyevans)) - Avoid NoMethodError when accessing Rack::Session::Cookie without requiring delegate first. ([#1610](https://github.com/rack/rack/issues/1610), [@onigra](https://github.com/onigra)) - Handle cookies with values that end in '=' ([#1645](https://github.com/rack/rack/pull/1645), [@lukaso](https://github.com/lukaso)) diff --git a/lib/rack/session/abstract/id.rb b/lib/rack/session/abstract/id.rb index 638bd3b3b..bbc79668b 100644 --- a/lib/rack/session/abstract/id.rb +++ b/lib/rack/session/abstract/id.rb @@ -455,7 +455,12 @@ class SecureSessionHash < SessionHash def [](key) if key == "session_id" load_for_read! - id.public_id if id + case id + when SessionId + id.public_id + else + id + end else super end diff --git a/test/spec_session_abstract_persisted_secure_secure_session_hash.rb b/test/spec_session_abstract_persisted_secure_secure_session_hash.rb index 1a007eb4a..bbb8a900e 100644 --- a/test/spec_session_abstract_persisted_secure_secure_session_hash.rb +++ b/test/spec_session_abstract_persisted_secure_secure_session_hash.rb @@ -44,6 +44,15 @@ def store.load_session(req) @hash = Rack::Session::Abstract::PersistedSecure::SecureSessionHash.new(store, nil) assert_nil hash['session_id'] end + + it "returns value for non SessionId 'session_id' key" do + store = @store.new + def store.load_session(req) + ["id", {}] + end + @hash = Rack::Session::Abstract::PersistedSecure::SecureSessionHash.new(store, nil) + assert_equal "id", hash['session_id'] + end end describe "#fetch" do