Use `#performed?` to terminate controller callbacks

Since 69009f, `ActionController::Metal::DataStreaming#send_file` doesn't
set `@_response_body` anymore.

`AbstractController::Callbacks` used `@_response_body` in its callback
terminator, so it failed to halt the callback cycle when using `#send_file`
from a `before_action`.

Instead, it now uses `#performed?` on `AbstractController::Base` and
`ActionController::Metal`, which checks `response.committed?`, besides
 checking if `@_response_body` is set, if possible.

Example application: https://gist.github.com/jeffkreeftmeijer/78ae4572f36b198e729724b0cf79ef8e

3 files changed, 23 insertions, 1 deletions

diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb
index d4317399ed..aa06f70433 100644
--- a/actionpack/lib/abstract_controller/base.rb
+++ b/actionpack/lib/abstract_controller/base.rb
@@ -150,6 +150,13 @@ module AbstractController
       _find_action_name(action_name)
     end
 
+    # Tests if a response body is set. Used to determine if the
+    # +process_action+ callback needs to be terminated in
+    # +AbstractController::Callbacks+.
+    def performed?
+      response_body
+    end
+
     # Returns true if the given controller is capable of rendering
     # a path. A subclass of +AbstractController::Base+
     # may return false. An Email controller for example does not
diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb
index d63ce9c1c3..3ef8da86fa 100644
--- a/actionpack/lib/abstract_controller/callbacks.rb
+++ b/actionpack/lib/abstract_controller/callbacks.rb
@@ -9,7 +9,7 @@ module AbstractController
 
     included do
       define_callbacks :process_action,
-                       terminator: ->(controller, result_lambda) { result_lambda.call if result_lambda.is_a?(Proc); controller.response_body },
+                       terminator: ->(controller, result_lambda) { result_lambda.call if result_lambda.is_a?(Proc); controller.performed? },
                        skip_after_callbacks_if_terminated: true
     end
 
diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb
index 2820425c31..9df70dacbf 100644
--- a/actionpack/test/controller/send_file_test.rb
+++ b/actionpack/test/controller/send_file_test.rb
@@ -11,6 +11,8 @@ class SendFileController < ActionController::Base
   include ActionController::Testing
   layout "layouts/standard" # to make sure layouts don't interfere
 
+  before_action :file, only: :file_from_before_action
+
   attr_writer :options
   def options
     @options ||= {}
@@ -20,6 +22,10 @@ class SendFileController < ActionController::Base
     send_file(file_path, options)
   end
 
+  def file_from_before_action
+    raise 'No file sent from before action.'
+  end
+
   def test_send_file_headers_bang
     options = {
       :type => Mime[:png],
@@ -192,6 +198,15 @@ class SendFileTest < ActionController::TestCase
     assert_nil @controller.headers['Content-Disposition']
   end
 
+  def test_send_file_from_before_action
+    response = nil
+    assert_nothing_raised { response = process('file_from_before_action') }
+    assert_not_nil response
+
+    assert_kind_of String, response.body
+    assert_equal file_data, response.body
+  end
+
   %w(file data).each do |method|
     define_method "test_send_#{method}_status" do
       @controller.options = { :stream => false, :status => 500 }