2.1.0+ can't remove keys from the session

[cheald]

transactional_write_session in 2.1.0+ merges stored sessions with the session being written. This means that the session can't remove keys!

Functionally, in my Rails 5.2 app, this means that the flash is never disappearing. In the course of a normal request, the flash stored in the session is consumed, and Rails, finding no further flash to store, deletes the flash key from session, but because redis-rack can't persist the session with the removed "flash" key, the flash is read from the existing store and merged into the payload written, effectively preserving it indefinitely.

Here's a simple test to illustrate the issue. Just add it into test/rack/session/redis_test.rb:

  it "doesn't persist keys that don't exist in the incoming session" do
    app = lambda do |env|
      req = Rack::Request.new(env)
      req.session[req.params["add"]] = true if req.params["add"]
      req.session.delete req.params["remove"] if req.params["remove"]
      Rack::Response.new(env["rack.session"].inspect).to_a
    end

    with_pool_management(app) do |pool|
      req = Rack::MockRequest.new(pool)
      res = req.get("/?add=foo")
      cookie = res["Set-Cookie"]
      session_id = cookie[session_match, 1]
      sid = Rack::Session::SessionId.new(session_id)

      session = pool.with { |c| c.get(sid.private_id) }
      session.must_equal("foo" => true)

      res = req.get("/?add=bar", "HTTP_COOKIE" => cookie)
      session = pool.with { |c| c.get(sid.private_id) }
      session.must_equal("foo" => true, "bar" => true)

      res = req.get("/?remove=foo", "HTTP_COOKIE" => cookie)
      session.must_equal("bar" => true) # <-- The test will fail here: the payload is {"foo" => true, "bar" => true}
    end
  end

[pelargir]

We're having the same problem (sticky flash messages) in v2.1.1 but when I downgrade to v2.1.0 the problem goes away. We're also using the latest rack v2.1.2 so that might have something to do with it. @cheald what version of rack are you using?

[ukdave]

Same here (flash messages not disappearing) in v2.1.1. Downgrading to v2.1.0 fixes the issue. I'm also using rack v2.1.2.

[tubbo]

i've yanked v2.1.1 and reverted the code that was causing the problem. Continuing to use v2.1.0 should be fine for now.

[cheald]

Ah, oops, I guess it is just 2.1.1. Thanks for the quick attention.

FWIW, I think that feature is broadly unnecessary. Redis operations are atomic and serialized already, and the behavior for other session stores (such as cookie) is that the last request to return wins. There's not really a race condition so much as a lack of transactions, and I'm of the opinion that if one needs transactional session storage, one should probably be using a transactional data store to get it.