This project is archived and is in readonly mode.

#4938 ✓resolved
Michael Lovitt

[PATCH] Session fixes: sessions should not be created until written to; and session data should be destroyed on session reset

Reported by Michael Lovitt | June 22nd, 2010 @ 11:33 PM | in 2.3.9

This patch resolves two issues with Rails session behavior:

1) Sessions should not be created until written to

The implementation of Rails lazy-loading sessions (starting in Rails 2.3) is suboptimal -- specifically, Rails creates a new session, if one doesn't already exists, as soon as the session hash is accessed. So in a simple case where a session-less user requests an action like this:

def test
  render :text => session[:foo]
end

A new session will be created.

This create-on-read behavior is not ideal. Many Rails applications spread session reads all over the place (for example, by checking flash[:error] in layouts); in these cases, sessions are often created when they're not actually used, which diminishes the benefits of lazy-loading. To make things worse, the facility for disabling session creation on a controller-by-controller basis has been removed (starting in 2.3). So it's very difficult in Rails right now to ensure that sessions are only created when they're actually needed.

This issue is probably not a big deal for applications that use the cookie store -- though even in that case, eager session creation can interfere with page or reverse proxy caching schemes that check HTTP headers -- but for applications that use the server-side stores (ActiveRecord, Memcache), it's inefficient to execute writes and consume space on the server for sessions that are never used.

There are several open issues in Lighthouse related to this eager session-creating behavior:

It seems like a better approach is to create a session only when data is put into the session hash. A read against a non-existent session (i.e., a session associated with a request that contains no session id cookie) should not cause a session to be created. So, for a session-less user:

def test
  session[:foo]         # no session created
  session[:foo] = "bar" # session created!
end

The patch makes this change and includes tests for the new behavior.

2) Session data should be destroyed on session reset

One side-effect of the 2.3 session changes is that, for server-based session stores, the reset_session method does not delete session data from the server. The session cookie is cleared, and a new session hash is created, but the data in the underlying session store is left alone.

Of course, server-based session stores must clean up expired sessions anyway, so these garbage sessions are ultimately purged, but it's surprising and problematic that invalid sessions are not simply deleted. A user who fiddles with his cookies, for example, could start re-using a session that the application thinks has been destroyed.

This patch changes the behavior to the following: when a session is destroyed, the associated session data is removed from the store.

Note that this change requires that session stores implement a #destroy method. I've done so for all the built-in stores -- CookieStore (where destroy is noop), MemCacheStore, and ActiveRecord::SessionStore. (ActiveRecord::SessionStore.destroy simply calls destroy on the ActiveRecord.session_class; implementers such as ActiveRecord::SessionStore::Session and ActiveRecord::SessionStore::SqlBypass are already required, per the docs, to provide a destroy method.)

Comments and changes to this ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Referenced by

Pages