Skip to content

Commit

Permalink
Fix using Rack::Session::Cookie with coder: Rack::Session::Cookie::Ba…
Browse files Browse the repository at this point in the history
…se64::{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 rack#1666
  • Loading branch information
jeremyevans committed May 28, 2020
1 parent c853819 commit ffab89c
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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))

Expand Down
7 changes: 6 additions & 1 deletion lib/rack/session/abstract/id.rb
Expand Up @@ -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
Expand Down
Expand Up @@ -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
Expand Down

0 comments on commit ffab89c

Please sign in to comment.