From b76b817629bf1504e3109c9872b250513690561d Mon Sep 17 00:00:00 2001
From: Sean Griffin <sean@seantheprogrammer.com>
Date: Fri, 11 Mar 2016 10:14:50 -0700
Subject: Use the most highest priority exception handler when cause is set

There was some subtle breakage caused by #18774, when we removed
`#original_exception` in favor of `#cause`. However, `#cause` is
automatically set by Ruby when raising an exception from a rescue block.
With this change, we will use whichever handler has the highest priority
(whichever call to `rescue_from` came last). In cases where the outer
has lower precidence than the cause, but the outer is what should be
handled, cause will need to be explicitly unset.

Fixes #23925
---
 actionpack/lib/action_controller/metal/rescue.rb |  8 ++++--
 actionpack/test/controller/rescue_test.rb        | 33 ++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 2 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_controller/metal/rescue.rb b/actionpack/lib/action_controller/metal/rescue.rb
index 81b9a7b9ed..0621a7368c 100644
--- a/actionpack/lib/action_controller/metal/rescue.rb
+++ b/actionpack/lib/action_controller/metal/rescue.rb
@@ -7,8 +7,12 @@ module ActionController #:nodoc:
     include ActiveSupport::Rescuable
 
     def rescue_with_handler(exception)
-      if exception.cause && handler_for_rescue(exception.cause)
-        exception = exception.cause
+      if exception.cause
+        handler_index = index_of_handler_for_rescue(exception) || Float::INFINITY
+        cause_handler_index = index_of_handler_for_rescue(exception.cause)
+        if cause_handler_index && cause_handler_index <= handler_index
+          exception = exception.cause
+        end
       end
       super(exception)
     end
diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb
index f42bef883f..ed78f859ce 100644
--- a/actionpack/test/controller/rescue_test.rb
+++ b/actionpack/test/controller/rescue_test.rb
@@ -147,6 +147,24 @@ class RescueController < ActionController::Base
     end
   end
 
+  def exception_with_more_specific_handler_for_wrapper
+    raise RecordInvalid
+  rescue
+    raise NotAuthorized
+  end
+
+  def exception_with_more_specific_handler_for_cause
+    raise NotAuthorized
+  rescue
+    raise RecordInvalid
+  end
+
+  def exception_with_no_handler_for_wrapper
+    raise RecordInvalid
+  rescue
+    raise RangeError
+  end
+
   protected
     def deny_access
       head :forbidden
@@ -301,6 +319,21 @@ class RescueControllerTest < ActionController::TestCase
     get :resource_unavailable_raise_as_string
     assert_equal "RescueController::ResourceUnavailableToRescueAsString", @response.body
   end
+
+  test 'rescue when wrapper has more specific handler than cause' do
+    get :exception_with_more_specific_handler_for_wrapper
+    assert_response :unprocessable_entity
+  end
+
+  test 'rescue when cause has more specific handler than wrapper' do
+    get :exception_with_more_specific_handler_for_cause
+    assert_response :unprocessable_entity
+  end
+
+  test 'rescue when cause has handler, but wrapper doesnt' do
+    get :exception_with_no_handler_for_wrapper
+    assert_response :unprocessable_entity
+  end
 end
 
 class RescueTest < ActionDispatch::IntegrationTest
-- 
cgit v1.2.3