Merge pull request #20851 from tomprats/indifferent-sessions

Give Sessions Indifferent Access

6 files changed, 127 insertions, 0 deletions

diff --git a/actionpack/test/dispatch/request/session_test.rb b/actionpack/test/dispatch/request/session_test.rb
index 7dcbcc5c21..3433d82791 100644
--- a/actionpack/test/dispatch/request/session_test.rb
+++ b/actionpack/test/dispatch/request/session_test.rb
@@ -105,6 +105,16 @@ module ActionDispatch
         end
       end
 
+      def test_indifferent_access
+        s = Session.create(store, req, {})
+
+        s[:one] = { test: "deep" }
+        s[:two] = { "test" => "deep" }
+
+        assert_equal 'deep', s[:one]["test"]
+        assert_equal 'deep', s[:two][:test]
+      end
+
       private
       def store
         Class.new {
diff --git a/actionpack/test/dispatch/session/abstract_store_test.rb b/actionpack/test/dispatch/session/abstract_store_test.rb
index d38d1bbce6..c9ce5cad42 100644
--- a/actionpack/test/dispatch/session/abstract_store_test.rb
+++ b/actionpack/test/dispatch/session/abstract_store_test.rb
@@ -46,6 +46,22 @@ module ActionDispatch
         assert_equal session.to_hash, session1.to_hash
       end
 
+      def test_previous_session_has_indifferent_access
+        env = {}
+        as = MemoryStore.new app
+        as.call(env)
+
+        assert @env
+        session = Request::Session.find ActionDispatch::Request.new @env
+        session[:foo] = { bar: "baz" }
+
+        as.call(@env)
+        session = Request::Session.find ActionDispatch::Request.new @env
+
+        assert_equal session[:foo][:bar], "baz"
+        assert_equal session[:foo]["bar"], "baz"
+      end
+
       private
       def app(&block)
         @env = nil
diff --git a/actionpack/test/dispatch/session/cache_store_test.rb b/actionpack/test/dispatch/session/cache_store_test.rb
index dbb996973d..b911392cf1 100644
--- a/actionpack/test/dispatch/session/cache_store_test.rb
+++ b/actionpack/test/dispatch/session/cache_store_test.rb
@@ -12,6 +12,11 @@ class CacheStoreTest < ActionDispatch::IntegrationTest
       head :ok
     end
 
+    def set_deep_session_value
+      session[:foo] = { bar: "baz" }
+      head :ok
+    end
+
     def set_serialized_session_value
       session[:foo] = SessionAutoloadTest::Foo.new
       head :ok
@@ -21,6 +26,14 @@ class CacheStoreTest < ActionDispatch::IntegrationTest
       render plain: "foo: #{session[:foo].inspect}"
     end
 
+    def get_deep_session_value_with_symbol
+      render plain: "foo: { bar: #{session[:foo][:bar].inspect} }"
+    end
+
+    def get_deep_session_value_with_string
+      render plain: "foo: { \"bar\" => #{session[:foo]["bar"].inspect} }"
+    end
+
     def get_session_id
       render plain: "#{request.session.id}"
     end
@@ -160,6 +173,22 @@ class CacheStoreTest < ActionDispatch::IntegrationTest
     end
   end
 
+  def test_previous_session_has_indifferent_access
+    with_test_route_set do
+      get '/set_deep_session_value'
+      assert_response :success
+      assert cookies['_session_id']
+
+      get '/get_deep_session_value_with_symbol'
+      assert_response :success
+      assert_equal 'foo: { bar: "baz" }', response.body
+
+      get '/get_deep_session_value_with_string'
+      assert_response :success
+      assert_equal 'foo: { "bar" => "baz" }', response.body
+    end
+  end
+
   private
     def with_test_route_set
       with_routing do |set|
diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb
index f07e215e3a..71402b021a 100644
--- a/actionpack/test/dispatch/session/cookie_store_test.rb
+++ b/actionpack/test/dispatch/session/cookie_store_test.rb
@@ -24,10 +24,23 @@ class CookieStoreTest < ActionDispatch::IntegrationTest
       render plain: Rack::Utils.escape(Verifier.generate(session.to_hash))
     end
 
+    def set_deep_session_value
+      session[:foo] = { bar: "baz" }
+      render plain: Rack::Utils.escape(Verifier.generate(session.to_hash))
+    end
+
     def get_session_value
       render plain: "foo: #{session[:foo].inspect}"
     end
 
+    def get_deep_session_value_with_symbol
+      render plain: "foo: { bar: #{session[:foo][:bar].inspect} }"
+    end
+
+    def get_deep_session_value_with_string
+      render plain: "foo: { \"bar\" => #{session[:foo]["bar"].inspect} }"
+    end
+
     def get_session_id
       render plain: "id: #{request.session.id}"
     end
@@ -81,6 +94,15 @@ class CookieStoreTest < ActionDispatch::IntegrationTest
     end
   end
 
+  def test_session_indifferent_access
+    with_test_route_set do
+      cookies[SessionKey] = SignedBar
+      get '/get_session_value'
+      assert_response :success
+      assert_equal 'foo: "bar"', response.body
+    end
+  end
+
   def test_getting_session_id
     with_test_route_set do
       cookies[SessionKey] = SignedBar
@@ -332,6 +354,18 @@ class CookieStoreTest < ActionDispatch::IntegrationTest
     end
   end
 
+  def test_previous_session_has_indifferent_access
+    with_test_route_set do
+      get '/set_deep_session_value'
+
+      get '/get_deep_session_value_with_symbol'
+      assert_equal 'foo: { bar: "baz" }', response.body
+
+      get '/get_deep_session_value_with_string'
+      assert_equal 'foo: { "bar" => "baz" }', response.body
+    end
+  end
+
   private
 
     # Overwrite get to send SessionSecret in env hash
diff --git a/actionpack/test/dispatch/session/mem_cache_store_test.rb b/actionpack/test/dispatch/session/mem_cache_store_test.rb
index 3fed9bad4f..2e6b42856f 100644
--- a/actionpack/test/dispatch/session/mem_cache_store_test.rb
+++ b/actionpack/test/dispatch/session/mem_cache_store_test.rb
@@ -13,6 +13,11 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest
       head :ok
     end
 
+    def set_deep_session_value
+      session[:foo] = { bar: "baz" }
+      head :ok
+    end
+
     def set_serialized_session_value
       session[:foo] = SessionAutoloadTest::Foo.new
       head :ok
@@ -22,6 +27,14 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest
       render plain: "foo: #{session[:foo].inspect}"
     end
 
+    def get_deep_session_value_with_symbol
+      render plain: "foo: { bar: #{session[:foo][:bar].inspect} }"
+    end
+
+    def get_deep_session_value_with_string
+      render plain: "foo: { \"bar\" => #{session[:foo]["bar"].inspect} }"
+    end
+
     def get_session_id
       render plain: "#{request.session.id}"
     end
@@ -179,6 +192,24 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest
     rescue Dalli::RingError => ex
       skip ex.message, ex.backtrace
     end
+
+    def test_previous_session_has_indifferent_access
+      with_test_route_set do
+        get '/set_deep_session_value'
+        assert_response :success
+        assert cookies['_session_id']
+
+        get '/get_deep_session_value_with_symbol'
+        assert_response :success
+        assert_equal 'foo: { bar: "baz" }', response.body
+
+        get '/get_deep_session_value_with_string'
+        assert_response :success
+        assert_equal 'foo: { "bar" => "baz" }', response.body
+      end
+    rescue Dalli::RingError => ex
+      skip ex.message, ex.backtrace
+    end
   rescue LoadError, RuntimeError, Dalli::DalliError
     $stderr.puts "Skipping MemCacheStoreTest tests. Start memcached and try again."
   end
diff --git a/actionpack/test/dispatch/session/test_session_test.rb b/actionpack/test/dispatch/session/test_session_test.rb
index 3e61d123e3..332c2ae3c8 100644
--- a/actionpack/test/dispatch/session/test_session_test.rb
+++ b/actionpack/test/dispatch/session/test_session_test.rb
@@ -60,4 +60,11 @@ class ActionController::TestSessionTest < ActiveSupport::TestCase
     session = ActionController::TestSession.new(one: '1')
     assert_equal(2, session.fetch('2') { |key| key.to_i })
   end
+
+  def test_fetch_returns_indifferent_access
+    session = ActionController::TestSession.new(three: { two: '1' })
+    three = session.fetch(:three)
+    assert_equal('1', three[:two])
+    assert_equal('1', three["two"])
+  end
 end