#4450 'expire_after' option on session forces session creation on each and every action - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#4450 ✓duplicate

'expire_after' option on session forces session creation on each and every action

Reported by Olek Poplavsky | April 21st, 2010 @ 04:29 PM

First of all, this bug was found in rails 2.3.5.

As we all know, sessions in rails 2.x are loaded/created when they are first referenced.

Therefore, if some action is to be hit many times by 'first time users' that we have no interest in tracking, one have to be just very vigilant about never referencing 'session' and junk session objects are not going to be created in database (yes, there are still some of us using active record session store, and not willing to switch to cookie based session store).

If somebody knows of a less fragile approach to creating session-less actions, please do let me know. Personally, I liked the old pre-2.0 behavior of sessions being always loaded, except in pre-configured cases. It was easy and foolproof approach that guaranteed that sessions are not going to be loaded/created for some actions, no matter what.

So, this fragile approach worked for us OK until I started to use new (and great) feature in rails, specifying ':expire_after => 2.weeks'.

Core functionality works great, cookies are indeed marked to expire in 2 weeks. But the side effect is that session object is CREATED in database for every request now, including those 'session-less' actions. In our case, it leads to ultra-fast bloat of sessions table with obvious performance implications.

For some strange reason this bug may be difficult to reproduce. It does not happen on my development machine (mac os x), but it does happen in our testing/staging/production environments (Ubuntu Hardy, a lot of servers). I do not understand why, those environments are very similar (we use bundler and package all our gems in the project).

I found a workaround, that hopefully will benefit somebody else in same situation.

It involves NOT setting expire_after in session initializer, but doing that in application controller, and doing that conditionally, only when it makes sense to have sessions.

  before_filter :change_session_expiration_time

  protected

  def change_session_expiration_time
    if session_allowed? # implement your own
      request.session_options = request.session_options.dup
      request.session_options[:expire_after] = 5.days
    end
  end

Comments and changes to this ticket

You flagged this item as spam.
Surendra Singhi May 26th, 2010 @ 06:48 PM
- Assigned user set to “Yehuda Katz (wycats)”
You flagged this item as spam.
Michael Lovitt June 28th, 2010 @ 11:19 PM
I think this issue is fixed by #4938.
Jeremy Kemper June 28th, 2010 @ 11:30 PM
- State changed from “new” to “duplicate”
- Importance changed from “” to “Low”
You flagged this item as spam.
Fotos Georgiadis July 12th, 2010 @ 12:55 AM
I can't see how the latest patch attached to #4938 (even tho a great work) is going to solve this issue.

If :expire_after is given a value then on AbstractStore#call the session will be loaded. Perhaps this bug is invalid since by defining :expire_after then cookies must be send for every request (due to the very nature of "expire_after").

Still there are requests that you want to explicitly turn-off sessions no matter what.
IMHO removing session :off was a bad idea. Please bring back the option to turn-off sessions!
You flagged this item as spam.
Michael Lovitt July 14th, 2010 @ 05:01 AM
- Tag changed from “rails 2.3.5, sessions” to “rails 2.3.5, 3.x, sessions”
Fotos is correct; the fix for #4938 does not resolve this issue.

Let me see if I can sum up the current situation, which I believe has grown out of two conflicting goals:

1) We want to load session data from the session store only when the session is accessed.
2) When we're using the :expire_after option, we want to keep the user's session cookie expiration date fresh -- so, for example, if we've set session cookies to :expire_after => 2.weeks, then every time the user requests a page in our app, we want to reset the expiration date on the cookie to 2 weeks from right now.

We attempt to accomplish the first goal with lazy-loaded sessions, but we undermine ourselves on the second goal, by loading the session on every request when the expire_after option is set. That is, when the expire_after option is set, sessions are created/loaded all the time, and lazy loaded sessions are effectively disabled. So, as it stands, our desire to keep the session cookie expiration date fresh is overriding our desire to load session data only when we need it.

This lazy-loading-thwarting expire_after behavior was introduced in commit 2ae8d307, and, honestly, I would like to see it removed, such that sessions are always loaded lazily, and the cookie expiration date is only freshened when the session is accessed. I think this behavior would be reasonable and expected -- expire_after sets a date in the future after which an unused session effectively expires. If the session has not been accessed, then it has not been used, and so there's no reason to reset its expiration date.

Since this is a relatively straightforward change to make and test, I went ahead and did so. The patch (for master) is attached. If folks agree with it, then I would be happy to see it applied.

(Alternatively, we could try to be more clever -- for instance, perhaps we treat the cookie store differently than the other stores, since it is less costly with the cookie store to create and load sessions. Or maybe we only load the session and reset the expiration date for an existing session (where we assume a session exists if a session cookie was included in the request) but not for a nonexistent session. And in the existing session case, we could conceivably reset the expiration date without loading anything from the server-side store, by simply sending back in the response exactly the cookie that was sent in the request, only with a new expiration date. This doesn't seem too weird, and I would not mind exploring this approach if others think the extra effort and complexity would be worthwhile. But the simple change I've submitted seems good to me.)
You flagged this item as spam.
Michael Lovitt July 14th, 2010 @ 05:12 AM
- no changes were found...
- expire_after_fix.diff 3.5 KB
You flagged this item as spam.
Olek Poplavsky July 14th, 2010 @ 03:32 PM
I agree that resetting cookie expiration date only when session is accessed is much better than forcing it every single time, like it happens right now. That kind of fits the 'principle of least surprise' (at the very least for me).
You flagged this item as spam.
Fotos Georgiadis July 14th, 2010 @ 09:13 PM
Spot on description Michael. Couldn't have said that better!

I second this patch as well. Would be nice to see this in 2.3 as well.

The other approach described ("if a cookie was send with the request and if the session hasn't been accessed then just send back the same cookie but with an updated expiration date only") seems to the be expected way things should work. That was the initial impression I had given the option name (:expire_after).

Keep in mind that cookies persist, after closing the browser, only if they have an expiration date set.
So this is kinda important to get working correctly if you want to track^H ahem remember users.

But even with this small fix I'm good.
You flagged this item as spam.
Fjan December 3rd, 2010 @ 01:04 PM
I agree that this patch is a good idea but I think we should go one step further and only update the session if it has both been accessed AND been changed, see 6111 for an explanation + patch.