Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Handle existing rack.session where the app modifies session vars #411

Merged
merged 1 commit into from

3 participants

@jamie

ID#prepare_session calls merge! on the newly-created SessionHash, but this method is not overridden to parse existing data. As such, if the app modifies session values, it winds up discarding the entire previous session.

I saw there was already a spec for a non-session-modifying app which ran fine, but failed exactly like I was experiencing when I used the example incrementing app.

For me, this was breaking Rack::Test while testing a Sinatra app.

@jamie jamie load session data for merge!
ID#prepare_session calls merge! on the newly-created SessionHash, but
this method is not overridden to parse existing data. As such, any
previous session data passed in from an earlier middleware is discarded.

For me, this was breaking Rack::Test while testing a Sinatra app.
630c4c3
@travisbot

This pull request passes (merged 630c4c3 into e4172e7).

@raggi raggi merged commit 03b1d6f into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 19, 2012
  1. @jamie

    load session data for merge!

    jamie authored
    ID#prepare_session calls merge! on the newly-created SessionHash, but
    this method is not overridden to parse existing data. As such, any
    previous session data passed in from an earlier middleware is discarded.
    
    For me, this was breaking Rack::Test while testing a Sinatra app.
This page is out of date. Refresh to see the latest.
Showing with 12 additions and 0 deletions.
  1. +5 −0 lib/rack/session/abstract/id.rb
  2. +7 −0 test/spec_session_cookie.rb
View
5 lib/rack/session/abstract/id.rb
@@ -120,6 +120,11 @@ def empty?
super
end
+ def merge!(hash)
+ load_for_write!
+ super
+ end
+
private
def load_for_read!
View
7 test/spec_session_cookie.rb
@@ -338,4 +338,11 @@ def call(env)
response = response_for(:app => session_id, :request => request)
response.body.should.match(/foo/)
end
+
+ it "allows modifying session data with session data from middleware in front" do
+ request = { 'rack.session' => { :foo => 'bar' }}
+ response = response_for(:app => incrementor, :request => request)
+ response.body.should.match(/counter/)
+ response.body.should.match(/foo/)
+ end
end
Something went wrong with that request. Please try again.