Merge pull request #33446 from ptoomey3/nested-respond-to

Raises exception when respond_to called multiple times in incompatible way

actionpack/CHANGELOG.md

@@ -1,3 +1,18 @@
+*.  Raises `ActionController::RespondToMismatchError` with confliciting `respond_to` invocations.
+
+    `respond_to` can match multiple types and lead to undefined behavior when
+    multiple invocations are made and the types do not match:
+
+        respond_to do |outer_type|
+          outer_type.js do
+            respond_to do |inner_type|
+              inner_type.html { render body: "HTML" }
+            end
+          end
+        end
+
+    *Patrick Toomey*
+
 *   `ActionDispatch::Http::UploadedFile` now delegates `to_path` to its tempfile.
 
     This allows uploaded file objects to be passed directly to `File.read`

actionpack/lib/action_controller/metal/exceptions.rb

@@ -51,6 +51,24 @@ module ActionController
   class UnknownFormat < ActionControllerError #:nodoc:
   end
 
+  # Raised when a nested respond_to is triggered and the content types of each
+  # are incompatible. For exampe:
+  #
+  #  respond_to do |outer_type|
+  #    outer_type.js do
+  #      respond_to do |inner_type|
+  #        inner_type.html { render body: "HTML" }
+  #      end
+  #    end
+  #  end
+  class RespondToMismatchError < ActionControllerError
+    DEFAULT_MESSAGE = "respond_to was called multiple times and matched with conflicting formats in this action. Please note that you may only call respond_to and match on a single format per action."
+
+    def initialize(message = nil)
+      super(message || DEFAULT_MESSAGE)
+    end
+  end
+
   class MissingExactTemplate < UnknownFormat #:nodoc:
   end
 end

actionpack/lib/action_controller/metal/mime_responds.rb

@@ -197,6 +197,9 @@ module ActionController #:nodoc:
       yield collector if block_given?
 
       if format = collector.negotiate_format(request)
+        if content_type && content_type != format
+          raise ActionController::RespondToMismatchError
+        end
         _process_format(format)
         _set_rendered_content_type format
         response = collector.response

actionpack/test/controller/mime/respond_to_test.rb

@@ -102,6 +102,26 @@ class RespondToController < ActionController::Base
     end
   end
 
+  def using_conflicting_nested_js_then_html
+    respond_to do |outer_type|
+      outer_type.js do
+        respond_to do |inner_type|
+          inner_type.html { render body: "HTML" }
+        end
+      end
+    end
+  end
+
+  def using_non_conflicting_nested_js_then_js
+    respond_to do |outer_type|
+      outer_type.js do
+        respond_to do |inner_type|
+          inner_type.js { render body: "JS" }
+        end
+      end
+    end
+  end
+
   def custom_type_handling
     respond_to do |type|
       type.html { render body: "HTML"    }
@@ -430,6 +450,20 @@ class RespondToControllerTest < ActionController::TestCase
     assert_equal "<p>Hello world!</p>\n", @response.body
   end
 
+  def test_using_conflicting_nested_js_then_html
+    @request.accept = "*/*"
+    assert_raises(ActionController::RespondToMismatchError) do
+      get :using_conflicting_nested_js_then_html
+    end
+  end
+
+  def test_using_non_conflicting_nested_js_then_js
+    @request.accept = "*/*"
+    get :using_non_conflicting_nested_js_then_js
+    assert_equal "text/javascript", @response.content_type
+    assert_equal "JS", @response.body
+  end
+
   def test_with_atom_content_type
     @request.accept = ""
     @request.env["CONTENT_TYPE"] = "application/atom+xml"