Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Make sure that Rails doesn't resent session_id cookie over and over a…

…gain if it's already there [#2485 state:resolved]

This apply to only Active Record store and Memcached store, as they both store only the session_id, which will be unchanged, in the cookie.

Signed-off-by: José Valim <jose.valim@gmail.com>
  • Loading branch information...
commit f8f365346ec55ef388eaa43e30bd7629c375d274 1 parent 4a745ca
@sikachu sikachu authored josevalim committed
View
32 actionpack/lib/action_controller/session/abstract_store.rb
@@ -139,21 +139,23 @@ def call(env)
return response
end
- cookie = Rack::Utils.escape(@key) + '=' + Rack::Utils.escape(sid)
- cookie << "; domain=#{options[:domain]}" if options[:domain]
- cookie << "; path=#{options[:path]}" if options[:path]
- if options[:expire_after]
- expiry = Time.now + options[:expire_after]
- cookie << "; expires=#{expiry.httpdate}"
- end
- cookie << "; Secure" if options[:secure]
- cookie << "; HttpOnly" if options[:httponly]
-
- headers = response[1]
- unless headers[SET_COOKIE].blank?
- headers[SET_COOKIE] << "\n#{cookie}"
- else
- headers[SET_COOKIE] = cookie
+ if (env["rack.request.cookie_hash"] && env["rack.request.cookie_hash"][@key] != sid) || options[:expire_after]
+ cookie = Rack::Utils.escape(@key) + '=' + Rack::Utils.escape(sid)
+ cookie << "; domain=#{options[:domain]}" if options[:domain]
+ cookie << "; path=#{options[:path]}" if options[:path]
+ if options[:expire_after]
+ expiry = Time.now + options[:expire_after]
+ cookie << "; expires=#{expiry.httpdate}"
+ end
+ cookie << "; Secure" if options[:secure]
+ cookie << "; HttpOnly" if options[:httponly]
+
+ headers = response[1]
+ unless headers[SET_COOKIE].blank?
+ headers[SET_COOKIE] << "\n#{cookie}"
+ else
+ headers[SET_COOKIE] = cookie
+ end
end
end
View
12 actionpack/test/activerecord/active_record_store_test.rb
@@ -107,6 +107,18 @@ def test_getting_session_id
end
end
+ def test_doesnt_write_session_cookie_if_session_id_is_already_exists
+ with_test_route_set do
+ get '/set_session_value'
+ assert_response :success
+ assert cookies['_session_id']
+
+ get '/get_session_value'
+ assert_response :success
+ assert_equal nil, headers['Set-Cookie'], "should not resend the cookie again if session_id cookie is already exists"
+ end
+ end
+
def test_prevents_session_fixation
with_test_route_set do
get '/set_session_value'
View
12 actionpack/test/controller/session/mem_cache_store_test.rb
@@ -95,6 +95,18 @@ def test_getting_session_id
end
end
+ def test_doesnt_write_session_cookie_if_session_id_is_already_exists
+ with_test_route_set do
+ get '/set_session_value'
+ assert_response :success
+ assert cookies['_session_id']
+
+ get '/get_session_value'
+ assert_response :success
+ assert_equal nil, headers['Set-Cookie'], "should not resend the cookie again if session_id cookie is already exists"
+ end
+ end
+
def test_prevents_session_fixation
with_test_route_set do
get '/get_session_value'
Please sign in to comment.
Something went wrong with that request. Please try again.