aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatthew Draper <matthew@trebex.net>2016-02-26 03:06:38 +1030
committerMatthew Draper <matthew@trebex.net>2016-02-26 03:06:38 +1030
commit6216a092ccfe6422f113db906a52fe8ffdafdbe6 (patch)
tree7f6fe35ff8f02185aeb9f2c8184665eb44bda81f
parent1f8ce32edcce503490aa873dd49788c6fd79c135 (diff)
downloadrails-6216a092ccfe6422f113db906a52fe8ffdafdbe6.tar.gz
rails-6216a092ccfe6422f113db906a52fe8ffdafdbe6.tar.bz2
rails-6216a092ccfe6422f113db906a52fe8ffdafdbe6.zip
Revert "Update Session to utilize indiffernt access"
This reverts commit 45a75a3fcc96b22954caf69be2df4e302b134d7a. HWIAs are better than silently deeply-stringified hashes... but that's a reaction to a shortcoming of one particular session store: we should not break the basic behaviour of other, more featureful, session stores in the process. Fixes #23884
-rw-r--r--actionpack/lib/action_controller/test_case.rb2
-rw-r--r--actionpack/lib/action_dispatch/request/session.rb22
-rw-r--r--actionpack/test/dispatch/request/session_test.rb2
-rw-r--r--actionpack/test/dispatch/session/abstract_store_test.rb16
-rw-r--r--actionpack/test/dispatch/session/cache_store_test.rb29
-rw-r--r--actionpack/test/dispatch/session/cookie_store_test.rb34
-rw-r--r--actionpack/test/dispatch/session/mem_cache_store_test.rb31
-rw-r--r--actionpack/test/dispatch/session/test_session_test.rb7
8 files changed, 16 insertions, 127 deletions
diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb
index 6548ce326b..700317614f 100644
--- a/actionpack/lib/action_controller/test_case.rb
+++ b/actionpack/lib/action_controller/test_case.rb
@@ -176,7 +176,7 @@ module ActionController
def initialize(session = {})
super(nil, nil)
@id = SecureRandom.hex(16)
- @data = session.with_indifferent_access
+ @data = stringify_keys(session)
@loaded = true
end
diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb
index 38d0da3e67..f6428af636 100644
--- a/actionpack/lib/action_dispatch/request/session.rb
+++ b/actionpack/lib/action_dispatch/request/session.rb
@@ -88,13 +88,13 @@ module ActionDispatch
# nil if the given key is not found in the session.
def [](key)
load_for_read!
- @delegate[key]
+ @delegate[key.to_s]
end
# Returns true if the session has the given key or false.
def has_key?(key)
load_for_read!
- @delegate.key?(key)
+ @delegate.key?(key.to_s)
end
alias :key? :has_key?
alias :include? :has_key?
@@ -112,7 +112,7 @@ module ActionDispatch
# Writes given value to given key of the session.
def []=(key, value)
load_for_write!
- @delegate[key] = value
+ @delegate[key.to_s] = value
end
# Clears the session.
@@ -139,13 +139,13 @@ module ActionDispatch
# # => {"session_id"=>"e29b9ea315edf98aad94cc78c34cc9b2", "foo" => "bar"}
def update(hash)
load_for_write!
- @delegate.update hash
+ @delegate.update stringify_keys(hash)
end
# Deletes given key from the session.
def delete(key)
load_for_write!
- @delegate.delete key
+ @delegate.delete key.to_s
end
# Returns value of the given key from the session, or raises +KeyError+
@@ -165,9 +165,9 @@ module ActionDispatch
def fetch(key, default=Unspecified, &block)
load_for_read!
if default == Unspecified
- @delegate.fetch(key, &block)
+ @delegate.fetch(key.to_s, &block)
else
- @delegate.fetch(key, default, &block)
+ @delegate.fetch(key.to_s, default, &block)
end
end
@@ -211,9 +211,15 @@ module ActionDispatch
def load!
id, session = @by.load_session @req
options[:id] = id
- @delegate.replace(session)
+ @delegate.replace(stringify_keys(session))
@loaded = true
end
+
+ def stringify_keys(other)
+ other.each_with_object({}) { |(key, value), hash|
+ hash[key.to_s] = value
+ }
+ end
end
end
end
diff --git a/actionpack/test/dispatch/request/session_test.rb b/actionpack/test/dispatch/request/session_test.rb
index 3433d82791..3fc4ffd71c 100644
--- a/actionpack/test/dispatch/request/session_test.rb
+++ b/actionpack/test/dispatch/request/session_test.rb
@@ -105,7 +105,7 @@ module ActionDispatch
end
end
- def test_indifferent_access
+ def test_with_indifferent_access
s = Session.create(store, req, {})
s[:one] = { test: "deep" }
diff --git a/actionpack/test/dispatch/session/abstract_store_test.rb b/actionpack/test/dispatch/session/abstract_store_test.rb
index c9ce5cad42..d38d1bbce6 100644
--- a/actionpack/test/dispatch/session/abstract_store_test.rb
+++ b/actionpack/test/dispatch/session/abstract_store_test.rb
@@ -46,22 +46,6 @@ 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 b911392cf1..dbb996973d 100644
--- a/actionpack/test/dispatch/session/cache_store_test.rb
+++ b/actionpack/test/dispatch/session/cache_store_test.rb
@@ -12,11 +12,6 @@ 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
@@ -26,14 +21,6 @@ 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
@@ -173,22 +160,6 @@ 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 71402b021a..f07e215e3a 100644
--- a/actionpack/test/dispatch/session/cookie_store_test.rb
+++ b/actionpack/test/dispatch/session/cookie_store_test.rb
@@ -24,23 +24,10 @@ 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
@@ -94,15 +81,6 @@ 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
@@ -354,18 +332,6 @@ 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 2e6b42856f..3fed9bad4f 100644
--- a/actionpack/test/dispatch/session/mem_cache_store_test.rb
+++ b/actionpack/test/dispatch/session/mem_cache_store_test.rb
@@ -13,11 +13,6 @@ 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
@@ -27,14 +22,6 @@ 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
@@ -192,24 +179,6 @@ 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 332c2ae3c8..3e61d123e3 100644
--- a/actionpack/test/dispatch/session/test_session_test.rb
+++ b/actionpack/test/dispatch/session/test_session_test.rb
@@ -60,11 +60,4 @@ 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