diff options
6 files changed, 113 insertions, 36 deletions
diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index bc1d6fab83..08bc80dbc2 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -88,9 +88,7 @@ module ActionDispatch def exists? return @exists if instance_variable_defined?(:@exists) - stale_session_check! do - @exists = @by.send(:exists?, @env) - end + @exists = @by.send(:exists?, @env) end def loaded? @@ -115,29 +113,12 @@ module ActionDispatch end def load! - stale_session_check! do - id, session = @by.send(:load_session, @env) - @env[ENV_SESSION_OPTIONS_KEY][:id] = id - replace(session.stringify_keys) - @loaded = true - end + id, session = @by.send(:load_session, @env) + @env[ENV_SESSION_OPTIONS_KEY][:id] = id + replace(session.stringify_keys) + @loaded = true 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 ActionDispatch::Session::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 DEFAULT_OPTIONS = { @@ -204,16 +185,20 @@ module ActionDispatch end def load_session(env) - sid = current_session_id(env) - sid, session = get_session(env, sid) - [sid, session] + stale_session_check! do + sid = current_session_id(env) + sid, session = get_session(env, sid) + [sid, session] + end end def extract_session_id(env) - request = ActionDispatch::Request.new(env) - sid = request.cookies[@key] - sid ||= request.params[@key] unless @cookie_only - sid + stale_session_check! do + request = ActionDispatch::Request.new(env) + sid = request.cookies[@key] + sid ||= request.params[@key] unless @cookie_only + sid + end end def current_session_id(env) @@ -229,6 +214,22 @@ module ActionDispatch end 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 ActionDispatch::Session::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 + def exists?(env) current_session_id(env).present? end @@ -245,7 +246,6 @@ module ActionDispatch def destroy(env) raise '#destroy needs to be implemented.' end - end end end diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index 7ebd532f4a..dce47c63bc 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -63,11 +63,13 @@ module ActionDispatch def unpacked_cookie_data(env) env["action_dispatch.request.unsigned_session_cookie"] ||= begin - request = ActionDispatch::Request.new(env) - if data = request.cookie_jar.signed[@key] - data.stringify_keys! + stale_session_check! do + request = ActionDispatch::Request.new(env) + if data = request.cookie_jar.signed[@key] + data.stringify_keys! + end + data || {} end - data || {} end end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 84c5395610..2640e96453 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -202,6 +202,21 @@ class ActionController::IntegrationTest < ActiveSupport::TestCase self.class.app = old_app silence_warnings { Object.const_set(:SharedTestRoutes, old_routes) } end + + def with_autoload_path(path) + path = File.join(File.dirname(__FILE__), "fixtures", path) + if ActiveSupport::Dependencies.autoload_paths.include?(path) + yield + else + begin + ActiveSupport::Dependencies.autoload_paths << path + yield + ensure + ActiveSupport::Dependencies.autoload_paths.reject! {|p| p == path} + ActiveSupport::Dependencies.clear + end + end + end end # Temporary base class diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index 6aca22b456..fd63f5ad5e 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -96,6 +96,31 @@ class CookieStoreTest < ActionController::IntegrationTest end end + # {:foo=>#<SessionAutoloadTest::Foo bar:"baz">, :session_id=>"ce8b0752a6ab7c7af3cdb8a80e6b9e46"} + SignedSerializedCookie = "BAh7BzoIZm9vbzodU2Vzc2lvbkF1dG9sb2FkVGVzdDo6Rm9vBjoJQGJhciIIYmF6Og9zZXNzaW9uX2lkIiVjZThiMDc1MmE2YWI3YzdhZjNjZGI4YTgwZTZiOWU0Ng==--2bf3af1ae8bd4e52b9ac2099258ace0c380e601c" + + def test_deserializes_unloaded_classes_on_get_id + with_test_route_set do + with_autoload_path "session_autoload_test" do + cookies[SessionKey] = SignedSerializedCookie + get '/get_session_id' + assert_response :success + assert_equal 'id: ce8b0752a6ab7c7af3cdb8a80e6b9e46', response.body, "should auto-load unloaded class" + end + end + end + + def test_deserializes_unloaded_classes_on_get_value + with_test_route_set do + with_autoload_path "session_autoload_test" do + cookies[SessionKey] = SignedSerializedCookie + get '/get_session_value' + assert_response :success + assert_equal 'foo: #<SessionAutoloadTest::Foo bar:"baz">', response.body, "should auto-load unloaded class" + end + end + end + def test_close_raises_when_data_overflows with_test_route_set do assert_raise(ActionDispatch::Cookies::CookieOverflow) { @@ -247,4 +272,5 @@ class CookieStoreTest < ActionController::IntegrationTest yield end end + end diff --git a/actionpack/test/dispatch/session/mem_cache_store_test.rb b/actionpack/test/dispatch/session/mem_cache_store_test.rb index d388992b98..9bd6f9b8c4 100644 --- a/actionpack/test/dispatch/session/mem_cache_store_test.rb +++ b/actionpack/test/dispatch/session/mem_cache_store_test.rb @@ -11,6 +11,11 @@ class MemCacheStoreTest < ActionController::IntegrationTest session[:foo] = "bar" head :ok end + + def set_serialized_session_value + session[:foo] = SessionAutoloadTest::Foo.new + head :ok + end def get_session_value render :text => "foo: #{session[:foo].inspect}" @@ -117,6 +122,25 @@ class MemCacheStoreTest < ActionController::IntegrationTest end end + def test_deserializes_unloaded_class + with_test_route_set do + with_autoload_path "session_autoload_test" do + get '/set_serialized_session_value' + assert_response :success + assert cookies['_session_id'] + end + with_autoload_path "session_autoload_test" do + get '/get_session_id' + assert_response :success + end + with_autoload_path "session_autoload_test" do + get '/get_session_value' + assert_response :success + assert_equal 'foo: #<SessionAutoloadTest::Foo bar:"baz">', response.body, "should auto-load unloaded class" + end + end + end + def test_doesnt_write_session_cookie_if_session_id_is_already_exists with_test_route_set do get '/set_session_value' diff --git a/actionpack/test/fixtures/session_autoload_test/session_autoload_test/foo.rb b/actionpack/test/fixtures/session_autoload_test/session_autoload_test/foo.rb new file mode 100644 index 0000000000..4ee7a24561 --- /dev/null +++ b/actionpack/test/fixtures/session_autoload_test/session_autoload_test/foo.rb @@ -0,0 +1,10 @@ +module SessionAutoloadTest + class Foo + def initialize(bar='baz') + @bar = bar + end + def inspect + "#<#{self.class} bar:#{@bar.inspect}>" + end + end +end |