Lifetime mismatch between session key and session value when `expire_after: nil` is set.

[hogelog]

I was developing a Rails app that switches the session store to redis and accidentally set expire_after: nil. I found a strange behavior with that setting and I'd like to report it.

• redis-rack stores session keys in cookies and session values in redis.
• expire_after: nil setting makes lifetime mismatch
  • Session key is saved as "session cookie" (expires when browser session ends)
  • Session value is saved in redis without ttl

With expire_after: nil setting, client's session key is sometime expired, but session value is not expired.
So the redis will accumulate more and more non-volatile keys.

Of course, I am aware that using the redis session store with expire_after: nil is not a good idea. However, the current behavior is confused for me.

Reproduction

I checked with the following code.

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "sinatra", "2.2.0"
  gem "webrick", "1.7.0"
  gem "redis-rack", "2.1.4"
end

require "sinatra/base"
require "redis-rack"

enable :sessions
set :session_store, Rack::Session::Redis, expire_after: nil

get "/" do
  session[:data] = "hello"
end

Sinatra::Application.run!

Procedure

• Launch above application with ruby app.rb
• Access by browser
  • Session key is saved as "session cookie" (expires when browser session ends)
    
  • Session value is saved in redis without ttl

    127.0.0.1:6379> ttl rack:session:2::8e895f0a1d6372551dc5cc220bf5d9d2e82540710540f2c19bbf697d5f3d7e1b (integer) -1

What to do

How the expire_after: nil setting should work.

1. Match the session key deadline

• Session key is stored in session cookie
• Session values as volatile as session keys
  • However, the timing of session cookie expiration is unknown, so set an appropriate default expiration date?

Storing session with expire_after: nil in the session cookie setting is rack's default behavior.
This method seems the least confusing.

2. Match the session value expiration date

• Session key is stored in non-volatile cookie
• Session values stored as redis without ttl

There may be some confusion because the default session behavior of rack.
However, it may be consistent within redis-rack.

3. Nothing to do

As I described earlier, I think it is a confusing and not happy behavior.

[tubbo]

@hogelog The behavior of :expire_after isn't defined by this gem, it's ultimately up to rack-session to define how this should work. The redis-store gem is solely responsible for passing options down to Redis properly.

I can see that on line 14, we're calling #to_i on the TTL, which given all of the options for expiry being nil, should result in setting the Redis TTL to 0. This effectively gives the keys no expiry at all.

The closest I'd like to get in this gem is option 3, but if you set expire_after: nil the gem should just throw an error. I can't think of a good reason why you would want to set a default session expiration to nil, and the other two solutions you describe are both just best guesses at the intention of the developer, which I feel would be too error-prone and could result in a lot of unexpected behavior, especially for any "power-users" of this gem. In a former project that I helped build, we defaulted the session expiry time so that it would never be nil, because we ran into similar problems: https://github.com/workarea-commerce/workarea/blob/master/core/config/initializers/22_session_store.rb#L10, so I would vote for preventing this kind of behavior at all since it seems to be a state you don't ever want to end up in.

[hogelog]

I can't think of a good reason why you would want to set a default session expiration to nil

Yes. I had a little accident on my application with the redis-rack gem this expire_after: nil setting's behavior. But my application's expire_after: nil setting is accidently mixed in. So your idea that "if you set expire_after: nil the gem should just throw an error" is sounds good for me.

[tubbo]

@hogelog yeah I can imagine something like setting that expire after to an env var that doesn't exist would trigger this behavior. Looking into it now to see if this would be safe.