Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use request.GET and request.POST instead of request.params in abstract_store #5817

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ def extract_session_id(env)
stale_session_check! do
request = ActionDispatch::Request.new(env)
sid = request.cookies[@key]
sid ||= request.params[@key] unless @cookie_only
sid ||= (request.GET[@key] || request.POST[@key]) unless @cookie_only
sid
end
end
Expand Down
41 changes: 38 additions & 3 deletions actionpack/test/activerecord/active_record_store_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ def get_session_value
render :text => "foo: #{session[:foo].inspect}"
end

def test_param
raise MiniTest::Assertion.new("param not set") unless 'bar' == params[:test_param]

head :ok
end

def get_session_id
render :text => "#{request.session_options[:id]}"
end
Expand Down Expand Up @@ -181,8 +187,23 @@ def test_prevents_session_fixation
end
end

class NastySessionMiddleware
def initialize(app, args={})
@app = app
@args = args
end

def call(env, options={})
# Call params, which sets env variable
session_middleware = ActiveRecord::SessionStore.new(@app, @args)
session_middleware.send :extract_session_id, env
session_middleware.call(env)
@app.call(env)
end
end

def test_allows_session_fixation
with_test_route_set(:cookie_only => false) do
with_test_route_set_and_middleware(ActiveRecordStoreTest::NastySessionMiddleware, :cookie_only => false) do
get '/set_session_value'
assert_response :success
assert cookies['_session_id']
Expand All @@ -198,6 +219,15 @@ def test_allows_session_fixation
get '/set_session_value', :_session_id => session_id, :foo => "baz"
assert_response :success
assert_equal session_id, cookies['_session_id']
assert_equal 'baz', request.params[:foo]

reset!

get '/test_param/bar'
assert_response :success
assert_equal 'bar', request.params[:test_param]

reset!

get '/get_session_value', :_session_id => session_id
assert_response :success
Expand All @@ -208,14 +238,19 @@ def test_allows_session_fixation

private

def with_test_route_set(options = {})
def with_test_route_set(options = {}, &block)
with_test_route_set_and_middleware(nil, options, &block)
end

def with_test_route_set_and_middleware(session_middleware, options = {})
with_routing do |set|
set.draw do |map|
match ':action', :to => 'active_record_store_test/test'
match '/test_param/:test_param', :to => 'active_record_store_test/test#test_param'
end

@app = self.class.build_app(set) do |middleware|
middleware.use ActiveRecord::SessionStore, options.reverse_merge(:key => '_session_id')
middleware.use (session_middleware || ActiveRecord::SessionStore), options.reverse_merge(:key => '_session_id')
middleware.delete "ActionDispatch::ShowExceptions"
end

Expand Down