Ticket #10048: fix_broken_session_fixation_catching.patch

actionpack/test/controller/session_fixation_test.rb

@@ +1 @@
+require File.dirname(__FILE__) + '/../abstract_unit'
+
+class SessionFixationTest < Test::Unit::TestCase
+  class MockCGI < CGI #:nodoc:
+    attr_accessor :stdoutput, :env_table
+
+    def initialize(env, data = '')
+      self.env_table = env
+      self.stdoutput = StringIO.new
+      super(nil, StringIO.new(data))
+    end
+  end
+
+  class TestController < ActionController::Base
+    session :session_key => '_myapp_session_id', :secret => 'secret', :except => :default_session_key
+    session :cookie_only => false, :only => :allow_session_fixation
+
+    def default_session_key
+      render :text => "default_session_key"
+    end
+
+    def custom_session_key
+      render :text => "custom_session_key: #{params[:id]}"
+    end
+
+    def allow_session_fixation
+      render :text => "allow_session_fixation"
+    end
+
+    def rescue_action(e) raise end
+  end
+
+  def setup
+    @controller = TestController.new
+  end
+
+  def test_should_be_able_to_make_a_successful_request
+    cgi = mock_cgi_for_request_to(:custom_session_key, :id => 1)
+
+    assert_nothing_raised do
+      @controller.send(:process, ActionController::CgiRequest.new(cgi, {}), ActionController::CgiResponse.new(cgi))
+    end
+    assert_equal 'custom_session_key: 1', @controller.response.body
+    assert_not_nil @controller.session
+  end
+
+  def test_should_catch_session_fixation_attempt
+    cgi = mock_cgi_for_request_to(:custom_session_key, :_myapp_session_id => 42)
+
+    assert_raises ActionController::CgiRequest::SessionFixationAttempt do
+      @controller.send(:process, ActionController::CgiRequest.new(cgi, {}), ActionController::CgiResponse.new(cgi))
+    end
+    assert_nil @controller.session
+  end
+
+  def test_should_not_catch_session_fixation_attempt_when_cookie_only_setting_is_disabled
+    cgi = mock_cgi_for_request_to(:allow_session_fixation, :_myapp_session_id => 42)
+
+    assert_nothing_raised do
+      @controller.send(:process, ActionController::CgiRequest.new(cgi, {}), ActionController::CgiResponse.new(cgi))
+    end
+    assert ! @controller.response.body.blank?
+    assert_not_nil @controller.session
+  end
+
+  def test_should_catch_session_fixation_attempt_with_default_session_key
+    ActionController::Base.session_store = :p_store # using the default session_key is not possible with cookie store
+    cgi = mock_cgi_for_request_to(:default_session_key, :_session_id => 42)
+
+    assert_raises ActionController::CgiRequest::SessionFixationAttempt do
+      @controller.send(:process, ActionController::CgiRequest.new(cgi, {}), ActionController::CgiResponse.new(cgi))
+    end
+    assert @controller.response.body.blank?
+    assert_nil @controller.session
+  end
+
+private
+
+  def mock_cgi_for_request_to(action, params = {})
+    MockCGI.new({
+      "REQUEST_METHOD" => "GET",
+      "QUERY_STRING"   => "action=#{action}&#{params.to_query}",
+      "REQUEST_URI"    => "/",
+      "SERVER_PORT"    => "80",
+      "HTTP_HOST"      => "testdomain.com" }, '')
+  end
+
+end

actionpack/lib/action_controller/cgi_process.rb

@@ -33 +33 @@
-  end
-
-  class CgiRequest < AbstractRequest #:nodoc:
-, :cookie_only
+
-    class SessionFixationAttempt < StandardError; end #:nodoc:
-
-    DEFAULT_SESSION_OPTIONS = {
-      :database_manager => CGI::Session::CookieStore, # store data in cookie
-      :prefix           => "ruby_sess.",    # prefix session file names
-      :session_path     => "/",             # available to all paths in app
+      :session_key      => "_session_id",
-      :cookie_only      => true
-    } unless const_defined?(:DEFAULT_SESSION_OPTIONS)
-
@@ -47 +48 @@
-      @cgi = cgi
-      @session_options = session_options
-      @env = @cgi.send!(:env_table)
-      @cookie_only = session_options.delete :cookie_only
-      super()
-    end
-
@@ -112 +112 @@
-          @session = Hash.new
-        else
-          stale_session_check! do
-@cookie_only && request
+cookie_only? && query
-              raise SessionFixationAttempt
-            end
-            case value = session_options_with_string_keys['new_session']
@@ -158 +158 @@
-        end
-      end
-
+      def cookie_only?
+        session_options_with_string_keys['cookie_only']
+      end
+
-      def stale_session_check!
-        yield
-      rescue ArgumentError => argument_error