diff --git a/lib/rack.rb b/lib/rack.rb index c8b6cc86a..48ac3e211 100644 --- a/lib/rack.rb +++ b/lib/rack.rb @@ -18,7 +18,7 @@ def self.version VERSION.join(".") end - RELEASE = "2.0.7" + RELEASE = "2.0.8" # Return the Rack release as a dotted string. def self.release diff --git a/lib/rack/session/abstract/id.rb b/lib/rack/session/abstract/id.rb index 1bb8d5d06..9d63ca2dd 100644 --- a/lib/rack/session/abstract/id.rb +++ b/lib/rack/session/abstract/id.rb @@ -6,11 +6,38 @@ require 'rack/request' require 'rack/response' require 'securerandom' +require 'digest/sha2' module Rack module Session + class SessionId + ID_VERSION = 2 + + attr_reader :public_id + + def initialize(public_id) + @public_id = public_id + end + + def private_id + "#{ID_VERSION}::#{hash_sid(public_id)}" + end + + alias :cookie_value :public_id + + def empty?; false; end + def to_s; raise; end + def inspect; public_id.inspect; end + + private + + def hash_sid(sid) + Digest::SHA256.hexdigest(sid) + end + end + module Abstract # SessionHash is responsible to lazily load the session from store. @@ -357,7 +384,7 @@ def commit_session(req, res) req.get_header(RACK_ERRORS).puts("Deferring cookie for #{session_id}") if $VERBOSE else cookie = Hash.new - cookie[:value] = data + cookie[:value] = cookie_value(data) cookie[:expires] = Time.now + options[:expire_after] if options[:expire_after] cookie[:expires] = Time.now + options[:max_age] if options[:max_age] set_cookie(req, res, cookie.merge!(options)) @@ -365,6 +392,10 @@ def commit_session(req, res) end public :commit_session + def cookie_value(data) + data + end + # 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. @@ -406,6 +437,40 @@ def delete_session(req, sid, options) end end + class PersistedSecure < Persisted + class SecureSessionHash < SessionHash + def [](key) + if key == "session_id" + load_for_read! + id.public_id + else + super + end + end + end + + def generate_sid(*) + public_id = super + + SessionId.new(public_id) + end + + def extract_session_id(*) + public_id = super + public_id && SessionId.new(public_id) + end + + private + + def session_class + SecureSessionHash + end + + def cookie_value(data) + data.cookie_value + end + end + class ID < Persisted def self.inherited(klass) k = klass.ancestors.find { |kl| kl.respond_to?(:superclass) && kl.superclass == ID } diff --git a/lib/rack/session/cookie.rb b/lib/rack/session/cookie.rb index 71bb96f4f..90ed5cf17 100644 --- a/lib/rack/session/cookie.rb +++ b/lib/rack/session/cookie.rb @@ -45,7 +45,7 @@ module Session # }) # - class Cookie < Abstract::Persisted + class Cookie < Abstract::PersistedSecure # Encode session cookies as Base64 class Base64 def encode(str) @@ -153,6 +153,15 @@ def persistent_session_id!(data, sid=nil) data end + class SessionId < DelegateClass(Session::SessionId) + attr_reader :cookie_value + + def initialize(session_id, cookie_value) + super(session_id) + @cookie_value = cookie_value + end + end + def write_session(req, session_id, session, options) session = session.merge("session_id" => session_id) session_data = coder.encode(session) @@ -165,7 +174,7 @@ def write_session(req, session_id, session, options) req.get_header(RACK_ERRORS).puts("Warning! Rack::Session::Cookie data size exceeds 4K.") nil else - session_data + SessionId.new(session_id, session_data) end end diff --git a/lib/rack/session/memcache.rb b/lib/rack/session/memcache.rb index 4cf5ea09e..1f9d3ec56 100644 --- a/lib/rack/session/memcache.rb +++ b/lib/rack/session/memcache.rb @@ -19,7 +19,7 @@ module Session # Note that memcache does drop data before it may be listed to expire. For # a full description of behaviour, please see memcache's documentation. - class Memcache < Abstract::ID + class Memcache < Abstract::PersistedSecure attr_reader :mutex, :pool DEFAULT_OPTIONS = Abstract::ID::DEFAULT_OPTIONS.merge \ @@ -42,15 +42,15 @@ def initialize(app, options={}) def generate_sid loop do sid = super - break sid unless @pool.get(sid, true) + break sid unless @pool.get(sid.private_id, true) end end - def get_session(env, sid) - with_lock(env) do - unless sid and session = @pool.get(sid) + def find_session(req, sid) + with_lock(req) do + unless sid and session = get_session_with_fallback(sid) sid, session = generate_sid, {} - unless /^STORED/ =~ @pool.add(sid, session) + unless /^STORED/ =~ @pool.add(sid.private_id, session) raise "Session collision on '#{sid.inspect}'" end end @@ -58,25 +58,26 @@ def get_session(env, sid) end end - def set_session(env, session_id, new_session, options) + def write_session(req, session_id, new_session, options) expiry = options[:expire_after] expiry = expiry.nil? ? 0 : expiry + 1 - with_lock(env) do - @pool.set session_id, new_session, expiry + with_lock(req) do + @pool.set session_id.private_id, new_session, expiry session_id end end - def destroy_session(env, session_id, options) - with_lock(env) do - @pool.delete(session_id) + def delete_session(req, session_id, options) + with_lock(req) do + @pool.delete(session_id.public_id) + @pool.delete(session_id.private_id) generate_sid unless options[:drop] end end - def with_lock(env) - @mutex.lock if env[RACK_MULTITHREAD] + def with_lock(req) + @mutex.lock if req.multithread? yield rescue MemCache::MemCacheError, Errno::ECONNREFUSED if $VERBOSE @@ -88,6 +89,11 @@ def with_lock(env) @mutex.unlock if @mutex.locked? end + private + + def get_session_with_fallback(sid) + @pool.get(sid.private_id) || @pool.get(sid.public_id) + end end end end diff --git a/lib/rack/session/pool.rb b/lib/rack/session/pool.rb index 4c9c25c7a..6c0f66811 100644 --- a/lib/rack/session/pool.rb +++ b/lib/rack/session/pool.rb @@ -24,7 +24,7 @@ module Session # ) # Rack::Handler::WEBrick.run sessioned - class Pool < Abstract::Persisted + class Pool < Abstract::PersistedSecure attr_reader :mutex, :pool DEFAULT_OPTIONS = Abstract::ID::DEFAULT_OPTIONS.merge :drop => false @@ -37,15 +37,15 @@ def initialize(app, options={}) def generate_sid loop do sid = super - break sid unless @pool.key? sid + break sid unless @pool.key? sid.private_id end end def find_session(req, sid) with_lock(req) do - unless sid and session = @pool[sid] + unless sid and session = get_session_with_fallback(sid) sid, session = generate_sid, {} - @pool.store sid, session + @pool.store sid.private_id, session end [sid, session] end @@ -53,14 +53,15 @@ def find_session(req, sid) def write_session(req, session_id, new_session, options) with_lock(req) do - @pool.store session_id, new_session + @pool.store session_id.private_id, new_session session_id end end def delete_session(req, session_id, options) with_lock(req) do - @pool.delete(session_id) + @pool.delete(session_id.public_id) + @pool.delete(session_id.private_id) generate_sid unless options[:drop] end end @@ -71,6 +72,12 @@ def with_lock(req) ensure @mutex.unlock if @mutex.locked? end + + private + + def get_session_with_fallback(sid) + @pool[sid.private_id] || @pool[sid.public_id] + end end end end diff --git a/test/spec_session_memcache.rb b/test/spec_session_memcache.rb index 93a03d120..824db6836 100644 --- a/test/spec_session_memcache.rb +++ b/test/spec_session_memcache.rb @@ -226,15 +226,52 @@ req = Rack::MockRequest.new(pool) res0 = req.get("/") - session_id = (cookie = res0["Set-Cookie"])[session_match, 1] - ses0 = pool.pool.get(session_id, true) + session_id = Rack::Session::SessionId.new (cookie = res0["Set-Cookie"])[session_match, 1] + ses0 = pool.pool.get(session_id.private_id, true) req.get("/", "HTTP_COOKIE" => cookie) - ses1 = pool.pool.get(session_id, true) + ses1 = pool.pool.get(session_id.private_id, true) ses1.wont_equal ses0 end + it "can read the session with the legacy id" do + pool = Rack::Session::Memcache.new(incrementor) + req = Rack::MockRequest.new(pool) + + res0 = req.get("/") + cookie = res0["Set-Cookie"] + session_id = Rack::Session::SessionId.new cookie[session_match, 1] + ses0 = pool.pool.get(session_id.private_id, true) + pool.pool.set(session_id.public_id, ses0, 0, true) + pool.pool.delete(session_id.private_id) + + res1 = req.get("/", "HTTP_COOKIE" => cookie) + res1["Set-Cookie"].must_be_nil + res1.body.must_equal '{"counter"=>2}' + pool.pool.get(session_id.private_id, true).wont_be_nil + end + + it "drops the session in the legacy id as well" do + pool = Rack::Session::Memcache.new(incrementor) + req = Rack::MockRequest.new(pool) + drop = Rack::Utils::Context.new(pool, drop_session) + dreq = Rack::MockRequest.new(drop) + + res0 = req.get("/") + cookie = res0["Set-Cookie"] + session_id = Rack::Session::SessionId.new cookie[session_match, 1] + ses0 = pool.pool.get(session_id.private_id, true) + pool.pool.set(session_id.public_id, ses0, 0, true) + pool.pool.delete(session_id.private_id) + + res2 = dreq.get("/", "HTTP_COOKIE" => cookie) + res2["Set-Cookie"].must_be_nil + res2.body.must_equal '{"counter"=>2}' + pool.pool.get(session_id.private_id, true).must_be_nil + pool.pool.get(session_id.public_id, true).must_be_nil + end + # anyone know how to do this better? it "cleanly merges sessions when multithreaded" do skip unless $DEBUG diff --git a/test/spec_session_pool.rb b/test/spec_session_pool.rb index 2d0616915..62a0505e7 100644 --- a/test/spec_session_pool.rb +++ b/test/spec_session_pool.rb @@ -6,7 +6,7 @@ describe Rack::Session::Pool do session_key = Rack::Session::Pool::DEFAULT_OPTIONS[:key] - session_match = /#{session_key}=[0-9a-fA-F]+;/ + session_match = /#{session_key}=([0-9a-fA-F]+);/ incrementor = lambda do |env| env["rack.session"]["counter"] ||= 0 @@ -14,7 +14,7 @@ Rack::Response.new(env["rack.session"].inspect).to_a end - session_id = Rack::Lint.new(lambda do |env| + get_session_id = Rack::Lint.new(lambda do |env| Rack::Response.new(env["rack.session"].inspect).to_a end) @@ -143,6 +143,43 @@ pool.pool.size.must_equal 1 end + it "can read the session with the legacy id" do + pool = Rack::Session::Pool.new(incrementor) + req = Rack::MockRequest.new(pool) + + res0 = req.get("/") + cookie = res0["Set-Cookie"] + session_id = Rack::Session::SessionId.new cookie[session_match, 1] + ses0 = pool.pool[session_id.private_id] + pool.pool[session_id.public_id] = ses0 + pool.pool.delete(session_id.private_id) + + res1 = req.get("/", "HTTP_COOKIE" => cookie) + res1["Set-Cookie"].must_be_nil + res1.body.must_equal '{"counter"=>2}' + pool.pool[session_id.private_id].wont_be_nil + end + + it "drops the session in the legacy id as well" do + pool = Rack::Session::Pool.new(incrementor) + req = Rack::MockRequest.new(pool) + drop = Rack::Utils::Context.new(pool, drop_session) + dreq = Rack::MockRequest.new(drop) + + res0 = req.get("/") + cookie = res0["Set-Cookie"] + session_id = Rack::Session::SessionId.new cookie[session_match, 1] + ses0 = pool.pool[session_id.private_id] + pool.pool[session_id.public_id] = ses0 + pool.pool.delete(session_id.private_id) + + res2 = dreq.get("/", "HTTP_COOKIE" => cookie) + res2["Set-Cookie"].must_be_nil + res2.body.must_equal '{"counter"=>2}' + pool.pool[session_id.private_id].must_be_nil + pool.pool[session_id.public_id].must_be_nil + end + # anyone know how to do this better? it "should merge sessions when multithreaded" do unless $DEBUG @@ -191,7 +228,7 @@ end it "does not return a cookie if cookie was not written (only read)" do - app = Rack::Session::Pool.new(session_id) + app = Rack::Session::Pool.new(get_session_id) res = Rack::MockRequest.new(app).get("/") res["Set-Cookie"].must_be_nil end