Skip to content

Commit

Permalink
Sessions should not be created until written to and session data shou…
Browse files Browse the repository at this point in the history 
…ld be destroyed on reset. [#4938 state:resolved]

Signed-off-by: José Valim <jose.valim@gmail.com>
  • Loading branch information
@lovitt @josevalim
lovitt authored and josevalim committed Jul 14, 2010
1 parent 8298bef commit 257a29d
Show file tree
Hide file tree
Showing 10 changed files with 376 additions and 77 deletions.
4 changes: 2 additions & 2 deletions actionpack/lib/action_controller/request.rb
View file
Expand Up @@ -446,8 +446,8 @@ def session=(session) #:nodoc:
end

def reset_session
@env['rack.session.options'].delete(:id)
@env['rack.session'] = {}
session.destroy if session

This comment has been minimized.

Sign in to view

Copy link
@sigmike

sigmike Mar 6, 2011

I had an error here because session is a Hash and doesn't have a destroy method. Seems consistent with the line below where session is initialized as a Hash. Shouldn't this case be checked ? (like if session.responds_to? :destroy, or something)
self.session = {}
end

def session_options
Expand Down
173 changes: 131 additions & 42 deletions actionpack/lib/action_controller/session/abstract_store.rb
View file
Expand Up @@ -2,13 +2,42 @@

module ActionController
module Session
class AbstractStore
class AbstractStore
ENV_SESSION_KEY = 'rack.session'.freeze
ENV_SESSION_OPTIONS_KEY = 'rack.session.options'.freeze

HTTP_COOKIE = 'HTTP_COOKIE'.freeze
SET_COOKIE = 'Set-Cookie'.freeze

# thin wrapper around Hash that allows us to lazily
# load session id into session_options
class OptionsHash < Hash
def initialize(by, env, default_options)
@by = by
@env = env
@session_id_loaded = false
merge!(default_options)
end

def [](key)
if key == :id
load_session_id! unless super(:id) || has_session_id?
end
super(key)
end

private

def has_session_id?
@session_id_loaded
end

def load_session_id!
self[:id] = @by.send(:extract_session_id, @env)
@session_id_loaded = true
end
end

class SessionHash < Hash
def initialize(by, env)
super()
Expand All @@ -25,26 +54,42 @@ def session_id
end

def [](key)
load! unless @loaded
load_for_read!
super
end

def has_key?(key)
load_for_read!
super
end

def []=(key, value)
load! unless @loaded
load_for_write!
super
end

def clear
load! unless @loaded
load_for_write!
super
end

def to_hash
load_for_read!
h = {}.replace(self)
h.delete_if { |k,v| v.nil? }
h
end

def update(hash)
load_for_write!
super
end

def delete(key)
load_for_write!
super
end

def data
ActiveSupport::Deprecation.warn(
"ActionController::Session::AbstractStore::SessionHash#data " +
Expand All @@ -53,40 +98,43 @@ def data
end

def inspect
load! unless @loaded
load_for_read!
super
end

def exists?
return @exists if instance_variable_defined?(:@exists)
@exists = @by.send(:exists?, @env)
end

def loaded?
@loaded
end

def destroy
clear
@by.send(:destroy, @env) if @by
@env[ENV_SESSION_OPTIONS_KEY][:id] = nil if @env && @env[ENV_SESSION_OPTIONS_KEY]
@loaded = false
end

private
def loaded?
@loaded

def load_for_read!
load! if !loaded? && exists?
end

def load!
stale_session_check! do
id, session = @by.send(:load_session, @env)
(@env[ENV_SESSION_OPTIONS_KEY] ||= {})[:id] = id
replace(session)
@loaded = true
end
def load_for_write!
load! unless loaded?
end

def stale_session_check!
yield
rescue ArgumentError => argument_error
if argument_error.message =~ %r{undefined class/module ([\w:]*\w)}
begin
# Note that the regexp does not allow $1 to end with a ':'
$1.constantize
rescue LoadError, NameError => const_error
raise ActionController::SessionRestoreError, "Session contains objects whose class definition isn\\'t available.\nRemember to require the classes for all objects kept in the session.\n(Original exception: \#{const_error.message} [\#{const_error.class}])\n"
end

retry
else
raise
end
def load!
id, session = @by.send(:load_session, @env)
@env[ENV_SESSION_OPTIONS_KEY][:id] = id
replace(session)
@loaded = true
end

end

DEFAULT_OPTIONS = {
Expand Down Expand Up @@ -125,18 +173,14 @@ def initialize(app, options = {})
end

def call(env)
session = SessionHash.new(self, env)

env[ENV_SESSION_KEY] = session
env[ENV_SESSION_OPTIONS_KEY] = @default_options.dup

prepare!(env)
response = @app.call(env)

session_data = env[ENV_SESSION_KEY]
options = env[ENV_SESSION_OPTIONS_KEY]

if !session_data.is_a?(AbstractStore::SessionHash) || session_data.send(:loaded?) || options[:expire_after]
session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.send(:loaded?)
if !session_data.is_a?(AbstractStore::SessionHash) || session_data.loaded? || options[:expire_after]
session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.loaded?

sid = options[:id] || generate_sid

Expand Down Expand Up @@ -168,18 +212,39 @@ def call(env)
end

private

def prepare!(env)
env[ENV_SESSION_KEY] = SessionHash.new(self, env)
env[ENV_SESSION_OPTIONS_KEY] = OptionsHash.new(self, env, @default_options)
end

def generate_sid
ActiveSupport::SecureRandom.hex(16)
end

def load_session(env)
request = Rack::Request.new(env)
sid = request.cookies[@key]
unless @cookie_only
sid ||= request.params[@key]
stale_session_check! do
sid = current_session_id(env)
sid, session = get_session(env, sid)
[sid, session]
end
end

def extract_session_id(env)
stale_session_check! do
request = Rack::Request.new(env)
sid = request.cookies[@key]
sid ||= request.params[@key] unless @cookie_only
sid
end
sid, session = get_session(env, sid)
[sid, session]
end

def current_session_id(env)
env[ENV_SESSION_OPTIONS_KEY][:id]
end

def exists?(env)
current_session_id(env).present?
end

def get_session(env, sid)
Expand All @@ -189,6 +254,30 @@ def get_session(env, sid)
def set_session(env, sid, session_data)
raise '#set_session needs to be implemented.'
end

def destroy(env)
raise '#destroy needs to be implemented.'
end

module SessionUtils
private
def stale_session_check!
yield
rescue ArgumentError => argument_error
if argument_error.message =~ %r{undefined class/module ([\w:]*\w)}
begin
# Note that the regexp does not allow $1 to end with a ':'
$1.constantize
rescue LoadError, NameError => const_error
raise ActionController::SessionRestoreError, "Session contains objects whose class definition isn\\'t available.\nRemember to require the classes for all objects kept in the session.\n(Original exception: \#{const_error.message} [\#{const_error.class}])\n"
end
retry
else
raise
end
end
end
include SessionUtils
end
end
end
60 changes: 49 additions & 11 deletions actionpack/lib/action_controller/session/cookie_store.rb
View file
Expand Up @@ -36,6 +36,8 @@ module Session
#
# Note that changing digest or secret invalidates all existing sessions!
class CookieStore
include AbstractStore::SessionUtils

# Cookies can typically store 4096 bytes.
MAX = 4096
SECRET_MIN_LENGTH = 30 # characters
Expand Down Expand Up @@ -93,20 +95,20 @@ def initialize(app, options = {})
end

def call(env)
env[ENV_SESSION_KEY] = AbstractStore::SessionHash.new(self, env)
env[ENV_SESSION_OPTIONS_KEY] = @default_options.dup

prepare!(env)

status, headers, body = @app.call(env)

session_data = env[ENV_SESSION_KEY]
options = env[ENV_SESSION_OPTIONS_KEY]

if !session_data.is_a?(AbstractStore::SessionHash) || session_data.send(:loaded?) || options[:expire_after]
session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.send(:loaded?)
if !session_data.is_a?(AbstractStore::SessionHash) || session_data.loaded? || options[:expire_after]
session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.loaded?

persistent_session_id!(session_data)
session_data = marshal(session_data.to_hash)

raise CookieOverflow if session_data.size > MAX

cookie = Hash.new
cookie[:value] = session_data
unless options[:expire_after].nil?
Expand All @@ -122,6 +124,12 @@ def call(env)
end

private

def prepare!(env)
env[ENV_SESSION_KEY] = AbstractStore::SessionHash.new(self, env)
env[ENV_SESSION_OPTIONS_KEY] = AbstractStore::OptionsHash.new(self, env, @default_options)
end

# Should be in Rack::Utils soon
def build_cookie(key, value)
case value
Expand All @@ -143,20 +151,46 @@ def build_cookie(key, value)
end

def load_session(env)
request = Rack::Request.new(env)
session_data = request.cookies[@key]
data = unmarshal(session_data) || persistent_session_id!({})
data = unpacked_cookie_data(env)
data = persistent_session_id!(data)
[data[:session_id], data]
end

def extract_session_id(env)
if data = unpacked_cookie_data(env)
persistent_session_id!(data) unless data.empty?
data[:session_id]
else
nil
end
end

def current_session_id(env)
env[ENV_SESSION_OPTIONS_KEY][:id]
end

def exists?(env)
current_session_id(env).present?
end

def unpacked_cookie_data(env)
env["action_dispatch.request.unsigned_session_cookie"] ||= begin
stale_session_check! do
request = Rack::Request.new(env)
session_data = request.cookies[@key]
unmarshal(session_data) || {}
end
end
end

# Marshal a session hash into safe cookie data. Include an integrity hash.
def marshal(session)
@verifier.generate(persistent_session_id!(session))
@verifier.generate(session)
end

# Unmarshal cookie data to a hash and verify its integrity.
def unmarshal(cookie)
persistent_session_id!(@verifier.verify(cookie)) if cookie
@verifier.verify(cookie) if cookie
rescue ActiveSupport::MessageVerifier::InvalidSignature
nil
end
Expand Down Expand Up @@ -204,6 +238,10 @@ def generate_sid
ActiveSupport::SecureRandom.hex(16)
end

def destroy(env)
# session data is stored on client; nothing to do here
end

def persistent_session_id!(data)
(data ||= {}).merge!(inject_persistent_session_id(data))
end
Expand Down
9 changes: 9 additions & 0 deletions actionpack/lib/action_controller/session/mem_cache_store.rb
View file
Expand Up @@ -43,6 +43,15 @@ def set_session(env, sid, session_data)
rescue MemCache::MemCacheError, Errno::ECONNREFUSED
return false
end

def destroy(env)
if sid = current_session_id(env)
@pool.delete(sid)
end
rescue MemCache::MemCacheError, Errno::ECONNREFUSED
false
end

end
end
end
Expand Down

0 comments on commit 257a29d

Please sign in to comment.