This project is archived and is in readonly mode.

reset_session broken in rails 2.3.8 when using ActiveRecordStore for sessions

#5086

reset_session doesn't work properly when using ActiveRecordStore, or probably other derivatives of ActionController::Session::AbstractStore.

After a reset_session, when ActionController::Session::AbstractStore generates a new session ID and tries to set the session cookie, it creates an invalid header instead.

Here are the relevant extracts from a debugging session with rails 2.3.8 and rack 1.1.0, although the problem was not fixed by upgrading rack to 1.2.1.

[127, 136] in /home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/session/abstract_store.rb

130         puts "abstract_store env session before call = #{env[ENV_SESSION_KEY]}"
131          response = @app.call(env)

=> 132 puts "abstract_store env session after call = #{env[ENV_SESSION_KEY]}" /home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/session/abstract_store.rb:132 puts "abstract_store env session after call = #{env[ENV_SESSION_KEY]}"

(rdb:1) p env['rack.session.options'] {:path=>"/", :httponly=>true, :domain=>nil, :expire_after=>nil, :cookie_only=>true, :key=>"_session_id", :secret=>"35be19722d70e3549c1707b87baf40f341585c32bd2c9c63cfb19902675647d6b94f07fdcc2b1dc77a5a62a03851bb1e2ccdff7722e6718b367a064a034ade01", :secure=>false}

There is no saved session ID in rack.session.options at this point...

[135, 144] in /home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/session/abstract_store.rb

135          options = env[ENV_SESSION_OPTIONS_KEY]
136  
137          if !session_data.is_a?(AbstractStore::SessionHash) || session_data.send(:loaded?) || options[:expire_after]
138            session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.send(:loaded?)
139

=> 140 sid = options[:id] || generate_sid

141
142            unless set_session(env, sid, session_data.to_hash)
143              return response
144            end

/home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/session/abstract_store.rb:140 sid = options[:id] || generate_sid

(rdb:1) p options[:id] nil

So we generate a new session ID here, and save it in the database, then generate a new cookie:

[141, 150] in /home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/session/abstract_store.rb

141
142            unless set_session(env, sid, session_data.to_hash)
143              return response
144            end
145

=> 146 cookie = Rack::Utils.escape(@key) + '=' + Rack::Utils.escape(sid)

147            cookie << "; domain=#{options[:domain]}" if options[:domain]
148            cookie << "; path=#{options[:path]}" if options[:path]
149            if options[:expire_after]
150              expiry = Time.now + options[:expire_after]

/home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/session/abstract_store.rb:146 cookie = Rack::Utils.escape(@key) + '=' + Rack::Utils.escape(sid)

(rdb:1) p @key "_session_id"

(rdb:1) p sid "fca9862ce0015820072e85ae40262159"

Stepped through the rest of the cookie creation to the header setting:

[153, 162] in /home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/session/abstract_store.rb

153            cookie << "; Secure" if options[:secure]
154            cookie << "; HttpOnly" if options[:httponly]
155  
156            headers = response[1]
157            unless headers[SET_COOKIE].blank?

=> 158 headers[SET_COOKIE] << "\n#{cookie}"

159            else
160              headers[SET_COOKIE] = cookie
161            end
162          end

/home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/session/abstract_store.rb:158 headers[SET_COOKIE] << "\n#{cookie}"

(rdb:1) p headers[SET_COOKIE] ["auth_token=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT"]

Whoops, it's an Array, not a String, but we're about to append a new element:

[159, 168] in /home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/session/abstract_store.rb

159            else
160              headers[SET_COOKIE] = cookie
161            end
162          end
163

=> 164 response

165        end
166  
167        private
168          def generate_sid

/home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/session/abstract_store.rb:164 response

(rdb:1) p headers[SET_COOKIE] ["auth_token=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT", "\n_session_id=fca9862ce0015820072e85ae40262159; path=/; HttpOnly"]

Boom, this is not a good cookie. Note the newline before _session_id, which will prevent it being used as the actual session ID, so the next request resumes the old session rather than starting a new one, meaning that reset_session had no effect.

We are now here:

/home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/session/abstract_store.rb:164:in call'
/home/chris/project/minetoo/vendor/rails/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb:34:in cache'
/home/chris/project/minetoo/vendor/rails/activerecord/lib/active_record/query_cache.rb:9:in cache'
/home/chris/project/minetoo/vendor/rails/activerecord/lib/active_record/query_cache.rb:28:in call'
/home/chris/project/minetoo/vendor/rails/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:361:in call'
/home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/failsafe.rb:26:in call'
/home/chris/project/minetoo/vendor/gems/rack-1.1.0/lib/rack/lock.rb:11:in call'
/home/chris/project/minetoo/vendor/gems/rack-1.1.0/lib/rack/lock.rb:11:in synchronize'
/home/chris/project/minetoo/vendor/gems/rack-1.1.0/lib/rack/lock.rb:11:in call'
/home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/dispatcher.rb:110:in call'
/home/chris/project/minetoo/vendor/gems/rack-1.1.0/lib/rack/lint.rb:47:in _call'
/home/chris/project/minetoo/vendor/gems/rack-1.1.0/lib/rack/lint.rb:35:in call'
/home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/integration.rb:316:in process'
/home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/integration.rb:197:in get'
/home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/integration.rb:503:in __send__'
/home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/integration.rb:503:in get'
test/functional/about_controller_test.rb:329:in test_about_page'
test/functional/about_controller_test.rb:284:in each'
test/functional/about_controller_test.rb:284:in test_about_page'
test/functional/about_controller_test.rb:281:in each'
test/functional/about_controller_test.rb:281:in test_about_page'
/home/chris/project/minetoo/vendor/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:62:in __send__'
/home/chris/project/minetoo/vendor/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:62:in run'
/home/chris/project/minetoo/vendor/rails/actionpack/lib/action_controller/integration.rb:661:in run'
/usr/lib/ruby/1.8/test/unit/testsuite.rb:34:in run'
/usr/lib/ruby/1.8/test/unit/testsuite.rb:33:in each'
/usr/lib/ruby/1.8/test/unit/testsuite.rb:33:in run'
/usr/lib/ruby/1.8/test/unit/testsuite.rb:34:in run'
/usr/lib/ruby/1.8/test/unit/testsuite.rb:33:in each'
/usr/lib/ruby/1.8/test/unit/testsuite.rb:33:in run'
/usr/lib/ruby/1.8/test/unit/ui/testrunnermediator.rb:46:in run_suite'
/usr/lib/ruby/1.8/test/unit/ui/console/testrunner.rb:67:in start_mediator'
/usr/lib/ruby/1.8/test/unit/ui/console/testrunner.rb:41:in start'
/usr/lib/ruby/1.8/test/unit/ui/testrunnerutilities.rb:29:in run'
/usr/lib/ruby/1.8/test/unit/autorunner.rb:216:in run'
/usr/lib/ruby/1.8/test/unit/autorunner.rb:12:in run'
/usr/lib/ruby/1.8/test/unit.rb:278
test/functional/about_controller_test.rb:280

I think this is the same problem as #4743 Session Cookie breaks if used with custom cookie in rails 2.3.8, but was not resolved along with #4743 Session Cookie breaks if used with custom cookie in rails 2.3.8 because AbstractStore contains a (still broken) copy of the old header code that CookieStore used to have, which assumed that headers were Strings. It seems from #4743 Session Cookie breaks if used with custom cookie in rails 2.3.8 that Rack recently changed headers to be Arrays instead of Strings, and all the duplicated copies of this header handling code in Rails need to be updated.

I think you could demonstrate this bug by applying the same tests written for #4743 Session Cookie breaks if used with custom cookie in rails 2.3.8 (from [85b6d79d8a17fdef667770e31b44ac6647f8b584]) with an ActiveRecordStore session store instead of CookieStore, but I don't know how to run these tests or how to switch session store on the fly.

I was able to resolve the problem with the following patch:

--- /usr/lib/ruby/gems/1.8/gems/actionpack-2.3.8/lib/action_controller/session/abstract_store.rb 2010-07-11 15:55:36.000000000 +0200 +++ vendor/rails/actionpack/lib/action_controller/session/abstract_store.rb 2010-07-11 16:04:43.000000000 +0200 @@ -150,11 +154,8 @@

        cookie << "; HttpOnly" if options[:httponly]

        headers = response[1]

```
    unless headers[SET_COOKIE].blank?
```

      headers[SET_COOKIE] << "\n#{cookie}"

```
    else
```
```
      headers[SET_COOKIE] = cookie
```
```
    end
```

            headers[SET_COOKIE] = [] if headers[SET_COOKIE].blank?

            headers[SET_COOKIE] << cookie
  end

  response

Reported by Chris Wilson · July 11th, 2010 @ 03:41 PM

Activity

Chris Wilson
- Title changed from reset_session broken in raisl 2.3.8 when using ActiveRecordStore for cookies to reset_session broken in rails 2.3.8 when using ActiveRecordStore for sessions
July 11th, 2010 @ 03:42 PM
Chris Wilson
- bug-report-5086.txt 9.31 KB
July 11th, 2010 @ 05:02 PM
Chris Wilson

Please note #1957 Sessions break in 2.3 with Mongrel:

"there is a crapload of duplicated code (almost all of the session cookie store and the abstract session store are duplicated code).

I'll look into searching for all of the places where a cookie may be built into an array, which is the real issue."

But I don't think this was done, and #1957 Sessions break in 2.3 with Mongrel was closed without it.

July 11th, 2010 @ 05:23 PM
Sean Rhea
+1

I'm seeing this same bug, and Chris's patch to abstract_store.rb fixes the issue for me as well.

Sean

July 30th, 2010 @ 10:04 PM
Sean Rhea
- Assigned user set to Jeremy Kemper
On further inspection, Chris's patch is not quite right. Attached is an alternate patch using code from Rack 1.1.0. It's working for me.
- 0001-fix-set-cookie-for-multiple-cookies.patch 1.39 KB
August 11th, 2010 @ 01:15 AM
Pascal Friederich

I've also provided a fix for that at #4941 session is not preserved between requests in integration tests

that patch utilizes Rack::Utils directly instead of duplicating the code

August 26th, 2010 @ 11:45 AM
Santiago Pastorino
- State changed from new to open
- Tag changed from abstract_store.rb, activerecord, active_record_store.rb, cookie, session to abstract_storerb, activerecord, active_record_storerb, cookie, session
This issue has been automatically marked as stale because it has not been commented on for at least three months.

The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

February 2nd, 2011 @ 04:33 PM
Santiago Pastorino
- State changed from open to stale
February 2nd, 2011 @ 04:33 PM

There was a problem

reset_session broken in rails 2.3.8 when using ActiveRecordStore for sessions

Activity